vnmakarov / mir

A lightweight JIT compiler based on MIR (Medium Internal Representation) and C11 JIT compiler and interpreter based on MIR
MIT License
2.29k stars 145 forks source link

Miscompilation on bbv branch due to dead store elimination #308

Open remy-luisant opened 1 year ago

remy-luisant commented 1 year ago

The following code will be miscompiled on the bbv branch.

f:  func    p:load_pointer
    local   i64:effective_addr, i64:other_stack, i64:A7

# 1 arg, 3 locals, 0 globals
    mov A7, u32:4(load_pointer)
    mov other_stack, u32:12(load_pointer)
    mov effective_addr, 262144
    mov A7, effective_addr
    mov u32:4(load_pointer), A7
    mov u32:12(load_pointer), other_stack
    ret
    endfunc
    endmodule

It seems that the wrong store will be eliminated. The store at offset 4 will be cut and the one at offset 12 will stay. I'm fully aware that this is not a stable branch and bugs are to be expected. This is not critical, and can be left alone.

remy-luisant commented 1 year ago

Might be related to displacement handling through the GVN, though I'm not sure of that. Seems like we have some memory get assigned to the same value in GVN, despite them differing in displacement. I might be wrong on this.

remy-luisant commented 1 year ago

I did more digging. Seems like the error happens before the GVN, even. I'm seeing aliasing between locations that should not alias.

alias between   mov u32:(load_pointer), R0 and  mov R0, u32:(load_pointer)
alias between   mov u32:(load_pointer), R0 and  mov R1, u32:4(load_pointer)

This would point to the problem being before availability. Possibly may_mem_alias_p or before.

remy-luisant commented 1 year ago

Manually adding the alias information fixes the problem for me. That said, there still is the issue of the compiler not being sufficiently conservative when it comes to performing the work on memory, in the absence of this information. I might try to come up with a patch, if I have time.