Generate Callee Saved Register (CSR) related cfi directives like .cfi_restore.

https://reviews.llvm.org/D42848 only handled CFA related cfi directives but
didn't handle CSR related cfi. The patch adds the CSR part. Basically it reuses
the framework created in D42848. For each basicblock, the patch tracks which
CSR set have been saved at its CFG predecessors's exits, and compare the CSR
set with the set at its previous basicblock's exit (The previous block is the
block laid before the current block). If the saved CSR set at its previous
basicblock's exit is larger, .cfi_restore will be inserted.

The patch also generates proper .cfi_restore in epilogue to make sure the
saved CSR set is consistent for the incoming edges of each block.

Differential Revision: https://reviews.llvm.org/D74303

diff --git a/llvm/lib/CodeGen/CFIInstrInserter.cpp b/llvm/lib/CodeGen/CFIInstrInserter.cpp
index ef548c8..2fc436e 100644 (file)
--- a/llvm/lib/CodeGen/CFIInstrInserter.cpp
+++ b/llvm/lib/CodeGen/CFIInstrInserter.cpp
@@ -18,6 +18,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/ADT/DepthFirstIterator.h"
+#include "llvm/ADT/SetOperations.h"
 #include "llvm/CodeGen/MachineFunctionPass.h"
 #include "llvm/CodeGen/MachineInstrBuilder.h"
 #include "llvm/CodeGen/MachineModuleInfo.h"
@@ -76,6 +77,10 @@ class CFIInstrInserter : public MachineFunctionPass {
     unsigned IncomingCFARegister = 0;
     /// Value of cfa register valid at basic block exit.
     unsigned OutgoingCFARegister = 0;
+    /// Set of callee saved registers saved at basic block entry.
+    BitVector IncomingCSRSaved;
+    /// Set of callee saved registers saved at basic block exit.
+    BitVector OutgoingCSRSaved;
     /// If in/out cfa offset and register values for this block have already
     /// been set or not.
     bool Processed = false;
@@ -108,7 +113,8 @@ class CFIInstrInserter : public MachineFunctionPass {
     return -MBBVector[MBB->getNumber()].IncomingCFAOffset;
   }
 
-  void report(const MBBCFAInfo &Pred, const MBBCFAInfo &Succ);
+  void reportCFAError(const MBBCFAInfo &Pred, const MBBCFAInfo &Succ);
+  void reportCSRError(const MBBCFAInfo &Pred, const MBBCFAInfo &Succ);
   /// Go through each MBB in a function and check that outgoing offset and
   /// register of its predecessors match incoming offset and register of that
   /// MBB, as well as that incoming offset and register of its successors match
@@ -132,6 +138,8 @@ void CFIInstrInserter::calculateCFAInfo(MachineFunction &MF) {
   // function.
   unsigned InitialRegister =
       MF.getSubtarget().getFrameLowering()->getInitialCFARegister(MF);
+  const TargetRegisterInfo &TRI = *MF.getSubtarget().getRegisterInfo();
+  unsigned NumRegs = TRI.getNumRegs();
 
   // Initialize MBBMap.
   for (MachineBasicBlock &MBB : MF) {
@@ -141,6 +149,8 @@ void CFIInstrInserter::calculateCFAInfo(MachineFunction &MF) {
     MBBInfo.OutgoingCFAOffset = InitialOffset;
     MBBInfo.IncomingCFARegister = InitialRegister;
     MBBInfo.OutgoingCFARegister = InitialRegister;
+    MBBInfo.IncomingCSRSaved.resize(NumRegs);
+    MBBInfo.OutgoingCSRSaved.resize(NumRegs);
     MBBVector[MBB.getNumber()] = MBBInfo;
   }
 
@@ -159,8 +169,11 @@ void CFIInstrInserter::calculateOutgoingCFAInfo(MBBCFAInfo &MBBInfo) {
   int SetOffset = MBBInfo.IncomingCFAOffset;
   // Outgoing cfa register set by the block.
   unsigned SetRegister = MBBInfo.IncomingCFARegister;
-  const std::vector<MCCFIInstruction> &Instrs =
-      MBBInfo.MBB->getParent()->getFrameInstructions();
+  MachineFunction *MF = MBBInfo.MBB->getParent();
+  const std::vector<MCCFIInstruction> &Instrs = MF->getFrameInstructions();
+  const TargetRegisterInfo &TRI = *MF->getSubtarget().getRegisterInfo();
+  unsigned NumRegs = TRI.getNumRegs();
+  BitVector CSRSaved(NumRegs), CSRRestored(NumRegs);
 
   // Determine cfa offset and register set by the block.
   for (MachineInstr &MI : *MBBInfo.MBB) {
@@ -181,6 +194,15 @@ void CFIInstrInserter::calculateOutgoingCFAInfo(MBBCFAInfo &MBBInfo) {
         SetRegister = CFI.getRegister();
         SetOffset = CFI.getOffset();
         break;
+      case MCCFIInstruction::OpOffset:
+      case MCCFIInstruction::OpRegister:
+      case MCCFIInstruction::OpRelOffset:
+        CSRSaved.set(CFI.getRegister());
+        break;
+      case MCCFIInstruction::OpRestore:
+      case MCCFIInstruction::OpUndefined:
+        CSRRestored.set(CFI.getRegister());
+        break;
       case MCCFIInstruction::OpRememberState:
         // TODO: Add support for handling cfi_remember_state.
 #ifndef NDEBUG
@@ -199,12 +221,7 @@ void CFIInstrInserter::calculateOutgoingCFAInfo(MBBCFAInfo &MBBInfo) {
         break;
       // Other CFI directives do not affect CFA value.
       case MCCFIInstruction::OpSameValue:
-      case MCCFIInstruction::OpOffset:
-      case MCCFIInstruction::OpRelOffset:
       case MCCFIInstruction::OpEscape:
-      case MCCFIInstruction::OpRestore:
-      case MCCFIInstruction::OpUndefined:
-      case MCCFIInstruction::OpRegister:
       case MCCFIInstruction::OpWindowSave:
       case MCCFIInstruction::OpNegateRAState:
       case MCCFIInstruction::OpGnuArgsSize:
@@ -218,6 +235,11 @@ void CFIInstrInserter::calculateOutgoingCFAInfo(MBBCFAInfo &MBBInfo) {
   // Update outgoing CFA info.
   MBBInfo.OutgoingCFAOffset = SetOffset;
   MBBInfo.OutgoingCFARegister = SetRegister;
+
+  // Update outgoing CSR info.
+  MBBInfo.OutgoingCSRSaved = MBBInfo.IncomingCSRSaved;
+  MBBInfo.OutgoingCSRSaved |= CSRSaved;
+  MBBInfo.OutgoingCSRSaved.reset(CSRRestored);
 }
 
 void CFIInstrInserter::updateSuccCFAInfo(MBBCFAInfo &MBBInfo) {
@@ -236,6 +258,7 @@ void CFIInstrInserter::updateSuccCFAInfo(MBBCFAInfo &MBBInfo) {
       if (!SuccInfo.Processed) {
         SuccInfo.IncomingCFAOffset = CurrentInfo.OutgoingCFAOffset;
         SuccInfo.IncomingCFARegister = CurrentInfo.OutgoingCFARegister;
+        SuccInfo.IncomingCSRSaved = CurrentInfo.OutgoingCSRSaved;
         Stack.push_back(Succ);
       }
     }
@@ -287,12 +310,23 @@ bool CFIInstrInserter::insertCFIInstrs(MachineFunction &MF) {
           .addCFIIndex(CFIIndex);
       InsertedCFIInstr = true;
     }
+
+    BitVector SetDifference = PrevMBBInfo->OutgoingCSRSaved;
+    SetDifference.reset(MBBInfo.IncomingCSRSaved);
+    for (int Reg : SetDifference.set_bits()) {
+      unsigned CFIIndex =
+          MF.addFrameInst(MCCFIInstruction::createRestore(nullptr, Reg));
+      BuildMI(*MBBInfo.MBB, MBBI, DL, TII->get(TargetOpcode::CFI_INSTRUCTION))
+          .addCFIIndex(CFIIndex);
+      InsertedCFIInstr = true;
+    }
     PrevMBBInfo = &MBBInfo;
   }
   return InsertedCFIInstr;
 }
 
-void CFIInstrInserter::report(const MBBCFAInfo &Pred, const MBBCFAInfo &Succ) {
+void CFIInstrInserter::reportCFAError(const MBBCFAInfo &Pred,
+                                      const MBBCFAInfo &Succ) {
   errs() << "*** Inconsistent CFA register and/or offset between pred and succ "
             "***\n";
   errs() << "Pred: " << Pred.MBB->getName() << " #" << Pred.MBB->getNumber()
@@ -307,6 +341,22 @@ void CFIInstrInserter::report(const MBBCFAInfo &Pred, const MBBCFAInfo &Succ) {
          << " incoming CFA Offset:" << Succ.IncomingCFAOffset << "\n";
 }
 
+void CFIInstrInserter::reportCSRError(const MBBCFAInfo &Pred,
+                                      const MBBCFAInfo &Succ) {
+  errs() << "*** Inconsistent CSR Saved between pred and succ in function "
+         << Pred.MBB->getParent()->getName() << " ***\n";
+  errs() << "Pred: " << Pred.MBB->getName() << " #" << Pred.MBB->getNumber()
+         << " outgoing CSR Saved: ";
+  for (int Reg : Pred.OutgoingCSRSaved.set_bits())
+    errs() << Reg << " ";
+  errs() << "\n";
+  errs() << "Succ: " << Succ.MBB->getName() << " #" << Succ.MBB->getNumber()
+         << " incoming CSR Saved: ";
+  for (int Reg : Succ.IncomingCSRSaved.set_bits())
+    errs() << Reg << " ";
+  errs() << "\n";
+}
+
 unsigned CFIInstrInserter::verify(MachineFunction &MF) {
   unsigned ErrorNum = 0;
   for (auto *CurrMBB : depth_first(&MF)) {
@@ -321,7 +371,13 @@ unsigned CFIInstrInserter::verify(MachineFunction &MF) {
         // we don't generate epilogues inside such blocks.
         if (SuccMBBInfo.MBB->succ_empty() && !SuccMBBInfo.MBB->isReturnBlock())
           continue;
-        report(CurrMBBInfo, SuccMBBInfo);
+        reportCFAError(CurrMBBInfo, SuccMBBInfo);
+        ErrorNum++;
+      }
+      // Check that IncomingCSRSaved of every successor matches the
+      // OutgoingCSRSaved of CurrMBB
+      if (SuccMBBInfo.IncomingCSRSaved != CurrMBBInfo.OutgoingCSRSaved) {
+        reportCSRError(CurrMBBInfo, SuccMBBInfo);
         ErrorNum++;
       }
     }

diff --git a/llvm/lib/Target/X86/X86FrameLowering.cpp b/llvm/lib/Target/X86/X86FrameLowering.cpp
index c8987da..9d7d5b7 100644 (file)
--- a/llvm/lib/Target/X86/X86FrameLowering.cpp
+++ b/llvm/lib/Target/X86/X86FrameLowering.cpp
@@ -486,7 +486,7 @@ void X86FrameLowering::BuildCFI(MachineBasicBlock &MBB,
 
 void X86FrameLowering::emitCalleeSavedFrameMoves(
     MachineBasicBlock &MBB, MachineBasicBlock::iterator MBBI,
-    const DebugLoc &DL) const {
+    const DebugLoc &DL, bool IsPrologue) const {
   MachineFunction &MF = *MBB.getParent();
   MachineFrameInfo &MFI = MF.getFrameInfo();
   MachineModuleInfo &MMI = MF.getMMI();
@@ -501,10 +501,15 @@ void X86FrameLowering::emitCalleeSavedFrameMoves(
          I = CSI.begin(), E = CSI.end(); I != E; ++I) {
     int64_t Offset = MFI.getObjectOffset(I->getFrameIdx());
     unsigned Reg = I->getReg();
-
     unsigned DwarfReg = MRI->getDwarfRegNum(Reg, true);
-    BuildCFI(MBB, MBBI, DL,
-             MCCFIInstruction::createOffset(nullptr, DwarfReg, Offset));
+
+    if (IsPrologue) {
+      BuildCFI(MBB, MBBI, DL,
+               MCCFIInstruction::createOffset(nullptr, DwarfReg, Offset));
+    } else {
+      BuildCFI(MBB, MBBI, DL,
+               MCCFIInstruction::createRestore(nullptr, DwarfReg));
+    }
   }
 }
 
@@ -1675,7 +1680,7 @@ void X86FrameLowering::emitPrologue(MachineFunction &MF,
     }
 
     // Emit DWARF info specifying the offsets of the callee-saved registers.
-    emitCalleeSavedFrameMoves(MBB, MBBI, DL);
+    emitCalleeSavedFrameMoves(MBB, MBBI, DL, true);
   }
 
   // X86 Interrupt handling function cannot assume anything about the direction
@@ -1825,6 +1830,8 @@ void X86FrameLowering::emitEpilogue(MachineFunction &MF,
   }
   uint64_t SEHStackAllocAmt = NumBytes;
 
+  // AfterPop is the position to insert .cfi_restore.
+  MachineBasicBlock::iterator AfterPop = MBBI;
   if (HasFP) {
     // Pop EBP.
     BuildMI(MBB, MBBI, DL, TII.get(Is64Bit ? X86::POP64r : X86::POP32r),
@@ -1835,6 +1842,13 @@ void X86FrameLowering::emitEpilogue(MachineFunction &MF,
           TRI->getDwarfRegNum(Is64Bit ? X86::RSP : X86::ESP, true);
       BuildCFI(MBB, MBBI, DL, MCCFIInstruction::createDefCfa(
                                   nullptr, DwarfStackPtr, -SlotSize));
+      if (!MBB.succ_empty() && !MBB.isReturnBlock()) {
+        unsigned DwarfFramePtr = TRI->getDwarfRegNum(MachineFramePtr, true);
+        BuildCFI(MBB, AfterPop, DL,
+                 MCCFIInstruction::createRestore(nullptr, DwarfFramePtr));
+        --MBBI;
+        --AfterPop;
+      }
       --MBBI;
     }
   }
@@ -1934,6 +1948,13 @@ void X86FrameLowering::emitEpilogue(MachineFunction &MF,
     }
   }
 
+  // Emit DWARF info specifying the restores of the callee-saved registers.
+  // For epilogue with return inside or being other block without successor,
+  // no need to generate .cfi_restore for callee-saved registers.
+  if (NeedsDwarfCFI && !MBB.succ_empty() && !MBB.isReturnBlock()) {
+    emitCalleeSavedFrameMoves(MBB, AfterPop, DL, false);
+  }
+
   if (Terminator == MBB.end() || !isTailCallOpcode(Terminator->getOpcode())) {
     // Add the return addr area delta back since we are not tail calling.
     int Offset = -1 * X86FI->getTCReturnAddrDelta();

diff --git a/llvm/lib/Target/X86/X86FrameLowering.h b/llvm/lib/Target/X86/X86FrameLowering.h
index c7b4154..43c81d9 100644 (file)
--- a/llvm/lib/Target/X86/X86FrameLowering.h
+++ b/llvm/lib/Target/X86/X86FrameLowering.h
@@ -60,7 +60,7 @@ public:
 
   void emitCalleeSavedFrameMoves(MachineBasicBlock &MBB,
                                  MachineBasicBlock::iterator MBBI,
-                                 const DebugLoc &DL) const;
+                                 const DebugLoc &DL, bool IsPrologue) const;
 
   /// emitProlog/emitEpilog - These methods insert prolog and epilog code into
   /// the function.

diff --git a/llvm/test/CodeGen/X86/cfi-epilogue-with-return.mir b/llvm/test/CodeGen/X86/cfi-epilogue-with-return.mir
new file mode 100644 (file)
index 0000000..583e54b
--- /dev/null
+++ b/llvm/test/CodeGen/X86/cfi-epilogue-with-return.mir
@@ -0,0 +1,48 @@
+# RUN: llc -o - %s -mtriple=x86_64-- -run-pass=prologepilog 2>&1 | FileCheck %s
+--- |
+  define i64 @_Z3foob(i1 zeroext %cond) #0 {
+    ret i64 0
+  }
+  attributes #0 = {"frame-pointer"="all"}
+...
+---
+# If the epilogue bb.1 is a return block, no .cfi_restore is
+# needed in it.
+# CHECK:    bb.1:
+# CHECK-NOT:  CFI_INSTRUCTION restore
+# CHECK:      RET 0
+# CHECK:    bb.2:
+# CHECK:      RET 0
+name:            _Z3foob
+alignment:       16
+tracksRegLiveness: true
+liveins:
+  - { reg: '$edi' }
+frameInfo:
+  maxAlignment:    1
+  hasCalls:        true
+  savePoint:       '%bb.1'
+  restorePoint:    '%bb.1'
+machineFunctionInfo: {}
+body:             |
+  bb.0:
+    liveins: $edi
+  
+    TEST8rr renamable $dil, renamable $dil, implicit-def $eflags, implicit killed $edi
+    JCC_1 %bb.2, 4, implicit killed $eflags
+    JMP_1 %bb.1
+  
+  bb.1:
+    renamable $rbx = IMPLICIT_DEF
+    renamable $r14 = IMPLICIT_DEF
+    renamable $r15 = IMPLICIT_DEF
+    renamable $r12 = IMPLICIT_DEF
+    renamable $r13 = IMPLICIT_DEF
+    dead $eax = MOV32r0 implicit-def dead $eflags, implicit-def $rax
+    RET 0, killed $rax
+  
+  bb.2:
+    dead $eax = MOV32r0 implicit-def dead $eflags, implicit-def $rax
+    RET 0, killed $rax
+
+...

diff --git a/llvm/test/CodeGen/X86/cfi-epilogue-without-return.mir b/llvm/test/CodeGen/X86/cfi-epilogue-without-return.mir
new file mode 100644 (file)
index 0000000..8f04721
--- /dev/null
+++ b/llvm/test/CodeGen/X86/cfi-epilogue-without-return.mir
@@ -0,0 +1,53 @@
+# RUN: llc -o - %s -mtriple=x86_64-- -run-pass=prologepilog 2>&1 | FileCheck %s
+--- |
+  declare dso_local void @_Z3goov()
+  define i64 @_Z3foob(i1 zeroext %cond) #0 {
+    ret i64 0
+  }
+  attributes #0 = {"frame-pointer"="all"}
+...
+---
+# If the epilogue bb.1.if.then is not a return block, .cfi_restore is
+# needed in it, otherwise bb.2.return will see different outgoing CFI
+# information from its predecessors.
+# CHECK:    bb.1:
+# CHECK:      CFI_INSTRUCTION restore $rbx
+# CHECK-NEXT: CFI_INSTRUCTION restore $r12
+# CHECK-NEXT: CFI_INSTRUCTION restore $r13
+# CHECK-NEXT: CFI_INSTRUCTION restore $r14
+# CHECK-NEXT: CFI_INSTRUCTION restore $r15
+# CHECK-NEXT: CFI_INSTRUCTION restore $rbp
+# CHECK-NOT:  RET 0
+# CHECK:    bb.2:
+# CHECK:      RET 0
+name:            _Z3foob
+alignment:       16
+tracksRegLiveness: true
+liveins:
+  - { reg: '$edi' }
+frameInfo:
+  maxAlignment:    1
+  hasCalls:        true
+  savePoint:       '%bb.1'
+  restorePoint:    '%bb.1'
+machineFunctionInfo: {}
+body:             |
+  bb.0:
+    liveins: $edi
+  
+    TEST8rr renamable $dil, renamable $dil, implicit-def $eflags, implicit killed $edi
+    JCC_1 %bb.2, 4, implicit killed $eflags
+    JMP_1 %bb.1
+  
+  bb.1:
+    renamable $rbx = IMPLICIT_DEF
+    renamable $r14 = IMPLICIT_DEF
+    renamable $r15 = IMPLICIT_DEF
+    renamable $r12 = IMPLICIT_DEF
+    renamable $r13 = IMPLICIT_DEF
+  
+  bb.2:
+    dead $eax = MOV32r0 implicit-def dead $eflags, implicit-def $rax
+    RET 0, killed $rax
+
+...

diff --git a/llvm/test/CodeGen/X86/cfi-inserter-callee-save-register.mir b/llvm/test/CodeGen/X86/cfi-inserter-callee-save-register.mir
new file mode 100644 (file)
index 0000000..b17c9a6
--- /dev/null
+++ b/llvm/test/CodeGen/X86/cfi-inserter-callee-save-register.mir
@@ -0,0 +1,34 @@
+# RUN: llc -o - %s -mtriple=x86_64-- -verify-cfiinstrs \
+# RUN:     -run-pass=cfi-instr-inserter 2>&1 | FileCheck %s
+# Test that CFI inserter inserts .cfi_restore properly for
+# callee saved registers.
+--- |
+  define void @foo() {
+    ret void
+  }
+...
+---
+# CHECK:      bb.3:
+# CHECK:      CFI_INSTRUCTION restore $rbx
+# CHECK-NEXT: CFI_INSTRUCTION restore $rbp
+name:            foo
+body:             |
+  bb.0:
+    TEST8rr renamable $dil, renamable $dil, implicit-def $eflags, implicit killed $edi
+    JCC_1 %bb.2, 5, implicit killed $eflags
+
+  bb.1:
+    JMP_1 %bb.3
+
+  bb.2:
+    CFI_INSTRUCTION def_cfa_offset 16
+    CFI_INSTRUCTION offset $rbp, -16
+    CFI_INSTRUCTION def_cfa_register $rbp
+    CFI_INSTRUCTION offset $rbx, -24
+    CFI_INSTRUCTION def_cfa $rsp, 8
+    RET 0, $rax
+
+  bb.3:
+    RET 0, $rax
+
+...

diff --git a/llvm/test/CodeGen/X86/cfi-inserter-verify-inconsistent-csr.mir b/llvm/test/CodeGen/X86/cfi-inserter-verify-inconsistent-csr.mir
new file mode 100644 (file)
index 0000000..63957ae
--- /dev/null
+++ b/llvm/test/CodeGen/X86/cfi-inserter-verify-inconsistent-csr.mir
@@ -0,0 +1,28 @@
+# RUN: not --crash llc -o - %s -mtriple=x86_64-- -verify-cfiinstrs \
+# RUN:     -run-pass=cfi-instr-inserter 2>&1 | FileCheck %s
+# Test that CFI verifier finds inconsistent csr saved set between bb.end and
+# one of its precedessors.
+--- |
+  define void @inconsistentCSR() {
+  entry:
+    br label %then
+  then:
+    br label %end
+  end:
+    ret void
+  }
+...
+---
+# CHECK: *** Inconsistent CSR Saved between pred and succ in function inconsistentCSR ***
+# CHECK: LLVM ERROR: Found 1 in/out CFI information errors.
+name: inconsistentCSR
+body: |
+  bb.0.entry:
+    JCC_1 %bb.2, 5, implicit undef $eflags
+
+  bb.1.then:
+    CFI_INSTRUCTION offset $rbp, -16
+
+  bb.2.end:
+    RET 0
+...