[ARM] Turn some undefined encoding bits into 0s.

[simon_tatham]

Summary

The family of 32-bit Thumb instruction encodings that include t2ORR,
t2AND and t2EOR are all listed in the ArmARM as having (0) in bit 15.
The Tablegen descriptions of those instructions listed them as ?. This
change tightens that up by making them into 0 + Unpredictable.

In the specific case of t2ORR, we tighten it up still further by
making the zero bit mandatory. This change comes from Arm v8.1-M, in
which encodings with that bit equal to 1 will now be used for
different instructions.

[dmgreen]

LGTM

[efriedma]

The use of "?" here doesn't seem right. All those instructions where you're adding a "?" do in fact require that the bit in question must be zero.

We should also be using the "Unpredictable" field on these instructions so that we soft-fail in the disassembler. This shouldn't conflict with the v8.1-M instructions, I think? If it does, you can comment it out and leave a FIXME.

Please make sure we have tests in test/MC/Disassembler/ for all these encodings.

[simon_tatham]

If I set the Unpredictable field on the ORR instruction in particular, then it clashes with CSEL. But I think you're right that all the other affected instructions can be changed from ? into 0 + Unpredictable.

Here's a revised version of the patch which does it that way, and also includes a test.

[efriedma]

llvm/lib/Target/ARM/ARMInstrThumb2.td

895 ↗

(On Diff #202410)

Can you use Unpredictable here?

1889 ↗

(On Diff #202410)

Can you use Unpredictable here?

[simon_tatham]

llvm/lib/Target/ARM/ARMInstrThumb2.td

895 ↗

(On Diff #202410)

No – this encoding is a subset of the ORR encoding, which is the one that v8.1-M has tightened up to make the 0 bit mandatory. If I turn this into a softfail, then the v8.1-M conditional select instructions added by D62667 will be mis-decoded as unpredictable variants of move or shift instructions.

1889 ↗

(On Diff #202410)

No – same reason as above.

[efriedma]

LGTM

llvm/lib/Target/ARM/ARMInstrThumb2.td

895 ↗

(On Diff #202410)

Oh, I didn't realize the encodings worked that way. Makes sense.

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

multiclass T2I_bin_irs<bits<4> opcod, string opc,
    def rr : T2sThreeReg<(outs rGPR:$Rd), (ins rGPR:$Rn, rGPR:$Rm), iir,
                  opc, !strconcat(wide, "\t$Rd, $Rn, $Rm"),
                  [(set rGPR:$Rd, (opnode rGPR:$Rn, rGPR:$Rm))]>,
                  Sched<[WriteALU, ReadALU, ReadALU]> {
      let isCommutable = Commutable;
      let Inst{31-27} = 0b11101;
      let Inst{26-25} = 0b01;
      let Inst{24-21} = opcod;
+     let Inst{15} = 0b0;
+     // In most of these instructions, and most versions of the Arm
+     // architecture, bit 15 of this encoding is listed as (0) rather
+     // than 0, i.e. setting it to 1 is UNPREDICTABLE or a soft-fail
+     // rather than a hard failure. In v8.1-M, this requirement is
+     // upgraded to a hard one for ORR, so that the encodings with 1
+     // in this bit can be reused for other instructions (such as
+     // CSEL). Setting Unpredictable{15} = 1 here would reintroduce
+     // that encoding clash in the auto- generated MC decoder, so I
+     // comment it out.
+     let Unpredictable{15} = !if(!eq(opcod, 0b0010), 0b0, 0b1);
      let Inst{14-12} = 0b000; // imm3
      let Inst{7-6} = 0b00; // imm2
      let Inst{5-4} = 0b00; // type
    }
    // shifted register
    def rs : T2sTwoRegShiftedReg<
                  (outs rGPR:$Rd), (ins rGPR:$Rn, t2_so_reg:$ShiftedRm), iis,
                  opc, !strconcat(wide, "\t$Rd, $Rn, $ShiftedRm"),
                  [(set rGPR:$Rd, (opnode rGPR:$Rn, t2_so_reg:$ShiftedRm))]>,
                  Sched<[WriteALUsi, ReadALU]>  {
      let Inst{31-27} = 0b11101;
      let Inst{26-25} = 0b01;
      let Inst{24-21} = opcod;
+     let Inst{15} = 0;
+     let Unpredictable{15} = !if(!eq(opcod, 0b0010), 0b0, 0b1); // see above
    }
   // Assembly aliases for optional destination operand when it's the same
   // as the source operand.
   def : t2InstAlias<!strconcat(opc, "${s}${p} $Rdn, $imm"),
      (!cast<Instruction>(NAME#"ri") rGPR:$Rdn, rGPR:$Rdn,
                                                     t2_so_imm:$imm, pred:$p,
                                                     cc_out:$s)>;
   def : t2InstAlias<!strconcat(opc, "${s}${p}", wide, " $Rdn, $Rm"),
multiclass T2I_sh_ir<bits<2> opcod, string opc, Operand ty, SDNode opnode> {
    def ri : T2sTwoRegShiftImm<
                  (outs rGPR:$Rd), (ins rGPR:$Rm, ty:$imm), IIC_iMOVsi,
                  opc, ".w\t$Rd, $Rm, $imm",
                  [(set rGPR:$Rd, (opnode rGPR:$Rm, (i32 ty:$imm)))]>,
                  Sched<[WriteALU]> {
      let Inst{31-27} = 0b11101;
      let Inst{26-21} = 0b010010;
      let Inst{19-16} = 0b1111; // Rn
+     let Inst{15}    = 0b0;
      let Inst{5-4} = opcod;
    }
    // register
    def rr : T2sThreeReg<
                  (outs rGPR:$Rd), (ins rGPR:$Rn, rGPR:$Rm), IIC_iMOVsr,
                  opc, ".w\t$Rd, $Rn, $Rm",
                  [(set rGPR:$Rd, (opnode rGPR:$Rn, rGPR:$Rm))]>,
                  Sched<[WriteALU]> {

 
 let hasSideEffects = 0 in
 def t2MOVr : T2sTwoReg<(outs GPRnopc:$Rd), (ins GPRnopc:$Rm), IIC_iMOVr,
                    "mov", ".w\t$Rd, $Rm", []>, Sched<[WriteALU]> {
   let Inst{31-27} = 0b11101;
   let Inst{26-25} = 0b01;
   let Inst{24-21} = 0b0010;
   let Inst{19-16} = 0b1111; // Rn
+  let Inst{15} = 0b0;
   let Inst{14-12} = 0b000;
   let Inst{7-4} = 0b0000;
 }
 def : t2InstAlias<"mov${p}.w $Rd, $Rm", (t2MOVr GPRnopc:$Rd, GPRnopc:$Rm,
                                                 pred:$p, zero_reg)>;
 def : t2InstAlias<"movs${p}.w $Rd, $Rm", (t2MOVr GPRnopc:$Rd, GPRnopc:$Rm,
                                                  pred:$p, CPSR)>;
 def : t2InstAlias<"movs${p} $Rd, $Rm", (t2MOVr GPRnopc:$Rd, GPRnopc:$Rm,

 let Uses = [CPSR] in {
 def t2RRX : T2sTwoReg<(outs rGPR:$Rd), (ins rGPR:$Rm), IIC_iMOVsi,
                    "rrx", "\t$Rd, $Rm",
                    [(set rGPR:$Rd, (ARMrrx rGPR:$Rm))]>, Sched<[WriteALU]> {
   let Inst{31-27} = 0b11101;
   let Inst{26-25} = 0b01;
   let Inst{24-21} = 0b0010;
   let Inst{19-16} = 0b1111; // Rn
+  let Inst{15} = 0b0;
+  let Unpredictable{15} = 0b1;
   let Inst{14-12} = 0b000;
   let Inst{7-4} = 0b0011;
 }
 }
 
 let isCodeGenOnly = 1, Defs = [CPSR] in {
 def t2MOVsrl_flag : T2TwoRegShiftImm<
                         (outs rGPR:$Rd), (ins rGPR:$Rm), IIC_iMOVsi,

llvm/trunk/test/MC/Disassembler/ARM/thumb2-bit-15.txt

+# RUN: not llvm-mc -triple=thumbv7 -mcpu=cortex-a8 -disassemble < %s 2> %t | FileCheck %s
+# RUN: FileCheck --check-prefix=ERROR < %t %s
+
+[0x09,0xea,0x08,0x04]
+# CHECK: and.w  r4, r9, r8
+
+[0x09,0xea,0x08,0x84]
+# CHECK: and.w  r4, r9, r8
+# ERROR: [[@LINE-2]]:2: warning: potentially undefined instruction encoding
+
+[0x04,0xea,0xe8,0x01]
+# CHECK: and.w  r1, r4, r8, asr #3
+
+[0x04,0xea,0xe8,0x81]
+# CHECK: and.w  r1, r4, r8, asr #3
+# ERROR: [[@LINE-2]]:2: warning: potentially undefined instruction encoding
+
+[0x11,0xea,0x47,0x02]
+# CHECK: ands.w r2, r1, r7, lsl #1
+
+[0x11,0xea,0x47,0x82]
+# CHECK: ands.w r2, r1, r7, lsl #1
+# ERROR: [[@LINE-2]]:2: warning: potentially undefined instruction encoding
+
+[0x45,0xea,0x06,0x04]
+# CHECK: orr.w  r4, r5, r6
+
+[0x45,0xea,0x06,0x84]
+# ERROR: [[@LINE-1]]:2: warning: invalid instruction encoding
+
+[0x45,0xea,0x46,0x14]
+# CHECK: orr.w  r4, r5, r6, lsl #5
+
+[0x45,0xea,0x46,0x94]
+# ERROR: [[@LINE-1]]:2: warning: invalid instruction encoding
+
+[0x55,0xea,0x56,0x14]
+# CHECK: orrs.w r4, r5, r6, lsr #5
+
+[0x55,0xea,0x56,0x94]
+# ERROR: [[@LINE-1]]:2: warning: invalid instruction encoding
+
+[0x85,0xea,0x06,0x04]
+# CHECK: eor.w  r4, r5, r6
+
+[0x85,0xea,0x06,0x84]
+# CHECK: eor.w  r4, r5, r6
+# ERROR: [[@LINE-2]]:2: warning: potentially undefined instruction encoding
+
+[0x85,0xea,0x46,0x14]
+# CHECK: eor.w  r4, r5, r6, lsl #5
+
+[0x85,0xea,0x46,0x94]
+# CHECK: eor.w  r4, r5, r6, lsl #5
+# ERROR: [[@LINE-2]]:2: warning: potentially undefined instruction encoding
+
+[0x4f,0xea,0x02,0x01]
+# CHECK: mov.w  r1, r2
+
+[0x4f,0xea,0x02,0x81]
+# ERROR: [[@LINE-1]]:2: warning: invalid instruction encoding
+
+[0x4f,0xea,0x02,0x46]
+# CHECK: lsl.w  r6, r2, #16
+
+[0x4f,0xea,0x02,0xc6]
+# ERROR: [[@LINE-1]]:2: warning: invalid instruction encoding
+
+[0x4f,0xea,0x12,0x46]
+# CHECK: lsr.w  r6, r2, #16
+
+[0x4f,0xea,0x12,0xc6]
+# ERROR: [[@LINE-1]]:2: warning: invalid instruction encoding
+
+[0x5f,0xea,0x22,0x06]
+# CHECK: asrs.w r6, r2, #32
+
+[0x5f,0xea,0x22,0x86]
+# ERROR: [[@LINE-1]]:2: warning: invalid instruction encoding
+
+[0x5f,0xea,0x72,0x16]
+# CHECK: rors.w r6, r2, #5
+
+[0x5f,0xea,0x72,0x96]
+# ERROR: [[@LINE-1]]:2: warning: invalid instruction encoding
+
+[0x4f,0xea,0x34,0x04]
+# CHECK: rrx  r4, r4
+
+[0x4f,0xea,0x34,0x84]
+# CHECK: rrx  r4, r4
+# ERROR: [[@LINE-2]]:2: warning: potentially undefined instruction encoding