simonlindholm / asm-differ

Assembly diff script
The Unlicense
106 stars 52 forks source link

Ignore hex/decimal differences in 'li' on MIPS #134

Closed mkst closed 10 months ago

mkst commented 10 months ago

Is this the best way to tackle this? I put it behind a guard so we aren't trying to do a regex match for every line...

With this change: image Without: image

[edit] Just seen the issue here https://github.com/simonlindholm/asm-differ/issues/121, which is pretty much what this PR is trying to address - outside of pseudo operations, its just trying to ignore 0x6 vs 6...

mkst commented 10 months ago

I've been playing around with this and only 1 thru 9 appear as 1 thru 9 (rather than 0x1 thru 0x9). Negative values are decimal, and values above 10 are hexidecimal. Does this still need to go behind an option - I know that -1 and 0xFF are different values under IDO, but this PR won't treat them as equivalent (after my latest changes)

AngheloAlf commented 10 months ago

Here's a scratch where the difference between the two matters https://decomp.me/scratch/xv8L5

This doesn't match when linked, so asm-differ showing no diffs would be misleading.

AngheloAlf commented 10 months ago

btw, for some reason when turning off pseudo instructions asm-differ shows dli $t1, 0x1 instead of ori $t1, $zero, 0x1. This seems to be some weird bug on asm-differ, since this doesn't happen on objdump

mkst commented 10 months ago

Wow, I had no idea IDO was this awful 😅 I'll stick it behind an option, one for tomorrow

simonlindholm commented 10 months ago

Have you looked into patching compiler/assembler to avoid this diff instead? Would be nice to avoid option bloat. (Though I'm not really opposed to adding one, either.)

simonlindholm commented 10 months ago

btw, for some reason when turning off pseudo instructions asm-differ shows dli $t1, 0x1 instead of ori $t1, $zero, 0x1. This seems to be some weird bug on asm-differ, since this doesn't happen on objdump

Guessing this is due to objdump flags. I don't know which ones decomp.me passes

mkst commented 10 months ago

Could be a mips1 vs mips2 thing. The assembler is gnu as here... I'll see whether it makes any difference to change the value in maspsx before it goes to as... But that won't help the N64 SN stuff.

[edit]: The instruction being assembled is li $v0,0x00000002, so I really don't understand where the 0x2 vs 2 is coming from.

mkst commented 10 months ago

Setting the arch for mipsel turns the dli into ori:

MIPSEL_SETTINGS = replace(MIPS_SETTINGS, name="mipsel", big_endian=False, arch_flags=["-m", "mips:3000"])

image

mkst commented 10 months ago

Very open to opinions on the name of the config option, ignore_equivalent_immediates was the best my tired brain could come up with.

AngheloAlf commented 10 months ago

btw, for some reason when turning off pseudo instructions asm-differ shows dli $t1, 0x1 instead of ori $t1, $zero, 0x1. This seems to be some weird bug on asm-differ, since this doesn't happen on objdump

Guessing this is due to objdump flags. I don't know which ones decomp.me passes

It doesn't seem to be a decomp.me issue since it happens locally too

mkst commented 10 months ago

I'll need to add this to create_config() to be able to use it in decomp.me...

mkst commented 10 months ago

OK chaps, please re-review now that I've made some changes; again, if you can think of a better (shorter) name for this option please shout!

mkst commented 10 months ago

This PR will become less important for anyone using maspsx because I've just pushed a commit to expand the li instruction.

simonlindholm commented 10 months ago

You didn't add a cmdline flag, so this doesn't work as is. Actually, I think it should probably be a project setting, since it feels assembler-/project-specific.

Given the commit to maspsx though, is there still a use-case for this? Or should we close it and reopen if the need comes up again?

mkst commented 10 months ago

I'm happy to just close this one, glædelige jul 🎅