matt-kempster / m2c

A MIPS and PowerPC decompiler.
GNU General Public License v3.0
411 stars 49 forks source link

fold_divmod mwcc inner add fix #265

Closed SeekyCt closed 1 year ago

SeekyCt commented 1 year ago

Currently fold_divmod's translation of (MULT_HI(x, N) + x) >> M --> MULT_HI(x, N) >> M assumes that x and N will be in that order inside of the MULT_HI, however that assumption fails for some MWCC code. This PR just allows for either argument to be equal to what's being added for this translation, which allows for a lot more of the divisions to be picked up on MWCC.

There's still a couple of other issues with MWCC for fold_divmod I'd like to try figure out too:

simonlindholm commented 1 year ago

The division by 3 in test_s32_div seems to be resolving the ((u32) MULT_HI(0x55555556, sp8) >> 0x1FU) into (sp8 / 6442450941) early, which means that the outer error removal ((x / N) + ((x / N) >> 31)) --> x / N can't take place (the statement is (MULT_HI(0x55555556, sp8) + (sp8 / 6442450941)) by the time this runs). I'm not sure I understand the logic & flow of this enough to know how to fix this, so any advice there would be appreciated

Hm. This can be pretty tricky, our pattern matching setup is wonky and not very flexible. In this particular case there's an easy out though: refuse to detect a division by more than 2^32.

SeekyCt commented 1 year ago

Thanks, that does work to block it but made me realise the actual issue here is different. The true problem is that the final if statement converting mult_his into divisions was also assuming that right_expr would be the literal, but if the original_expr is the mult_hi then this isn't actually guaranteed for mwcc, just fixing that massively improved results. Will try get another PR ready soon.