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

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