[RISCV] Add CFI directives for RISCV prologue/epilog.

[HsiangKai]

Summary

In order to generate correct debug frame information, it needs to generate CFI information in prologue and epilog.

[shiva0217]

Hi Kai,

Other codegen test cases will generate .cfi directive with the patch. So other codegen test cases may need to update.

[HsiangKai]

Update test cases for RISCV.

[asb]

Better to modify the tests to add nounwind. I have a patch that does that sitting around (I added CFI directives downstream but didn't clean it up for submission). Let me apply the test suite patch upstream, then can you rebase on that.

[asb]

In D61773#1504714, @asb wrote:

Better to modify the tests to add nounwind. I have a patch that does that sitting around (I added CFI directives downstream but didn't clean it up for submission). Let me apply the test suite patch upstream, then can you rebase on that.

Committed in rL360897

[HsiangKai]

In D61773#1504748, @asb wrote:

In D61773#1504714, @asb wrote:

Better to modify the tests to add nounwind. I have a patch that does that sitting around (I added CFI directives downstream but didn't clean it up for submission). Let me apply the test suite patch upstream, then can you rebase on that.

Committed in rL360897

Fine. I will rebase on it. Thanks.

[HsiangKai]

Update test cases.

[asb]

Hi @HsiangKai - it seems I applied an old/wip version of my nounwind patch upstream. I've now fixed that in rL361493. With that, there should be no test changes when implementing this feature.

Please take a look at the two minor suggestions I had on updating frame-info.ll.

test/CodeGen/RISCV/frame-info.ll

1 ↗

(On Diff #199958)

For completeness, please add a riscv64 RUN line too.

2 ↗

(On Diff #199958)

I think it would be worth generating this with update_llc_test_checks.py. Then you can also see the generated instructions in the CHECK lines which makes it easy to check that the cfi directives match up to the generated stack layout.

[asb]

lib/Target/RISCV/RISCVFrameLowering.cpp

125 ↗

(On Diff #199958)

Should say -StackSize?

142 ↗

(On Diff #199958)

I think you can do this more concisely with:

for (const auto &Entry : CSI) {
  unsigned Reg = Entry.getReg();
  int FI = Entry.getFrameIdx();
  ....

[HsiangKai]

lib/Target/RISCV/RISCVFrameLowering.cpp

125 ↗

(On Diff #199958)

It should be ".cfi_def_cfa_offset StackSize". This directive is put after "addi sp, sp, -StackSize". CFA should be a fixed point. So, after updating stack pointer to lower address, we should update CFA as current stack pointer plus StackSize.

[HsiangKai]

Update the test case.

[shiva0217]

Hi Kai,
We could also generate .cfi_restore for epilogue by:

unsigned CFIIndex = MF->addFrameInst(MCCFIInstruction::createRestore(
    nullptr, MRI->getDwarfRegNum(Reg, 1)));
BuildMI(MBB, MBBI, DL, TII.get(TargetOpcode::CFI_INSTRUCTION))
    .addCFIIndex(CFIIndex);

[HsiangKai]

Add .cfi_restore for callee-saved registers in epilogue.

[HsiangKai]

Update test cases.

[asb]

This looks good from my perspective - @probinson, how does it look to you?

[asb]

I'm happy this is a big improvement over what we have (which is no CFI directive emission :)), so please go ahead and commit this. Thanks!

llvm/trunk/lib/Target/RISCV/MCTargetDesc/RISCVMCAsmInfo.cpp

 
 void RISCVMCAsmInfo::anchor() {}
 
 RISCVMCAsmInfo::RISCVMCAsmInfo(const Triple &TT) {
   CodePointerSize = CalleeSaveStackSlotSize = TT.isArch64Bit() ? 8 : 4;
   CommentString = "#";
   AlignmentIsInBytes = false;
   SupportsDebugInformation = true;
+  ExceptionsType = ExceptionHandling::DwarfCFI;
   Data16bitsDirective = "\t.half\t";
   Data32bitsDirective = "\t.word\t";
 }

llvm/trunk/lib/Target/RISCV/MCTargetDesc/RISCVMCTargetDesc.cpp

 static MCRegisterInfo *createRISCVMCRegisterInfo(const Triple &TT) {
   MCRegisterInfo *X = new MCRegisterInfo();
   InitRISCVMCRegisterInfo(X, RISCV::X1);
   return X;
 }
 
 static MCAsmInfo *createRISCVMCAsmInfo(const MCRegisterInfo &MRI,
                                        const Triple &TT) {
-  return new RISCVMCAsmInfo(TT);
+  MCAsmInfo *MAI = new RISCVMCAsmInfo(TT);
+
+  unsigned SP = MRI.getDwarfRegNum(RISCV::X2, true);
+  MCCFIInstruction Inst = MCCFIInstruction::createDefCfa(nullptr, SP, 0);
+  MAI->addInitialFrameState(Inst);
+
+  return MAI;
 }
 
 static MCSubtargetInfo *createRISCVMCSubtargetInfo(const Triple &TT,
                                                    StringRef CPU, StringRef FS) {
   std::string CPUName = CPU;
   if (CPUName.empty())
     CPUName = TT.isArch64Bit() ? "generic-rv64" : "generic-rv32";
   return createRISCVMCSubtargetInfoImpl(TT, CPUName, FS);

llvm/trunk/lib/Target/RISCV/RISCVFrameLowering.cpp

 #include "RISCVFrameLowering.h"
 #include "RISCVMachineFunctionInfo.h"
 #include "RISCVSubtarget.h"
 #include "llvm/CodeGen/MachineFrameInfo.h"
 #include "llvm/CodeGen/MachineFunction.h"
 #include "llvm/CodeGen/MachineInstrBuilder.h"
 #include "llvm/CodeGen/MachineRegisterInfo.h"
 #include "llvm/CodeGen/RegisterScavenging.h"
+#include "llvm/MC/MCDwarf.h"
 
 using namespace llvm;
 
 bool RISCVFrameLowering::hasFP(const MachineFunction &MF) const {
   const TargetRegisterInfo *RegInfo = MF.getSubtarget().getRegisterInfo();
 
   const MachineFrameInfo &MFI = MF.getFrameInfo();
   return MF.getTarget().Options.DisableFramePointerElim(MF) ||

 static unsigned getSPReg(const RISCVSubtarget &STI) { return RISCV::X2; }
 
 void RISCVFrameLowering::emitPrologue(MachineFunction &MF,
                                       MachineBasicBlock &MBB) const {
   assert(&MF.front() == &MBB && "Shrink-wrapping not yet supported");
 
   MachineFrameInfo &MFI = MF.getFrameInfo();
   auto *RVFI = MF.getInfo<RISCVMachineFunctionInfo>();
+  const RISCVRegisterInfo *RI = STI.getRegisterInfo();
+  const RISCVInstrInfo *TII = STI.getInstrInfo();
   MachineBasicBlock::iterator MBBI = MBB.begin();
 
   unsigned FPReg = getFPReg(STI);
   unsigned SPReg = getSPReg(STI);
 
   // 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;
 
   // Allocate space on the stack if necessary.
   adjustReg(MBB, MBBI, DL, SPReg, SPReg, -StackSize, MachineInstr::FrameSetup);
 
+  // Emit ".cfi_def_cfa_offset StackSize"
+  unsigned CFIIndex = MF.addFrameInst(
+      MCCFIInstruction::createDefCfaOffset(nullptr, -StackSize));
+  BuildMI(MBB, MBBI, DL, TII->get(TargetOpcode::CFI_INSTRUCTION))
+      .addCFIIndex(CFIIndex);
+
   // 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());
 
+  // Iterate over list of callee-saved registers and emit .cfi_offset
+  // directives.
+  for (const auto &Entry : CSI) {
+    int64_t Offset = MFI.getObjectOffset(Entry.getFrameIdx());
+    unsigned Reg = Entry.getReg();
+    unsigned CFIIndex = MF.addFrameInst(MCCFIInstruction::createOffset(
+        nullptr, RI->getDwarfRegNum(Reg, true), Offset));
+    BuildMI(MBB, MBBI, DL, TII->get(TargetOpcode::CFI_INSTRUCTION))
+        .addCFIIndex(CFIIndex);
+  }
+
   // Generate new FP.
-  if (hasFP(MF))
+  if (hasFP(MF)) {
     adjustReg(MBB, MBBI, DL, FPReg, SPReg,
               StackSize - RVFI->getVarArgsSaveSize(), MachineInstr::FrameSetup);
+
+    // Emit ".cfi_def_cfa $fp, 0"
+    unsigned CFIIndex = MF.addFrameInst(MCCFIInstruction::createDefCfa(
+        nullptr, RI->getDwarfRegNum(FPReg, true), 0));
+    BuildMI(MBB, MBBI, DL, TII->get(TargetOpcode::CFI_INSTRUCTION))
+        .addCFIIndex(CFIIndex);
+  }
 }
 
 void RISCVFrameLowering::emitEpilogue(MachineFunction &MF,
                                       MachineBasicBlock &MBB) const {
   MachineBasicBlock::iterator MBBI = MBB.getLastNonDebugInstr();
   const RISCVRegisterInfo *RI = STI.getRegisterInfo();
   MachineFrameInfo &MFI = MF.getFrameInfo();
   auto *RVFI = MF.getInfo<RISCVMachineFunctionInfo>();
   DebugLoc DL = MBBI->getDebugLoc();
+  const RISCVInstrInfo *TII = STI.getInstrInfo();
   unsigned FPReg = getFPReg(STI);
   unsigned SPReg = getSPReg(STI);
 
   // Skip to before the restores of callee-saved registers
   // FIXME: assumes exactly one instruction is used to restore each
   // callee-saved register.
   auto LastFrameDestroy = std::prev(MBBI, MFI.getCalleeSavedInfo().size());
 
   uint64_t StackSize = MFI.getStackSize();
+  uint64_t FPOffset = StackSize - RVFI->getVarArgsSaveSize();
 
   // 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()) {
     assert(hasFP(MF) && "frame pointer should not have been eliminated");
-    adjustReg(MBB, LastFrameDestroy, DL, SPReg, FPReg,
-              -StackSize + RVFI->getVarArgsSaveSize(),
+    adjustReg(MBB, LastFrameDestroy, DL, SPReg, FPReg, -FPOffset,
               MachineInstr::FrameDestroy);
   }
 
+  if (hasFP(MF)) {
+    // To find the instruction restoring FP from stack.
+    for (auto &I = LastFrameDestroy; I != MBBI; ++I) {
+      if (I->mayLoad() && I->getOperand(0).isReg()) {
+        unsigned DestReg = I->getOperand(0).getReg();
+        if (DestReg == FPReg) {
+          // If there is frame pointer, after restoring $fp registers, we
+          // need adjust CFA to ($sp - FPOffset).
+          // Emit ".cfi_def_cfa $sp, -FPOffset"
+          unsigned CFIIndex = MF.addFrameInst(MCCFIInstruction::createDefCfa(
+              nullptr, RI->getDwarfRegNum(SPReg, true), -FPOffset));
+          BuildMI(MBB, std::next(I), DL,
+                  TII->get(TargetOpcode::CFI_INSTRUCTION))
+              .addCFIIndex(CFIIndex);
+          break;
+        }
+      }
+    }
+  }
+
+  // Add CFI directives for callee-saved registers.
+  const std::vector<CalleeSavedInfo> &CSI = MFI.getCalleeSavedInfo();
+  // Iterate over list of callee-saved registers and emit .cfi_restore
+  // directives.
+  for (const auto &Entry : CSI) {
+    unsigned Reg = Entry.getReg();
+    unsigned CFIIndex = MF.addFrameInst(MCCFIInstruction::createRestore(
+        nullptr, RI->getDwarfRegNum(Reg, true)));
+    BuildMI(MBB, MBBI, DL, TII->get(TargetOpcode::CFI_INSTRUCTION))
+        .addCFIIndex(CFIIndex);
+  }
+
   // Deallocate stack
   adjustReg(MBB, MBBI, DL, SPReg, SPReg, StackSize, MachineInstr::FrameDestroy);
+
+  // After restoring $sp, we need to adjust CFA to $(sp + 0)
+  // Emit ".cfi_def_cfa_offset 0"
+  unsigned CFIIndex =
+      MF.addFrameInst(MCCFIInstruction::createDefCfaOffset(nullptr, 0));
+  BuildMI(MBB, MBBI, DL, TII->get(TargetOpcode::CFI_INSTRUCTION))
+      .addCFIIndex(CFIIndex);
 }
 
 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>();

llvm/trunk/test/CodeGen/RISCV/frame-info.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=riscv32 < %s | FileCheck -check-prefix=RV32 %s
+; RUN: llc -mtriple=riscv64 < %s | FileCheck -check-prefix=RV64 %s
+
+define void @foo(i32 signext %size) {
+; RV32-LABEL: foo:
+; RV32:       # %bb.0: # %entry
+; RV32-NEXT:    addi sp, sp, -16
+; RV32-NEXT:    .cfi_def_cfa_offset 16
+; RV32-NEXT:    sw ra, 12(sp)
+; RV32-NEXT:    sw s0, 8(sp)
+; RV32-NEXT:    .cfi_offset ra, -4
+; RV32-NEXT:    .cfi_offset s0, -8
+; RV32-NEXT:    addi s0, sp, 16
+; RV32-NEXT:    .cfi_def_cfa s0, 0
+; RV32-NEXT:    addi a0, a0, 15
+; RV32-NEXT:    andi a0, a0, -16
+; RV32-NEXT:    sub a0, sp, a0
+; RV32-NEXT:    mv sp, a0
+; RV32-NEXT:    call bar
+; RV32-NEXT:    addi sp, s0, -16
+; RV32-NEXT:    lw s0, 8(sp)
+; RV32-NEXT:    .cfi_def_cfa sp, 16
+; RV32-NEXT:    lw ra, 12(sp)
+; RV32-NEXT:    .cfi_restore ra
+; RV32-NEXT:    .cfi_restore s0
+; RV32-NEXT:    addi sp, sp, 16
+; RV32-NEXT:    .cfi_def_cfa_offset 0
+; RV32-NEXT:    ret
+;
+; RV64-LABEL: foo:
+; RV64:       # %bb.0: # %entry
+; RV64-NEXT:    addi sp, sp, -16
+; RV64-NEXT:    .cfi_def_cfa_offset 16
+; RV64-NEXT:    sd ra, 8(sp)
+; RV64-NEXT:    sd s0, 0(sp)
+; RV64-NEXT:    .cfi_offset ra, -8
+; RV64-NEXT:    .cfi_offset s0, -16
+; RV64-NEXT:    addi s0, sp, 16
+; RV64-NEXT:    .cfi_def_cfa s0, 0
+; RV64-NEXT:    addi a1, zero, 1
+; RV64-NEXT:    slli a1, a1, 33
+; RV64-NEXT:    addi a1, a1, -16
+; RV64-NEXT:    slli a0, a0, 32
+; RV64-NEXT:    srli a0, a0, 32
+; RV64-NEXT:    addi a0, a0, 15
+; RV64-NEXT:    and a0, a0, a1
+; RV64-NEXT:    sub a0, sp, a0
+; RV64-NEXT:    mv sp, a0
+; RV64-NEXT:    call bar
+; RV64-NEXT:    addi sp, s0, -16
+; RV64-NEXT:    ld s0, 0(sp)
+; RV64-NEXT:    .cfi_def_cfa sp, 16
+; RV64-NEXT:    ld ra, 8(sp)
+; RV64-NEXT:    .cfi_restore ra
+; RV64-NEXT:    .cfi_restore s0
+; RV64-NEXT:    addi sp, sp, 16
+; RV64-NEXT:    .cfi_def_cfa_offset 0
+; RV64-NEXT:    ret
+entry:
+  %0 = alloca i8, i32 %size, align 16
+  call void @bar(i8* nonnull %0) #2
+  ret void
+}
+
+declare void @bar(i8*)