[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.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))
-    report_fatal_error("adjustReg cannot yet handle adjustments >12 bits");
-
+  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);
+    TII->movImm32(MBB, MBBI, DL, ScratchReg, Val, 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");
+  }
 }
 
 // Returns the register used to hold the frame pointer.
 static unsigned getFPReg(const RISCVSubtarget &STI) { return RISCV::X8; }
 
 // Returns the register used to hold the stack pointer.
 static unsigned getSPReg(const RISCVSubtarget &STI) { return RISCV::X2; }
 

lib/Target/RISCV/RISCVInstrInfo.h

void storeRegToStackSlot(MachineBasicBlock &MBB,
                            bool IsKill, int FrameIndex,
                            const TargetRegisterClass *RC,
                            const TargetRegisterInfo *TRI) const override;
 
   void loadRegFromStackSlot(MachineBasicBlock &MBB,
                             MachineBasicBlock::iterator MBBI, unsigned DstReg,
                             int FrameIndex, const TargetRegisterClass *RC,
                             const TargetRegisterInfo *TRI) const override;
+
+  // Materializes the given int32 Val into DstReg.
+  void movImm32(MachineBasicBlock &MBB, MachineBasicBlock::iterator MBBI,
+                const DebugLoc &DL, unsigned DstReg, uint64_t Val,
+                MachineInstr::MIFlag Flag = MachineInstr::NoFlags) const;
 };
 }
 #endif

lib/Target/RISCV/RISCVInstrInfo.cpp

void RISCVInstrInfo::loadRegFromStackSlot(MachineBasicBlock &MBB,
   if (I != MBB.end())
     DL = I->getDebugLoc();
 
   if (RISCV::GPRRegClass.hasSubClassEq(RC))
     BuildMI(MBB, I, DL, get(RISCV::LW), DstReg).addFrameIndex(FI).addImm(0);
   else
     llvm_unreachable("Can't load this register from stack slot");
 }
+
+void RISCVInstrInfo::movImm32(MachineBasicBlock &MBB,
+                              MachineBasicBlock::iterator MBBI,
+                              const DebugLoc &DL, unsigned DstReg, uint64_t Val,
+                              MachineInstr::MIFlag Flag) const {
+  assert(isInt<32>(Val) && "Can only materialize 32-bit constants");
+
+  // TODO: If the value can be materialized using only one instruction, only
+  // insert a single instruction.
+
+  uint64_t Hi20 = ((Val + 0x800) >> 12) & 0xfffff;
+  uint64_t Lo12 = SignExtend64<12>(Val);
+  BuildMI(MBB, MBBI, DL, get(RISCV::LUI), DstReg)
+      .addImm(Hi20)
+      .setMIFlag(Flag);
+  BuildMI(MBB, MBBI, DL, get(RISCV::ADDI), DstReg)
+      .addReg(DstReg, RegState::Kill)
+      .addImm(Lo12)
+      .setMIFlag(Flag);
+}

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 RISCVInstrInfo *TII = MF.getSubtarget<RISCVSubtarget>().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)) {
+    assert(isInt<32>(Offset) && "Int32 expected");
+    // 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);
+    TII->movImm32(MBB, II, DL, ScratchReg, Offset);
+    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
+}