[RISCV] Support stack frames and offsets up to 32-bits

[asb]

Summary

Also, factor out stack pointer manipulation to use the new 'adjustReg' helper function.

I think this is fairly straight-forward, but I'd appreciate an extra pair of eyes just to recheck I'm setting killflags etc appropriately.

[reames]

There are a couple of NFC parts that need extracted here before the semantic change can be easily reviewed.

lib/Target/RISCV/RISCVFrameLowering.cpp

60

Looks like this is an RFC extraction. Feel free to separate and land.

71

You could add a case for when Val == 0 and use a reg move.

Hm, reviewing the ISA doc, it doesn't look like there's a register register move? Ok, add is fine. :)

84

This really looks like a helper function along the lines of materializedInt32(Reg, Val). I'm pretty sure you already have other copies of this code as well.

91

Is it correct to indicate a kill on a use when we're updating the same scratch reg?

lib/Target/RISCV/RISCVRegisterInfo.cpp

85

Two problems:

1. You're missing an assert that this is an int32.
2. This is the other use of materializeInt32 I mentioned above. :)

[asb]

Update to address review comments. adjustReg is now introduced in an earlier patch, and we also add RISCVInstrInfo::movImm32 in order to reduce duplication of logic.

lib/Target/RISCV/RISCVFrameLowering.cpp

71

Correct, there is an assembler pseudoinstruction but mv rd, rs is just addi rd, rs, 0.

91

I believe so, or at least there seem to be a number of examples of the same thing within lib/Target/*.

[reames]

Much cleaner, thanks!

[asb]

Rebase patch.

lib/Target/RISCV/RISCVFrameLowering.h

eliminateCallFramePseudoInstr(MachineFunction &MF, MachineBasicBlock &MBB,
     return MBB.erase(MI);
   }
 
 protected:
   const RISCVSubtarget &STI;
 
 private:
   void determineFrameLayout(MachineFunction &MF) const;
+  void adjustReg(MachineBasicBlock &MBB, MachineBasicBlock::iterator MBBI,
+                 const DebugLoc &DL, unsigned DestReg, unsigned SrcReg,
+                 int64_t Val, MachineInstr::MIFlag Flag) const;
 };
 }
 #endif

lib/Target/RISCV/RISCVFrameLowering.cpp

void RISCVFrameLowering::determineFrameLayout(MachineFunction &MF) const {
 
   // Make sure the frame is aligned.
   FrameSize = alignTo(FrameSize, StackAlign);
 
   // Update frame info.
   MFI.setStackSize(FrameSize);
 }
 
+void RISCVFrameLowering::adjustReg(MachineBasicBlock &MBB,
+                                   MachineBasicBlock::iterator MBBI,
+                                   const DebugLoc &DL, unsigned DestReg,
+                                   unsigned SrcReg, int64_t Val,
+                                   MachineInstr::MIFlag Flag) const {
+  MachineRegisterInfo &MRI = MBB.getParent()->getRegInfo();
+  const RISCVInstrInfo *TII = STI.getInstrInfo();
+
+  if (DestReg == SrcReg && Val == 0)
+    return;
+
+  if (isInt<12>(Val)) {
+    BuildMI(MBB, MBBI, DL, TII->get(RISCV::ADDI), DestReg)
+        .addReg(SrcReg)
+        .addImm(Val)
+        .setMIFlag(Flag);
+  } else if (isInt<32>(Val)) {
+    unsigned Opc = RISCV::ADD;
+    bool isSub = Val < 0;
+    if (isSub) {
+      Val = -Val;
+      Opc = RISCV::SUB;
+    }
+
+    unsigned ScratchReg = MRI.createVirtualRegister(&RISCV::GPRRegClass);
+    uint64_t Hi20 = ((Val + 0x800) >> 12) & 0xfffff;
+    uint64_t Lo12 = SignExtend64<12>(Val);
+    BuildMI(MBB, MBBI, DL, TII->get(RISCV::LUI), ScratchReg)
+        .addImm(Hi20)
+        .setMIFlag(Flag);
+    BuildMI(MBB, MBBI, DL, TII->get(RISCV::ADDI), ScratchReg)
+        .addReg(ScratchReg, RegState::Kill)
+        .addImm(Lo12)
+        .setMIFlag(Flag);
+    BuildMI(MBB, MBBI, DL, TII->get(Opc), DestReg)
+        .addReg(SrcReg)
+        .addReg(ScratchReg, RegState::Kill)
+        .setMIFlag(Flag);
+  } else {
+    report_fatal_error("adjustReg cannot yet handle adjustments >32 bits");
+  }
+}
+
 void RISCVFrameLowering::emitPrologue(MachineFunction &MF,
                                       MachineBasicBlock &MBB) const {
   assert(&MF.front() == &MBB && "Shrink-wrapping not yet supported");
 
   if (!hasFP(MF)) {
     report_fatal_error(
         "emitPrologue doesn't support framepointer-less functions");
   }
 
   MachineFrameInfo &MFI = MF.getFrameInfo();
-  const RISCVInstrInfo *TII = STI.getInstrInfo();
   auto *RVFI = MF.getInfo<RISCVMachineFunctionInfo>();
   MachineBasicBlock::iterator MBBI = MBB.begin();
 
   unsigned FPReg = RISCV::X8;
   unsigned SPReg = RISCV::X2;
 
   // Debug location must be unknown since the first debug location is used
   // to determine the end of the prologue.
   DebugLoc DL;
 
   // Determine the correct frame layout
   determineFrameLayout(MF);
 
   // FIXME (note copied from Lanai): This appears to be overallocating.  Needs
   // investigation. Get the number of bytes to allocate from the FrameInfo.
   uint64_t StackSize = MFI.getStackSize();
 
   // Early exit if there is no need to allocate on the stack
   if (StackSize == 0 && !MFI.adjustsStack())
     return;
 
-  if (!isInt<12>(StackSize)) {
-    report_fatal_error("Stack adjustment won't fit in signed 12-bit immediate");
-  }
-
   // Allocate space on the stack if necessary.
-  if (StackSize != 0) {
-    BuildMI(MBB, MBBI, DL, TII->get(RISCV::ADDI), SPReg)
-        .addReg(SPReg)
-        .addImm(-StackSize)
-        .setMIFlag(MachineInstr::FrameSetup);
-  }
+  adjustReg(MBB, MBBI, DL, SPReg, SPReg, -StackSize, MachineInstr::FrameSetup);
 
   // The frame pointer is callee-saved, and code has been generated for us to
   // save it to the stack. We need to skip over the storing of callee-saved
   // registers as the frame pointer must be modified after it has been saved
   // to the stack, not before.
   // FIXME: assumes exactly one instruction is used to save each callee-saved
   // register.
   const std::vector<CalleeSavedInfo> &CSI = MFI.getCalleeSavedInfo();
   std::advance(MBBI, CSI.size());
 
   // Generate new FP.
-  BuildMI(MBB, MBBI, DL, TII->get(RISCV::ADDI), FPReg)
-      .addReg(SPReg)
-      .addImm(StackSize - RVFI->getVarArgsSaveSize())
-      .setMIFlag(MachineInstr::FrameSetup);
+  adjustReg(MBB, MBBI, DL, FPReg, SPReg, StackSize - RVFI->getVarArgsSaveSize(),
+            MachineInstr::FrameSetup);
 }
 
 void RISCVFrameLowering::emitEpilogue(MachineFunction &MF,
                                       MachineBasicBlock &MBB) const {
   if (!hasFP(MF)) {
     report_fatal_error(
         "emitEpilogue doesn't support framepointer-less functions");
   }
 
   MachineBasicBlock::iterator MBBI = MBB.getLastNonDebugInstr();
-  const RISCVInstrInfo *TII = STI.getInstrInfo();
   const RISCVRegisterInfo *RI = STI.getRegisterInfo();
   MachineFrameInfo &MFI = MF.getFrameInfo();
   auto *RVFI = MF.getInfo<RISCVMachineFunctionInfo>();
   DebugLoc DL = MBBI->getDebugLoc();
   unsigned FPReg = RISCV::X8;
   unsigned SPReg = RISCV::X2;
 
   // Skip to before the restores of callee-saved registers
   // FIXME: assumes exactly one instruction is used to restore each
   // callee-saved register.
   MachineBasicBlock::iterator LastFrameDestroy = MBBI;
   std::advance(LastFrameDestroy, -MFI.getCalleeSavedInfo().size());
 
   uint64_t StackSize = MFI.getStackSize();
 
   // Restore the stack pointer using the value of the frame pointer. Only
   // necessary if the stack pointer was modified, meaning the stack size is
   // unknown.
   if (RI->needsStackRealignment(MF) || MFI.hasVarSizedObjects()) {
-    BuildMI(MBB, LastFrameDestroy, DL, TII->get(RISCV::ADDI), SPReg)
-        .addReg(FPReg)
-        .addImm(-StackSize + RVFI->getVarArgsSaveSize())
-        .setMIFlag(MachineInstr::FrameDestroy);
-  }
-
-  if (!isInt<12>(StackSize)) {
-    report_fatal_error("Stack adjustment won't fit in signed 12-bit immediate");
+    adjustReg(MBB, LastFrameDestroy, DL, SPReg, FPReg,
+              -StackSize + RVFI->getVarArgsSaveSize(),
+              MachineInstr::FrameDestroy);
   }
 
   // Deallocate stack
-  BuildMI(MBB, MBBI, DL, TII->get(RISCV::ADDI), SPReg)
-      .addReg(SPReg)
-      .addImm(StackSize)
-      .setMIFlag(MachineInstr::FrameDestroy);
+  adjustReg(MBB, MBBI, DL, SPReg, SPReg, StackSize, MachineInstr::FrameDestroy);
 }
 
 int RISCVFrameLowering::getFrameIndexReference(const MachineFunction &MF,
                                                int FI,
                                                unsigned &FrameReg) const {
   const MachineFrameInfo &MFI = MF.getFrameInfo();
   const TargetRegisterInfo *RI = MF.getSubtarget().getRegisterInfo();
   const auto *RVFI = MF.getInfo<RISCVMachineFunctionInfo>();

lib/Target/RISCV/RISCVRegisterInfo.h

struct RISCVRegisterInfo : public RISCVGenRegisterInfo {
 
   const uint32_t *getNoPreservedMask() const override;
 
   void eliminateFrameIndex(MachineBasicBlock::iterator MI, int SPAdj,
                            unsigned FIOperandNum,
                            RegScavenger *RS = nullptr) const override;
 
   unsigned getFrameRegister(const MachineFunction &MF) const override;
+
+  bool requiresRegisterScavenging(const MachineFunction &MF) const override {
+    return true;
+  }
+
+  bool requiresFrameIndexScavenging(const MachineFunction &MF) const override {
+    return true;
+  }
 };
 }
 
 #endif

lib/Target/RISCV/RISCVRegisterInfo.cpp

 
 void RISCVRegisterInfo::eliminateFrameIndex(MachineBasicBlock::iterator II,
                                             int SPAdj, unsigned FIOperandNum,
                                             RegScavenger *RS) const {
   assert(SPAdj == 0 && "Unexpected non-zero SPAdj value");
 
   MachineInstr &MI = *II;
   MachineFunction &MF = *MI.getParent()->getParent();
+  MachineRegisterInfo &MRI = MF.getRegInfo();
+  const TargetInstrInfo *TII = MF.getSubtarget().getInstrInfo();
   DebugLoc DL = MI.getDebugLoc();
 
   int FrameIndex = MI.getOperand(FIOperandNum).getIndex();
   unsigned FrameReg;
   int Offset =
       getFrameLowering(MF)->getFrameIndexReference(MF, FrameIndex, FrameReg) +
       MI.getOperand(FIOperandNum + 1).getImm();
 
   assert(MF.getSubtarget().getFrameLowering()->hasFP(MF) &&
          "eliminateFrameIndex currently requires hasFP");
 
-  // Offsets must be directly encoded in a 12-bit immediate field
-  if (!isInt<12>(Offset)) {
+  if (!isInt<32>(Offset)) {
     report_fatal_error(
-        "Frame offsets outside of the signed 12-bit range not supported");
+        "Frame offsets outside of the signed 32-bit range not supported");
+  }
+
+  MachineBasicBlock &MBB = *MI.getParent();
+  bool FrameRegIsKill = false;
+
+  if (!isInt<12>(Offset)) {
+    // The offset won't fit in an immediate, so use a scratch register instead
+    // Modify Offset and FrameReg appropriately
+    unsigned ScratchReg = MRI.createVirtualRegister(&RISCV::GPRRegClass);
+    uint64_t Hi20 = ((Offset + 0x800) >> 12) & 0xfffff;
+    uint64_t Lo12 = SignExtend64<12>(Offset);
+    BuildMI(MBB, II, DL, TII->get(RISCV::LUI), ScratchReg).addImm(Hi20);
+    BuildMI(MBB, II, DL, TII->get(RISCV::ADDI), ScratchReg)
+        .addReg(ScratchReg, RegState::Kill)
+        .addImm(Lo12);
+    BuildMI(MBB, II, DL, TII->get(RISCV::ADD), ScratchReg)
+        .addReg(FrameReg)
+        .addReg(ScratchReg, RegState::Kill);
+    Offset = 0;
+    FrameReg = ScratchReg;
+    FrameRegIsKill = true;
   }
 
-  MI.getOperand(FIOperandNum).ChangeToRegister(FrameReg, false);
+  MI.getOperand(FIOperandNum)
+      .ChangeToRegister(FrameReg, false, false, FrameRegIsKill);
   MI.getOperand(FIOperandNum + 1).ChangeToImmediate(Offset);
 }
 
 unsigned RISCVRegisterInfo::getFrameRegister(const MachineFunction &MF) const {
   return RISCV::X8;
 }
 
 const uint32_t *
 RISCVRegisterInfo::getCallPreservedMask(const MachineFunction & /*MF*/,
                                         CallingConv::ID /*CC*/) const {
   return CSR_RegMask;
 }

test/CodeGen/RISCV/large-stack.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=riscv32 -verify-machineinstrs < %s \
+; RUN:   | FileCheck -check-prefix=RV32I %s
+
+; TODO: the quality of the generated code is poor
+
+define void @test() nounwind {
+; RV32I-LABEL: test:
+; RV32I:       # BB#0:
+; RV32I-NEXT:    lui a0, 74565
+; RV32I-NEXT:    addi a0, a0, 1664
+; RV32I-NEXT:    sub sp, sp, a0
+; RV32I-NEXT:    lui a0, 74565
+; RV32I-NEXT:    addi a0, a0, 1660
+; RV32I-NEXT:    add a0, sp, a0
+; RV32I-NEXT:    sw ra, 0(a0)
+; RV32I-NEXT:    lui a0, 74565
+; RV32I-NEXT:    addi a0, a0, 1656
+; RV32I-NEXT:    add a0, sp, a0
+; RV32I-NEXT:    sw s0, 0(a0)
+; RV32I-NEXT:    lui a0, 74565
+; RV32I-NEXT:    addi a0, a0, 1664
+; RV32I-NEXT:    add s0, sp, a0
+; RV32I-NEXT:    lui a0, 74565
+; RV32I-NEXT:    addi a0, a0, 1656
+; RV32I-NEXT:    add a0, sp, a0
+; RV32I-NEXT:    lw s0, 0(a0)
+; RV32I-NEXT:    lui a0, 74565
+; RV32I-NEXT:    addi a0, a0, 1660
+; RV32I-NEXT:    add a0, sp, a0
+; RV32I-NEXT:    lw ra, 0(a0)
+; RV32I-NEXT:    lui a0, 74565
+; RV32I-NEXT:    addi a0, a0, 1664
+; RV32I-NEXT:    add sp, sp, a0
+; RV32I-NEXT:    jalr zero, ra, 0
+  %tmp = alloca [ 305419896 x i8 ] , align 4
+  ret void
+}