[LLVMdev] [ARM] Bugs in decode / encode of PC-relative t2 LDR in the ARM backend

[Gordon Keiser]

I've been working with the ARM backend's disassembler and working it out pretty heavily over the past several months, and have run into a bug involving the LDR instruction and all variants which use the T2I_ld multiclass.   Codegen doesn't seem to trigger this issue, because either the correct variant is always selected, or certain combinations of parameters never occur, I haven't been looking into that heavily yet. 

The basic issue is that an instruction such as LDR.W r0, [pc, #1] (positive immediate) will decode into t2LDRi12, whereas the same literal load with a negative immediate will randomly either fail to decode (test with 0x5f 0xf8 0x01 0x20 which should bd LDR.W r2, [pc, #-1]),  or decode to some predictable version of t2LDRT, t2LDRi8,  etc... based on  certain bits in the immediate matching the ones set in the corresponding instruction.   The 'U' bit of t2LDRpci is additionally set as "?" which seems to always end up being 0 as there is no encoder method to deal with it, so it will happily output a negative-immediate version no matter what the input value.  

The shifted immediate version of t2LDR has a custom decoder method which handles conversion to a t2LDRpci if Rn == PC, but the t2LDRs case is actually missing from this, only the various other types (B, H, SB, SH) are handled.  The ARM manual lists almost all of the LDR variants as "If Rn == pc, See LDR (literal)".

So far to work around the decoder issue I've made custom decoder functions for all variants in the T2I_ld multiclass, as well as LDRT to basically do what the "s" version decoder in the multiclass did and switch over to the literal version when Rn == PC.   This exposed the issue with the encoding of the opcode t2LDRpci from an MCInst, so I gave that variant a PostEncode function to handle sign conversion.   This ended up exposing some broken llvm-lit tests (verified via IDA Pro 6.3 and by hand with the ARM manuals) which I also have fixes for. 



Anyway, now that I've explained this, I'm wanting to know if there might be some better way that I missed when doing this.   It seems kind of cumbersome to add 4-5 custom decoder methods and I'm concerned about the post-encode method because this didn't seem to be causing codegen issues before, and I haven't had time to test this yet.   The cleaner solution would seem to have been to put GPRnopc register constraints on the tablegen stuff for the index in other variants, but these are ignored during decoding / disassembly. 

Is there anyone more familiar with Tablegen who might be able to point out a cleaner way of fixing this issue?  I don't have the patch files on my home computer but can send them tomorrow at work, I'd really appreciate advice on better ways of handling this before I submit something for review.  

Thanks,
Gordon Keiser
Software Development Engineer
Arxan Technologies
Protecting the App EconomyT