[PATCH] ARM: fix pc-relative thumb loads decoding (literal loads)

[Amaury de la Vieuville]

  OK to commit?


================
Comment at: lib/Target/ARM/Disassembler/ARMDisassembler.cpp:3208
@@ -3208,1 +3207,3 @@
+        break;
+      case ARM::t2PLDs: {
         Inst.setOpcode(ARM::t2PLDi12);
----------------
Renato Golin wrote:
> It seems weird to lower PLDs here and all other instructions on DecodeT2LoadLabel(). Maybe that's what your FIXME is all about?
> 
> Unless I'm mistaken, it doesn't seem particularly complicated to implement it there right now, and avoid introducing another FIXME. But if not, I'm fine with leaving it there, maybe creating a bug to fix it?
Regarding PLD, what was here before was kept in order to keep the regression tests happy. This is not ideal and surely there are other issues, I will work on this very shortly.

As for the other instructions, they have to be decoded that way, this is due to ambiguities in their encodings the generated decoder is not able to handle.

================
Comment at: lib/Target/ARM/InstPrinter/ARMInstPrinter.cpp:339
@@ +338,3 @@
+      << markup(">");
+  }
+  O << "]" << markup(">");
----------------
Renato Golin wrote:
> I would have kept only the formatImm() inside the if/else block. Or even inverted its sign before the output, so to have only one.
> 
> if (OffImm == INT32_MIN)
>   OffImm = 0;
> else if (isSub)
>   OffImm = -OffImm;
> 
> O << ...
Agreed, but this is the current "idiom" in the printer. I will take care of those in a patch along with this one.

================
Comment at: lib/Target/ARM/Disassembler/ARMDisassembler.cpp:3372
@@ +3371,3 @@
+    Inst.setOpcode(ARM::t2PLDi12);
+    Inst.addOperand(MCOperand::CreateReg(ARM::PC));
+  } else {
----------------
Renato Golin wrote:
> Re-writing my comment: shouldn't you make sure that you have the correct number of operands here, before appending a new one?
I'm not sure I understand what you mean, why would I need to check the number of operands?
Anyway, PLD decoding is a work in progress!


http://llvm-reviews.chandlerc.com/D973