[ARM] Fix Machine Outliner LDRD/STRD handling in Thumb mode

[yroux]

Summary

This is a fix for PR50481

Immediate values for AddrModeT2_i8s4 are already scaled in MCinst operand, change the number of bits
and scale factor to reflect that state when checking stack offset status. AddrModeT2_i7s[2|4] also have
this particularity but since MVE instructions are outlined, just move these cases to the unhandled ones.

[efriedma]

llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp

5987

Maybe assert "Fixup" is a multiple of 4?

[chill]

llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp

6001

Can't we instead return false; here and get rid of enumerating all unsupported addressing modes on line 5933?

[yroux]

llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp

5987

ok, and I'll change the check line 6005 into an assert

6001

hmm, yes I agree it'll make it easier to read

[yroux]

llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp

6001

The issue is that getting the offset value line 5948 can only be done if the addressing mode has one, so we can't get rid of the checking line 5933. I already planned to rework this part and extract it since that kind of logic is used in multiple places in the backend, but what about keeping it as it for this bugfix ?

[chill]

llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp

6001

Agree, it's a separate concern.

[yroux]

@efriedma, @chill is this patch ok for you ? (it'd be great to have the issue fixed into 12.0.1 release)

[chill]

In D103167#2802486, @yroux wrote:

@efriedma, @chill is this patch ok for you ? (it'd be great to have the issue fixed into 12.0.1 release)

LGTM, just if you intend to reuse the summary as a commit message "... since MVE instructions are *not* outlined...".

[yroux]

In D103167#2802543, @chill wrote:

LGTM, just if you intend to reuse the summary as a commit message "... since MVE instructions are *not* outlined...".

OK will do, thanks

llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp

bool ARMBaseInstrInfo::checkAndUpdateStackOffset(MachineInstr *MI,
   // Stack might be involved but addressing mode doesn't handle any offset.
   // Rq: AddrModeT1_[1|2|4] don't operate on SP
   if (AddrMode == ARMII::AddrMode1        // Arithmetic instructions
       || AddrMode == ARMII::AddrMode4     // Load/Store Multiple
       || AddrMode == ARMII::AddrMode6     // Neon Load/Store Multiple
       || AddrMode == ARMII::AddrModeT2_so // SP can't be used as based register
       || AddrMode == ARMII::AddrModeT2_pc // PCrel access
       || AddrMode == ARMII::AddrMode2     // Used by PRE and POST indexed LD/ST
+      || AddrMode == ARMII::AddrModeT2_i7 // v8.1-M MVE
+      || AddrMode == ARMII::AddrModeT2_i7s2 // v8.1-M MVE
+      || AddrMode == ARMII::AddrModeT2_i7s4 // v8.1-M sys regs VLDR/VSTR
       || AddrMode == ARMII::AddrModeNone)
     return false;
 
   unsigned NumOps = MI->getDesc().getNumOperands();
   unsigned ImmIdx = NumOps - 3;
 
   const MachineOperand &Offset = MI->getOperand(ImmIdx);
   assert(Offset.isImm() && "Is not an immediate");
case ARMII::AddrMode5FP16:
     OffVal = ARM_AM::getAM5FP16Offset(OffVal);
     NumBits = 8;
     Scale = 2;
     break;
   case ARMII::AddrModeT2_i8:
     NumBits = 8;
     break;
   case ARMII::AddrModeT2_i8s4:
+    // FIXME: Values are already scaled in this addressing mode.
+    assert((Fixup & 3) == 0 && "Can't encode this offset!");
+    NumBits = 10;
+    break;
   case ARMII::AddrModeT2_ldrex:
     NumBits = 8;
     Scale = 4;
     break;
   case ARMII::AddrModeT2_i12:
   case ARMII::AddrMode_i12:
     NumBits = 12;
     break;
-  case ARMII::AddrModeT2_i7:
-    NumBits = 7;
-    break;
-  case ARMII::AddrModeT2_i7s2:
-    NumBits = 7;
-    Scale = 2;
-    break;
-  case ARMII::AddrModeT2_i7s4:
-    NumBits = 7;
-    Scale = 4;
-    break;
   case ARMII::AddrModeT1_s: // SP-relative LD/ST
     NumBits = 8;
     Scale = 4;
     break;
   default:
     llvm_unreachable("Unsupported addressing mode!");
   }
   // Make sure the offset is encodable for instructions that scale the
   // immediate.
-  if (((OffVal * Scale + Fixup) & (Scale - 1)) != 0)
-    return false;
+  assert(((OffVal * Scale + Fixup) & (Scale - 1)) == 0 &&
+         "Can't encode this offset!");
   OffVal += Fixup / Scale;
 
   unsigned Mask = (1 << NumBits) - 1;
 
   if (OffVal <= Mask) {
     if (Updt)
       MI->getOperand(ImmIdx).setImm(OffVal);
     return true;

llvm/test/CodeGen/ARM/machine-outliner-stack-fixup-thumb.mir

 tracksRegLiveness: true
 
 body:             |
   bb.0:
     liveins: $r1
     ;CHECK-LABEL: name:           CheckAddrModeT2_i8s4
     ;CHECK: $r0 = tMOVr $r1, 14 /* CC::al */, $noreg
     ;CHECK-NEXT: tBL 14 /* CC::al */, $noreg, @OUTLINED_FUNCTION_[[I8S4:[0-9]+]]
-    ;CHECK-NEXT: t2STRDi8 $r0, $r1, $sp, 254, 14 /* CC::al */, $noreg
+    ;CHECK-NEXT: t2STRDi8 $r0, $r1, $sp, 1020, 14 /* CC::al */, $noreg
     $r0 = tMOVr $r1, 14, $noreg
     tBL 14, $noreg, @foo, implicit-def dead $lr, implicit $sp
     t2STRDi8 $r0, $r1, $sp, 0, 14, $noreg
     t2STRDi8 $r0, $r1, $sp, 8, 14, $noreg
-    t2STRDi8 $r0, $r1, $sp, 253, 14, $noreg
-    t2STRDi8 $r0, $r1, $sp, 254, 14, $noreg
+    t2STRDi8 $r0, $r1, $sp, 1012, 14, $noreg
+    t2STRDi8 $r0, $r1, $sp, 1020, 14, $noreg
     tBL 14, $noreg, @foo, implicit-def dead $lr, implicit $sp
     t2STRDi8 $r0, $r1, $sp, 0, 14, $noreg
     t2STRDi8 $r0, $r1, $sp, 8, 14, $noreg
-    t2STRDi8 $r0, $r1, $sp, 253, 14, $noreg
-    t2STRDi8 $r0, $r1, $sp, 254, 14, $noreg
+    t2STRDi8 $r0, $r1, $sp, 1012, 14, $noreg
+    t2STRDi8 $r0, $r1, $sp, 1020, 14, $noreg
     tBL 14, $noreg, @foo, implicit-def dead $lr, implicit $sp
     t2STRDi8 $r0, $r1, $sp, 0, 14, $noreg
     t2STRDi8 $r0, $r1, $sp, 8, 14, $noreg
-    t2STRDi8 $r0, $r1, $sp, 253, 14, $noreg
-    t2STRDi8 $r0, $r1, $sp, 254, 14, $noreg
+    t2STRDi8 $r0, $r1, $sp, 1012, 14, $noreg
+    t2STRDi8 $r0, $r1, $sp, 1020, 14, $noreg
     BX_RET 14, $noreg
 ...
 ---
 
 name:           CheckAddrModeT2_ldrex
 tracksRegLiveness: true
 
 body:             |
bb.0:
     ;CHECK-NEXT: t2STRHi8 $r0, $sp, 255, 14 /* CC::al */, $noreg
     ;CHECK-NEXT: $lr, $sp = t2LDR_POST $sp, 8, 14 /* CC::al */, $noreg
 
     ;CHECK: name:           OUTLINED_FUNCTION_[[I8S4]]
     ;CHECK: early-clobber $sp = t2STR_PRE killed $lr, $sp, -8, 14 /* CC::al */, $noreg
     ;CHECK-NEXT: frame-setup CFI_INSTRUCTION def_cfa_offset 8
     ;CHECK-NEXT: frame-setup CFI_INSTRUCTION offset $lr, -8
     ;CHECK-NEXT: tBL 14 /* CC::al */, $noreg, @foo, implicit-def dead $lr, implicit $sp
-    ;CHECK-NEXT: t2STRDi8 $r0, $r1, $sp, 2, 14 /* CC::al */, $noreg
-    ;CHECK-NEXT: t2STRDi8 $r0, $r1, $sp, 10, 14 /* CC::al */, $noreg
-    ;CHECK-NEXT: t2STRDi8 $r0, $r1, $sp, 255, 14 /* CC::al */, $noreg
+    ;CHECK-NEXT: t2STRDi8 $r0, $r1, $sp, 8, 14 /* CC::al */, $noreg
+    ;CHECK-NEXT: t2STRDi8 $r0, $r1, $sp, 16, 14 /* CC::al */, $noreg
+    ;CHECK-NEXT: t2STRDi8 $r0, $r1, $sp, 1020, 14 /* CC::al */, $noreg
     ;CHECK-NEXT: $lr, $sp = t2LDR_POST $sp, 8, 14 /* CC::al */, $noreg
 
     ;CHECK: name:           OUTLINED_FUNCTION_[[I12]]
     ;CHECK: early-clobber $sp = t2STR_PRE killed $lr, $sp, -8, 14 /* CC::al */, $noreg
     ;CHECK-NEXT: frame-setup CFI_INSTRUCTION def_cfa_offset 8
     ;CHECK-NEXT: frame-setup CFI_INSTRUCTION offset $lr, -8
     ;CHECK-NEXT: tBL 14 /* CC::al */, $noreg, @foo, implicit-def dead $lr, implicit $sp
     ;CHECK-NEXT: $r0 = t2LDRi12 $sp, 8, 14 /* CC::al */, $noreg