nlsandler/write_a_c_compiler

Logical AND mistake

saitouena opened this issue · 2 comments

In code generation phase of Logical AND, this code:

    <CODE FOR e1 GOES HERE>
    push  %eax            ;save value of e1 on the stack
    <CODE FOR e2 GOES HERE>
    pop   %ecx            ;pop e1 from the stack into ECX
    ; Step 1: SET CL = 1 iff e1 != 0
    cmpl  $0, %ecx        ;compare e1 to 0
    setne %cl             ;set CL to 1 iff e1 != 0
    ; Step 2: SET AL = 1 iff e2 != 0
    cmpl  $0, %eax        ;compare e2 to 0
    movl  $0, %eax        ;zero EAX register before storing result
    setne %al             ;set AL to 1 iff e2 != 0
    ; Step 3: compute al & cl
    andb  %cl, %al        ;store AL & CL in AL

should be:

    <CODE FOR e1 GOES HERE>
    push  %eax            ;save value of e1 on the stack
    <CODE FOR e2 GOES HERE>
    pop   %ecx            ;pop e1 from the stack into ECX
    ; Step 1: SET CL = 1 iff e1 != 0
    cmpl  $0, %ecx        ;compare e1 to 0
    movl  $0, %ecx        ; HERE:
    setne %cl             ;set CL to 1 iff e1 != 0
    ; Step 2: SET AL = 1 iff e2 != 0
    cmpl  $0, %eax        ;compare e2 to 0
    movl  $0, %eax        ;zero EAX register before storing result
    setne %al             ;set AL to 1 iff e2 != 0
    ; Step 3: compute al & cl
    andb  %cl, %al        ;store AL & CL in AL

I think movl $0, %ecx shoud be.

Thanks for filing this issue! I actually don't think %ecx needs to be zeroed out here. We need to zero out the upper bytes of %eax because it is holds the final result of the AND expression, which will be used later in the program - for example, it might be a function's return value, or might be saved in a variable.

However, our compiler only uses %ecx to store intermediate results, so we know the value currently stored in %cl isn't ever examined again - it's only used in the final andb instruction, which doesn't look at the upper bytes. Therefore, those bytes don't need to be zeroed out.

Please feel free to reopen this issue if you have more questions or if I've misunderstood you.

I mistook. You are right.

it's only used in the final andb instruction, which doesn't look at the upper bytes.

I didn't know this.