[ARM] generate correct code for armv6-m XO big stack operations

[stuij]

Summary

The ARM backend codebase is dotted with places where armv6-m will generate
constant pools. Now that we can generate execute-only code for armv6-m, we need
to make sure we use the movs/lsls/adds/lsls/adds/lsls/adds pattern instead of
these.

Big stacks is one of the obvious places. In this patch we take care of two
sites:

1. take care of big stacks in prologue/epilogue
2. take care of save/tSTRspi nodes, which implicitly fixes emitThumbRegPlusImmInReg which is used in several frame lowering fns

[efriedma]

llvm/lib/Target/ARM/ARMAsmPrinter.cpp

1361

I'd prefer to actually verify the tLSLri has the expected shift amount/source/destination.

llvm/test/CodeGen/ARM/large-stack.ll

24

The generated code here is ridiculously inefficient; do you have some plan for generating code that's closer to optimal?

[stuij]

address review comment

[stuij]

llvm/test/CodeGen/ARM/large-stack.ll

24

Luckily armv6-M XO is a partner-requested feature. The current timeline is that we will first make it functionally work (which seems reasonable to me). Then we will benchmark and compare it with XO turned off. We will discuss the result and will optimise where needed.

[efriedma]

LGTM with a couple minor comments

llvm/lib/Target/ARM/ARMAsmPrinter.cpp

1145

For the instructions where it's relevant, can you add an assertion that the source and destination registers are equal?

1360

+=?

[stuij]

address review comments

[stuij]

llvm/lib/Target/ARM/ARMAsmPrinter.cpp

1360

yea, I went back and forth on this! Using addition which is analogous but won't represent what the processor will actually do felt more wrong than expressing what we expect it to do. But really I'm fine either way. Changed it.

[efriedma]

llvm/lib/Target/ARM/ARMAsmPrinter.cpp

1360

My primary concern with my review comments is making sure if someone messes with tMOVi32imm expansion, this code with either continue to work correctly, or assert that it's seeing something unexpected.

[stuij]

This came back with buildbot failures as the C in FileCheck wasn't uppercased. To my surprise I figured out that on MacOS HFS+ is often configured to be case-agnostic.

I pushed a fix.

llvm/lib/Target/ARM/ARMAsmPrinter.cpp

void ARMAsmPrinter::EmitUnwindingInstruction(const MachineInstr *MI) {
   switch (Opc) {
   case ARM::tPUSH:
     // special case: tPUSH does not have src/dst regs.
     SrcReg = DstReg = ARM::SP;
     break;
   case ARM::tLDRpci:
   case ARM::t2MOVi16:
   case ARM::t2MOVTi16:
+  case ARM::tMOVi8:
+  case ARM::tADDi8:
+  case ARM::tLSLri:
     // special cases:
     // 1) for Thumb1 code we sometimes materialize the constant via constpool
     //    load.
-    // 2) for Thumb2 execute only code we materialize the constant via
+    // 2) for Thumb1 execute only code we materialize the constant via the
+    // following pattern:
+    //        movs    r3, #:upper8_15:<const>
+    //        lsls    r3, #8
+    //        adds    r3, #:upper0_7:<const>
+    //        lsls    r3, #8
+    //        adds    r3, #:lower8_15:<const>
+    //        lsls    r3, #8
+    //        adds    r3, #:lower0_7:<const>
+    //    So we need to special-case MOVS, ADDS and LSLS, and keep track of
+    //    where we are in the sequence with the simplest of state machines.
+    // 3) for Thumb2 execute only code we materialize the constant via
     //    immediate constants in 2 separate instructions (MOVW/MOVT).
     SrcReg = ~0U;
     DstReg = MI->getOperand(0).getReg();
     break;
   default:
     SrcReg = MI->getOperand(1).getReg();
     DstReg = MI->getOperand(0).getReg();
     break;
if (SrcReg == ARM::SP) {
       case ARM::t2MOVi16:
         Offset = MI->getOperand(1).getImm();
         AFI->EHPrologueOffsetInRegs[DstReg] = Offset;
         break;
       case ARM::t2MOVTi16:
         Offset = MI->getOperand(2).getImm();
         AFI->EHPrologueOffsetInRegs[DstReg] |= (Offset << 16);
         break;
+      case ARM::tMOVi8:
+        Offset = MI->getOperand(2).getImm();
+        AFI->EHPrologueOffsetInRegs[DstReg] = Offset;
+        break;
+      case ARM::tADDi8:
+        Offset = MI->getOperand(3).getImm();
+        AFI->EHPrologueOffsetInRegs[DstReg] =
+            (AFI->EHPrologueOffsetInRegs[DstReg] << 8) | Offset;
+        break;
+      case ARM::tLSLri:
+        break;
       case ARM::t2PAC:
       case ARM::t2PACBTI:
         AFI->EHPrologueRemappedRegs[ARM::R12] = ARM::RA_AUTH_CODE;
         break;
       default:
         MI->print(errs());
         llvm_unreachable("Unsupported opcode for unwinding information");
       }

llvm/lib/Target/ARM/Thumb1FrameLowering.cpp

if (std::abs(NumBytes) > 508 * 3) {
     // emitThumbRegPlusImmediate so we don't have to deal with register
     // scavenging. (Scavenging could try to use the emergency spill slot
     // before we've actually finished setting up the stack.)
     if (ScratchReg == ARM::NoRegister)
       report_fatal_error("Failed to emit Thumb1 stack adjustment");
     MachineFunction &MF = *MBB.getParent();
     const ARMSubtarget &ST = MF.getSubtarget<ARMSubtarget>();
     if (ST.genExecuteOnly()) {
-      BuildMI(MBB, MBBI, dl, TII.get(ARM::t2MOVi32imm), ScratchReg)
+      unsigned XOInstr = ST.useMovt() ? ARM::t2MOVi32imm : ARM::tMOVi32imm;
+      BuildMI(MBB, MBBI, dl, TII.get(XOInstr), ScratchReg)
           .addImm(NumBytes).setMIFlags(MIFlags);
     } else {
       MRI.emitLoadConstPool(MBB, MBBI, dl, ScratchReg, 0, NumBytes, ARMCC::AL,
                             0, MIFlags);
     }
     BuildMI(MBB, MBBI, dl, TII.get(ARM::tADDhirr), ARM::SP)
         .addReg(ARM::SP)
         .addReg(ScratchReg, RegState::Kill)
         .add(predOps(ARMCC::AL))

llvm/lib/Target/ARM/ThumbRegisterInfo.cpp

assert((isARMLowRegister(DestReg) || DestReg.isVirtual()) &&
            "Thumb1 does not have ldr to high register");
     return emitThumb1LoadConstPool(MBB, MBBI, dl, DestReg, SubIdx, Val, Pred,
                                    PredReg, MIFlags);
   }
   return emitThumb2LoadConstPool(MBB, MBBI, dl, DestReg, SubIdx, Val, Pred,
                                  PredReg, MIFlags);
 }
 
-/// emitThumbRegPlusImmInReg - Emits a series of instructions to materialize
-/// a destreg = basereg + immediate in Thumb code. Materialize the immediate
-/// in a register using mov / mvn sequences or load the immediate from a
+/// emitThumbRegPlusImmInReg - Emits a series of instructions to materialize a
+/// destreg = basereg + immediate in Thumb code. Materialize the immediate in a
+/// register using mov / mvn (armv6-M >) sequences, movs / lsls / adds / lsls /
+/// adds / lsls / adds sequences (armv6-M) or load the immediate from a
 /// constpool entry.
 static void emitThumbRegPlusImmInReg(
     MachineBasicBlock &MBB, MachineBasicBlock::iterator &MBBI,
     const DebugLoc &dl, Register DestReg, Register BaseReg, int NumBytes,
     bool CanChangeCC, const TargetInstrInfo &TII,
     const ARMBaseRegisterInfo &MRI, unsigned MIFlags = MachineInstr::NoFlags) {
   MachineFunction &MF = *MBB.getParent();
   const ARMSubtarget &ST = MF.getSubtarget<ARMSubtarget>();
BuildMI(MBB, MBBI, dl, TII.get(ARM::tMOVi8), LdReg)
         .add(t1CondCodeOp())
         .addImm(NumBytes)
         .setMIFlags(MIFlags);
     BuildMI(MBB, MBBI, dl, TII.get(ARM::tRSB), LdReg)
         .add(t1CondCodeOp())
         .addReg(LdReg, RegState::Kill)
         .setMIFlags(MIFlags);
   } else if (ST.genExecuteOnly()) {
-    BuildMI(MBB, MBBI, dl, TII.get(ARM::t2MOVi32imm), LdReg)
+    unsigned XOInstr = ST.useMovt() ? ARM::t2MOVi32imm : ARM::tMOVi32imm;
+    BuildMI(MBB, MBBI, dl, TII.get(XOInstr), LdReg)
       .addImm(NumBytes).setMIFlags(MIFlags);
   } else
     MRI.emitLoadConstPool(MBB, MBBI, dl, LdReg, 0, NumBytes, ARMCC::AL, 0,
                           MIFlags);
 
   // Emit add / sub.
   int Opc = (isSub) ? ARM::tSUBrr
                     : ((isHigh || !CanChangeCC) ? ARM::tADDhirr : ARM::tADDrr);

llvm/test/CodeGen/ARM/large-stack.ll

 ; RUN: llc -mtriple=arm-eabi %s -o /dev/null
-; RUN: llc -mtriple=thumbv6m-eabi -mattr=+execute-only %s -o -
+; RUN: llc -mtriple=thumbv6m-eabi -mattr=+execute-only %s -o - -filetype=obj | \
+; RUN:   llvm-objdump -d --no-leading-addr --no-show-raw-insn - | Filecheck %s
 
 define void @test1() {
-    %tmp = alloca [ 64 x i32 ] , align 4
+; CHECK-LABEL: <test1>:
+;; are we using correct prologue immediate materialization pattern for
+;; execute only
+; CHECK: sub     sp, #0x100
     ret void
 }
 
 define void @test2() {
+; CHECK-LABEL: <test2>:
+;; are we using correct prologue immediate materialization pattern for
+;; execute-only
+; CHECK:      movs    [[REG:r[0-9]+]], #0xff
+; CHECK-NEXT: lsls    [[REG]], [[REG]], #0x8
+; CHECK-NEXT: adds    [[REG]], #0xff
+; CHECK-NEXT: lsls    [[REG]], [[REG]], #0x8
+; CHECK-NEXT: adds    [[REG]], #0xef
+; CHECK-NEXT: lsls    [[REG]], [[REG]], #0x8
+; CHECK-NEXT: adds    [[REG]], #0xb8
     %tmp = alloca [ 4168 x i8 ] , align 4
     ret void
 }
 
 define i32 @test3() {
+;; are we using correct prologue immediate materialization pattern for
+;; execute-only
+; CHECK-LABEL: <test3>:
+; CHECK: movs [[REG:r[0-9]+]], #0xcf
+; CHECK-NEXT: lsls    [[REG]], [[REG]], #0x8
+; CHECK-NEXT: adds    [[REG]], #0xff
+; CHECK-NEXT: lsls    [[REG]], [[REG]], #0x8
+; CHECK-NEXT: adds    [[REG]], #0xff
+; CHECK-NEXT: lsls    [[REG]], [[REG]], #0x8
+; CHECK-NEXT: adds    [[REG]], #0xf0
   %retval = alloca i32, align 4
   %tmp = alloca i32, align 4
-  %a = alloca [805306369 x i8], align 16
+  %a = alloca [u0x30000001 x i8], align 16
   store i32 0, ptr %tmp
+;; are we choosing correct store/tSTRspi pattern for execute-only
+; CHECK:      movs    [[REG:r[0-9]+]], #0x30
+; CHECK-NEXT: lsls    [[REG]], [[REG]], #0x8
+; CHECK-NEXT: adds    [[REG]], #0x0
+; CHECK-NEXT: lsls    [[REG]], [[REG]], #0x8
+; CHECK-NEXT: adds    [[REG]], #0x0
+; CHECK-NEXT: lsls    [[REG]], [[REG]], #0x8
+; CHECK-NEXT: adds    [[REG]], #0x8
   %tmp1 = load i32, ptr %tmp
         ret i32 %tmp1
 }