Incorrect reordering of function calls & memory reads
zbanks opened this issue · 0 comments
zbanks commented
From Discord:
I think mips2c is generating incorrect code. For func_80133038 in MM, it gives
extern ? D_801C5C9C; s32 func_80133038(GlobalContext *globalCtx, s8 *arg1, struct_80133038_arg2 *arg2) { u8 temp_s0; u8 temp_v0; do { temp_v0 = *arg1; temp_s0 = *(&D_801C5C9C + temp_v0); arg1 = &arg1[temp_s0]; } while ((&D_801C5C50)[temp_v0](globalCtx, &arg1, arg2) == 0); return arg2->unkC; }but what ended up matching was
s32 func_80133038(GlobalContext* globalCtx, u8* arg1, struct_80133038_arg2* arg2) { u8 size; s32 ret; do { size = D_801C5C9C[*arg1]; ret = (*D_801C5C50[*arg1])(globalCtx, &arg1, arg2); arg1 += size; } while (ret == 0); return arg2->unkC; }mips2c is changing arg1 before the function call when what's actually happening is it changes after the function call.
This was narrowed down to needing a prevent_later_function_calls()
on reads.
However, it's tricky to find the right balance of when to add this. Fixing func_80133038
usually makes typical output worse -- emitting incorrect code seems to be rare.
I think theoretically prevent_later_function_calls()
would need to be added for all reads (which definitely makes the output worse).