[PATCH] ARM: Tighten up disassembling of MSR mask for M-class

[James Molloy]

Hi Petr,

Thanks for working on this. As far as the softfail stuff goes, we're not too strict on that and there are bigger problems to solve before UNPREDICTABLE all works correctly, so let's not worry about that for the moment.

I do have a few specific comments, below.

Cheers,

James

================
Comment at: lib/Target/ARM/Disassembler/ARMDisassembler.cpp:3999
@@ +3998,3 @@
+    case 19: // faultmask
+      if (!(FeatureBits & ARM::HasV7Ops)) {
+        // Values basepri, basepri_max and faultmask are only valid for v7m.
----------------
Don't need the braces here.

================
Comment at: lib/Target/ARM/Disassembler/ARMDisassembler.cpp:4009
@@ +4008,3 @@
+    // Validate the 2-bit mask value in MSR for v7m.
+    if ((FeatureBits & ARM::HasV7Ops) && Inst.getOpcode() == ARM::t2MSR_M) {
+      unsigned Mask = Val >> 10;
----------------
MSR only appears to have one encoding. What are you trying to achieve by checking the opcode here?

================
Comment at: lib/Target/ARM/Disassembler/ARMDisassembler.cpp:4010
@@ +4009,3 @@
+    if ((FeatureBits & ARM::HasV7Ops) && Inst.getOpcode() == ARM::t2MSR_M) {
+      unsigned Mask = Val >> 10;
+      if (Mask == 0 || (Mask != 2 && ValLow > 3))
----------------
Mask is two bits, so there should be a & 3 here.

================
Comment at: lib/Target/ARM/Disassembler/ARMDisassembler.cpp:4011
@@ +4010,3 @@
+      unsigned Mask = Val >> 10;
+      if (Mask == 0 || (Mask != 2 && ValLow > 3))
+        return MCDisassembler::Fail;
----------------
Several points here:
  * Please add more documentation as to what this condition is supposed to check. I think I've reverse engineered it from the ARMARM but I shouldn't have to.
  * The mask values 0b01 and 0b11 are predicated on the DSP extension in the ARMARM. I *believe* we do model this extension in LLVM, so you should check it here.

http://reviews.llvm.org/D4694