[ARM] Tighten restrictions on use of SP in v8.1-M CSEL.

[simon_tatham]

Summary

In the CSEL Rd,Rm,Rn instruction family (also including CSINC, CSINV
and CSNEG), the architecture lists it as CONSTRAINED UNPREDICTABLE
(i.e. SoftFail) to use SP in the Rd or Rm slot, but outright illegal
to use it in the Rn slot, not least because some encodings of that
form are used by MVE instructions such as UQRSHLL.

MC was treating all three slots the same, as SoftFail. So the only
reason UQRSHLL was disassembled correctly at all was because the MVE
decode table is separate from the Thumb2 one and takes priority; if
you turned off MVE, then encodings such as [0x5f,0xea,0x0d,0x83]
would disassemble as spurious CSELs.

Fixed by inventing another version of the GPRwithZR register class,
which disallows SP completely instead of just SoftFailing it.

[DavidSpickett]

llvm/test/MC/Disassembler/ARM/thumbv8.1m.s

1 ↗

(On Diff #206804)

How did this work to begin with?

[simon_tatham]

llvm/test/MC/Disassembler/ARM/thumbv8.1m.s

1 ↗

(On Diff #206804)

No test in this file previously generated the message "warning: invalid instruction encoding". Two of the new ones do.

Apparently that message, representing MCDisassembler::Fail as opposed to SoftFail, causes the exit status of llvm-mc to become failure, in spite of the word "warning" on the front!

llvm/trunk/lib/Target/ARM/ARMInstrThumb2.td

 let isBranch = 1, isTerminator = 1, hasSideEffects = 1 in
 def t2LoopEnd :
   t2PseudoInst<(outs), (ins GPRlr:$elts, brtarget:$target),
   8, IIC_Br, []>, Sched<[WriteBr]>;
 
 } // end isNotDuplicable
 
 class CS<string iname, bits<4> opcode, list<dag> pattern=[]>
-  : V8_1MI<(outs rGPR:$Rd), (ins GPRwithZR:$Rn, GPRwithZR:$Rm, pred_noal:$fcond),
+  : V8_1MI<(outs rGPR:$Rd), (ins GPRwithZR:$Rn, GPRwithZRnosp:$Rm, pred_noal:$fcond),
            AddrModeNone, NoItinerary, iname, "$Rd, $Rn, $Rm, $fcond", "", pattern> {
   bits<4> Rd;
   bits<4> Rm;
   bits<4> Rn;
   bits<4> fcond;
 
   let Inst{31-20} = 0b111010100101;
   let Inst{19-16} = Rn{3-0};

 let Predicates = [HasV8_1MMainline] in {
   def : InstAlias<"csetm\t$Rd, $fcond",
                  (t2CSINV rGPR:$Rd, ZR, ZR, pred_noal_inv:$fcond)>;
 
   def : InstAlias<"cset\t$Rd, $fcond",
                  (t2CSINC rGPR:$Rd, ZR, ZR, pred_noal_inv:$fcond)>;
 
   def : InstAlias<"cinc\t$Rd, $Rn, $fcond",
-                 (t2CSINC rGPR:$Rd, GPRwithZR:$Rn, GPRwithZR:$Rn, pred_noal_inv:$fcond)>;
+                 (t2CSINC rGPR:$Rd, GPRwithZRnosp:$Rn, GPRwithZRnosp:$Rn, pred_noal_inv:$fcond)>;
 
   def : InstAlias<"cinv\t$Rd, $Rn, $fcond",
-                 (t2CSINV rGPR:$Rd, GPRwithZR:$Rn, GPRwithZR:$Rn, pred_noal_inv:$fcond)>;
+                 (t2CSINV rGPR:$Rd, GPRwithZRnosp:$Rn, GPRwithZRnosp:$Rn, pred_noal_inv:$fcond)>;
 
   def : InstAlias<"cneg\t$Rd, $Rn, $fcond",
-                 (t2CSNEG rGPR:$Rd, GPRwithZR:$Rn, GPRwithZR:$Rn, pred_noal_inv:$fcond)>;
+                 (t2CSNEG rGPR:$Rd, GPRwithZRnosp:$Rn, GPRwithZRnosp:$Rn, pred_noal_inv:$fcond)>;
 }

llvm/trunk/lib/Target/ARM/ARMRegisterInfo.td

def GPRwithAPSR : RegisterClass<"ARM", [i32], 32, (add (sub GPR, PC), APSR_NZCV)> {
   let DiagnosticString = "operand must be a register in range [r0, r14] or apsr_nzcv";
 }
 
 // GPRs without the PC and SP registers but with APSR. Used by CLRM instruction.
 def GPRwithAPSRnosp : RegisterClass<"ARM", [i32], 32, (add (sequence "R%u", 0, 12), LR, APSR)> {
   let isAllocatable = 0;
 }
 
-def GPRwithZR : RegisterClass<"ARM", [i32], 32, (add (sequence "R%u", 0, 12),
-                                              LR, ZR)> {
-
+def GPRwithZR : RegisterClass<"ARM", [i32], 32, (add (sub GPR, PC), ZR)> {
   let AltOrders = [(add LR, GPRwithZR), (trunc GPRwithZR, 8)];
   let AltOrderSelect = [{
       return 1 + MF.getSubtarget<ARMSubtarget>().isThumb1Only();
   }];
+  let DiagnosticString = "operand must be a register in range [r0, r14] or zr";
+}
+
+def GPRwithZRnosp : RegisterClass<"ARM", [i32], 32, (sub GPRwithZR, SP)> {
+  let AltOrders = [(add LR, GPRwithZRnosp), (trunc GPRwithZRnosp, 8)];
+  let AltOrderSelect = [{
+      return 1 + MF.getSubtarget<ARMSubtarget>().isThumb1Only();
+  }];
   let DiagnosticString = "operand must be a register in range [r0, r12] or r14 or zr";
 }
 
 // GPRsp - Only the SP is legal. Used by Thumb1 instructions that want the
 // implied SP argument list.
 // FIXME: It would be better to not use this at all and refactor the
 // instructions to not have SP an an explicit argument. That makes
 // frame index resolution a bit trickier, though.

llvm/trunk/lib/Target/ARM/Disassembler/ARMDisassembler.cpp

static DecodeStatus DecodeGPRnopcRegisterClass(MCInst &Inst,
                                                unsigned RegNo, uint64_t Address,
                                                const void *Decoder);
 static DecodeStatus DecodeGPRwithAPSRRegisterClass(MCInst &Inst,
                                                unsigned RegNo, uint64_t Address,
                                                const void *Decoder);
 static DecodeStatus DecodeGPRwithZRRegisterClass(MCInst &Inst,
                                                unsigned RegNo, uint64_t Address,
                                                const void *Decoder);
+static DecodeStatus DecodeGPRwithZRnospRegisterClass(
+    MCInst &Inst, unsigned RegNo, uint64_t Address, const void *Decoder);
 static DecodeStatus DecodetGPRRegisterClass(MCInst &Inst, unsigned RegNo,
                                    uint64_t Address, const void *Decoder);
 static DecodeStatus DecodetcGPRRegisterClass(MCInst &Inst, unsigned RegNo,
                                    uint64_t Address, const void *Decoder);
 static DecodeStatus DecoderGPRRegisterClass(MCInst &Inst, unsigned RegNo,
                                    uint64_t Address, const void *Decoder);
 static DecodeStatus DecodeGPRPairRegisterClass(MCInst &Inst, unsigned RegNo,
                                    uint64_t Address, const void *Decoder);
DecodeGPRwithZRRegisterClass(MCInst &Inst, unsigned RegNo,
 
   if (RegNo == 15)
   {
     Inst.addOperand(MCOperand::createReg(ARM::ZR));
     return MCDisassembler::Success;
   }
 
   if (RegNo == 13)
-    S = MCDisassembler::SoftFail;
+    Check(S, MCDisassembler::SoftFail);
 
   Check(S, DecodeGPRRegisterClass(Inst, RegNo, Address, Decoder));
   return S;
 }
 
+static DecodeStatus
+DecodeGPRwithZRnospRegisterClass(MCInst &Inst, unsigned RegNo,
+                                 uint64_t Address, const void *Decoder) {
+  DecodeStatus S = MCDisassembler::Success;
+  if (RegNo == 13)
+    return MCDisassembler::Fail;
+  Check(S, DecodeGPRwithZRRegisterClass(Inst, RegNo, Address, Decoder));
+  return S;
+}
+
 static DecodeStatus DecodetGPRRegisterClass(MCInst &Inst, unsigned RegNo,
                                    uint64_t Address, const void *Decoder) {
   if (RegNo > 7)
     return MCDisassembler::Fail;
   return DecodeGPRRegisterClass(Inst, RegNo, Address, Decoder);
 }
 
 static const uint16_t GPRPairDecoderTable[] = {

llvm/trunk/test/MC/Disassembler/ARM/mve-scalar-shift.txt

 # CHECK-NOMVE: [[@LINE-2]]:2: warning: potentially undefined instruction encoding
 
 [0x5e 0xea 0x1f 0x31]
 # CHECK: lsrl    lr, r1, #12  @ encoding: [0x5e,0xea,0x1f,0x31]
 # CHECK-NOMVE: [[@LINE-2]]:2: warning: potentially undefined instruction encoding
 
 [0x5f 0xea 0x2d 0x83]
 # CHECK: sqrshrl lr, r3, r8  @ encoding: [0x5f,0xea,0x2d,0x83]
-# CHECK-NOMVE: [[@LINE-2]]:2: warning: potentially undefined instruction encoding
+# CHECK-NOMVE: [[@LINE-2]]:2: warning: invalid instruction encoding
 
 [0x5e 0xea 0x7f 0x4f]
 # CHECK: sqshl   lr, #17  @ encoding: [0x5e,0xea,0x7f,0x4f]
 # CHECK-NOMVE: [[@LINE-2]]:2: warning: potentially undefined instruction encoding
 
 [0x5f 0xea 0x3f 0x7b]
 # CHECK: sqshll  lr, r11, #28  @ encoding: [0x5f,0xea,0x3f,0x7b]
 # CHECK-NOMVE: [[@LINE-2]]:2: warning: potentially undefined instruction encoding

llvm/trunk/test/MC/Disassembler/ARM/thumbv8.1m.s

-# RUN: llvm-mc -disassemble -triple=thumbv8.1m.main-none-eabi -show-encoding %s 2> %t | FileCheck %s
+# RUN: not llvm-mc -disassemble -triple=thumbv8.1m.main-none-eabi -show-encoding %s 2> %t | FileCheck %s
 # RUN: FileCheck --check-prefix=ERROR < %t %s
 
 [0x52 0xea 0x22 0x9e]
 # CHECK: cinc lr, r2, lo  @ encoding: [0x52,0xea,0x22,0x9e]
 
 [0x57 0xea 0x47 0x9e]
 # CHECK: cinc lr, r7, pl  @ encoding: [0x57,0xea,0x47,0x9e]
 

 # CHECK: cinv lr, r2, pl  @ encoding: [0x52,0xea,0x42,0xae]
 
 [0x51 0xea 0x7b 0xbe]
 # CHECK: csneg lr, r1, r11, vc  @ encoding: [0x51,0xea,0x7b,0xbe]
 
 [0x50,0xea,0x01,0x80]
 # CHECK: csel r0, r0, r1, eq @ encoding: [0x50,0xea,0x01,0x80]
 
+[0x51,0xea,0x02,0x8d]
+# CHECK: csel sp, r1, r2, eq @ encoding: [0x51,0xea,0x02,0x8d]
+# ERROR: [[@LINE-2]]:2: warning: potentially undefined instruction encoding
+
+[0x5d,0xea,0x02,0x80]
+# CHECK: csel r0, sp, r2, eq @ encoding: [0x5d,0xea,0x02,0x80]
+# ERROR: [[@LINE-2]]:2: warning: potentially undefined instruction encoding
+
+[0x51,0xea,0x0d,0x80]
+# ERROR: [[@LINE-1]]:2: warning: invalid instruction encoding
+
+[0x5f,0xea,0x0d,0x83]
+# ERROR: [[@LINE-1]]:2: warning: invalid instruction encoding
+
 [0x5d 0xea 0x22 0x9e]
 # ERROR: [[@LINE-1]]:2: warning: potentially undefined instruction encoding
 
 [0x5d 0xea 0x47 0x9e]
 # ERROR: [[@LINE-1]]:2: warning: potentially undefined instruction encoding
 
 [0x5d 0xea 0x3c 0xae]
 # ERROR: [[@LINE-1]]:2: warning: potentially undefined instruction encoding