[PATCH] D17540: [MIPS][LLVM-MC] Fix Disassemble of Negative Offset

[Daniel Sanders via llvm-commits]

dsanders accepted this revision.

================
Comment at: test/MC/Disassembler/Mips/mips32r6/valid-mips32r6.txt:36
@@ -35,2 +35,3 @@
 0x04 0x11 0x14 0x9b # CHECK: bal 21104
 # FIXME: The encode/decode functions are not inverses of each other.
+0x18 0x02 0x01 0x4d # CHECK: blezalc $2, 1336
----------------
dsanders wrote:
> nitesh.jain wrote:
> > dsanders wrote:
> > > Are these FIXME's still true? If you assemble the mnemonics from the disassembler do you get the same opcodes?
> > > I suspect this change may have fixed the ones next to the tests you've updated.
> > The encoding and decoding are not inverse of each other. Since while decoding the offset is left shifted by 2 and then 4 is added but in case of encoding it will just right shift by 2. Thus for example
> > 
> > encoding 0x18 0x42 0x01 0x4d -->disassemble -->bgezalc $2, 1336
> > bgezalc $2, 1336-->assemble-->0x18 0x42 0x01 0x4e
> > 0x18 0x42 0x01 0x4e-->disassemble-->bgezalc $2, 1340.
> > 
> > So these FIXME's will always be true
> That's definitely a bug, they should be inverses of each other.
> 
> I've just looked at the encode function and I think it's wrong for the path this test will use. The immediate path is just 'X >> 2' but the symbol reference path is '(X - 4) >> 2'. Would you be willing to fix the immediate-encode path as part of this patch? That way we can drop these FIXME's.
It appears I'm mistaken on this but we think the current behaviour is weird/wrong in both GAS and IAS. In GAS, immediates are absolute addresses that are modified with +4 for unknown reasons. In IAS, immediates are currently offsets and are not modified.

Your change matches GAS's behaviour so: LGTM with the 'FIXME: ' comments changed to '# The encode/decode functions are not inverses of each other in the immediate case'


Repository:
  rL LLVM

http://reviews.llvm.org/D17540