mkst / maspsx

MIT License
5 stars 4 forks source link

`li` instruction after `div` expansion does not get expanded properly #46

Closed bismurphy closed 10 months ago

bismurphy commented 10 months ago

First note, I'm not certain if the title of this issue is accurate.

As context, I have a matching scratch for a function here: https://decomp.me/scratch/U1RC7

Assembly line 0x6e8 uses a li a0,0x1000, which of course needs to be expanded. In the game binary, this is a ori $a0, $zero, 0x1000, but when I build, I get addiu $a0, $zero, 0x1000.

To diagnose the issue, I broke up the build process by running up until the step which invokes maspsx. That produced the following file:

https://gist.github.com/bismurphy/c45f2baa19144e249da115cab8bfdd35

The relevant instruction is under the LM188: label.

To replicate the problem, go to the sotn-decomp root directory, and copy the above gist file into that root directory (of course, you don't actually have to do this in the sotn-decomp directory, you could do it on a "naked" maspsx, but I'm describing my own process). Then, with the file in the working directory with the filename maspsx_testcase, run:

cat maspsx_testcase | python3 tools/maspsx/maspsx.py --no-macro-inc --expand-div --expand-li > maspsx_output.txt

If you then open maspsx_output.txt, you'll find that under LM188, there is a commented copy of the li line, as well as a non-commented version right above. But both of them retain the li, rather than expanding it according to the expand-li option.

I believe the root of this issue is here: https://github.com/mkst/maspsx/blob/master/maspsx/__init__.py#L413 in the fact that, if an instruction is skipped (which in this case the li is), then it will never get a chance to run the expand-li logic, and will not be expanded. This leads to the li instruction going through to the assembler unchanged. Ultimately, this is because the li instruction is sandwiched directly between div instructions which each get expanded.

I am not familiar enough with maspsx to propose a fix that would solve this, but hopefully this issue report is detailed enough to fully describe the problem. I hope having the compiled file to directly pass to maspsx is also helpful.

mkst commented 10 months ago

Closed via #47.