[PATCH] D118545: [ARM] Fix 8-bit immediate overflow in the instruction of segmented stack prologue.

[John Brawn via Phabricator via llvm-commits]

john.brawn added a comment.

Please add a test for this. llvm/test/CodeGen/ARM/segmented-stacks.ll already has some tests related to this, so it looks like you could add a couple of RUN lines for thumb and thumb2 targets, and add test_very_large that's like test_large but with an integer that's too large for ARM::MOVi.



================
Comment at: llvm/lib/Target/ARM/ARMFrameLowering.cpp:2459
+/// thumb1 shift and add instructions.
+static void thumb1LoadImm32(MachineBasicBlock *MBB, DebugLoc &DL,
+                            const ARMBaseInstrInfo &TII, unsigned Reg,
----------------
We already have pseudo-instructions for mov of 32-bit immediate in arm and thumb2, so instead of having a function to emit the code sequence here it would be better I think to create a tMOVi32imm pseudo-instruction and generate the instruction sequence when expanding it.


================
Comment at: llvm/lib/Target/ARM/ARMFrameLowering.cpp:2687
+      if (Thumb2) {
+        BuildMI(McrMBB, DL, TII.get(ARM::t2MOVCCi32imm), ScratchReg0)
+            .add(condCodeOp())
----------------
This isn't a conditional move, so this should be t2MOVi32imm.


================
Comment at: llvm/lib/Target/ARM/ARMFrameLowering.cpp:2701
   } else if (!CompareStackPointer) {
     BuildMI(McrMBB, DL, TII.get(ARM::SUBri), ScratchReg1)
         .addReg(ARM::SP)
----------------
We'll also have incorrect code generation when AlignedStackSize is larger than the max immediate of ARM::SUBri (and similar with ARM::MOVi down below), so we should similarly be making use of ARM::MOVi32imm to generate the immediate here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D118545/new/

https://reviews.llvm.org/D118545