nlsandler / write_a_c_compiler

Test suite to help you write your own C compiler
https://norasandler.com/2017/11/29/Write-a-Compiler.html
MIT License
855 stars 97 forks source link

Logical AND mistake #5

Closed saitouena closed 5 years ago

saitouena commented 6 years ago

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.

nlsandler commented 5 years ago

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.

saitouena commented 5 years ago

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.