[ARM] Verify addressing immediates

[dmgreen]

Summary

This adds at extra check into ARMBaseInstrInfo::verifyInstruction to verify the offsets used in addressing mode immediates using isLegalAddressImm. Some tests needed fixing up as a result, including using t2STRHT instead of t2STRHi8 to test some outliner offsets, and adjusting the opcode created from CMSE stack adjustments.

[samtebbs]

Is this the kind of constraint that can be baked into the operand type in the instruction's (ins ...) in tablegen?

[dmgreen]

In D114939#3183101, @samtebbs wrote:

Is this the kind of constraint that can be baked into the operand type in the instruction's (ins ...) in tablegen?

Yeah it already is, to a certain degree. The instructions have an AddrMode set in the AM variable, such as set by this MVE instruction: https://github.com/llvm/llvm-project/blob/8dcaf3aa0bf25508700a7452ed963c1487221dfd/llvm/lib/Target/ARM/ARMInstrMVE.td#L6222 (it's a bit hidden there, as it goes through the MVE_memsz. This t2LDRHi8 example is more clear, where it sets AM=AddrModeT2_i8neg https://github.com/llvm/llvm-project/blob/8dcaf3aa0bf25508700a7452ed963c1487221dfd/llvm/lib/Target/ARM/ARMInstrFormats.td#L1402.

That is what we are using here, along with some code already present in the load-store optimizer to validate the range of the immediates in the instruction.

[samtebbs]

I'm wondering if this verification could be done for us automatically, because the tablegen operands presumably already check what the immediate is before it's allowed to become a t2_addrmode_i7 etc. If that isn't possible then this LGTM.

llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp

bool ARMBaseInstrInfo::verifyInstruction(const MachineInstr &MI,
   if (MI.getOpcode() == ARM::MVE_VMOV_q_rr) {
     assert(MI.getOperand(4).isImm() && MI.getOperand(5).isImm());
     if ((MI.getOperand(4).getImm() != 2 && MI.getOperand(4).getImm() != 3) ||
         MI.getOperand(4).getImm() != MI.getOperand(5).getImm() + 2) {
       ErrInfo = "Incorrect array index for MVE_VMOV_q_rr";
       return false;
     }
   }
+
+  // Check the address model by taking the first Imm operand and checking it is
+  // legal for that addressing mode.
+  ARMII::AddrMode AddrMode =
+      (ARMII::AddrMode)(MI.getDesc().TSFlags & ARMII::AddrModeMask);
+  switch (AddrMode) {
+  default:
+    break;
+  case ARMII::AddrModeT2_i7:
+  case ARMII::AddrModeT2_i7s2:
+  case ARMII::AddrModeT2_i7s4:
+  case ARMII::AddrModeT2_i8:
+  case ARMII::AddrModeT2_i8pos:
+  case ARMII::AddrModeT2_i8neg:
+  case ARMII::AddrModeT2_i8s4:
+  case ARMII::AddrModeT2_i12: {
+    uint32_t Imm = 0;
+    for (auto Op : MI.operands()) {
+      if (Op.isImm()) {
+        Imm = Op.getImm();
+        break;
+      }
+    }
+    if (!isLegalAddressImm(MI.getOpcode(), Imm, this)) {
+      ErrInfo = "Incorrect AddrMode Imm for instruction";
+      return false;
+    }
+    break;
+  }
+  }
   return true;
 }
 
 void ARMBaseInstrInfo::expandLoadStackGuardBase(MachineBasicBlock::iterator MI,
                                                 unsigned LoadImmOpc,
                                                 unsigned LoadOpc) const {
   assert(!Subtarget.isROPI() && !Subtarget.isRWPI() &&
          "ROPI/RWPI not currently supported with stack guard");

llvm/lib/Target/ARM/ARMExpandPseudoInsts.cpp

if (ARM::DPR_VFP2RegClass.contains(Reg)) {
           .addReg(ARM::SP)
           .addImm(Reg - ARM::S0)
           .add(predOps(ARMCC::AL));
     }
   }
   // restore FPSCR from stack and clear bits 0-4, 7, 28-31
   // The other bits are program global according to the AAPCS
   if (passesFPReg) {
-    BuildMI(MBB, MBBI, DL, TII->get(ARM::t2LDRi8), SpareReg)
+    BuildMI(MBB, MBBI, DL, TII->get(ARM::tLDRspi), SpareReg)
         .addReg(ARM::SP)
-        .addImm(0x40)
+        .addImm(0x10)
         .add(predOps(ARMCC::AL));
     BuildMI(MBB, MBBI, DL, TII->get(ARM::t2BICri), SpareReg)
         .addReg(SpareReg)
         .addImm(0x0000009F)
         .add(predOps(ARMCC::AL))
         .add(condCodeOp());
     BuildMI(MBB, MBBI, DL, TII->get(ARM::t2BICri), SpareReg)
         .addReg(SpareReg)