[ARM][MachineOutliner] Emit more CFI instructions

[chill]

Summary

This patch make the outliner emit CFI instructions in a few more places:

• after LR is restored, but before the return in an outlined function
• around save/restore of LR to/from a register at calls to outlined functions
• around save/restore of LR to/from the stack at calls to outlined functions

The latter two only when the function does NOT spill LR. If the function spills
LR, then outliner generated saves/restores around calls are not considered
interesting for unwinding the frame.

[chill]

llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp

6109

Need to fix the sign here and elsewhere ...

[chill]

Ping.

[chill]

Ping.

[danielkiss]

llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp

6155

I would generate a .cfi_restore lr here for the first sight.
The LR in the caller is saved on the stack probably so here the value of the LR is different because that is set by the bl.

[chill]

llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp

6155

Thanks, yes, that's right, "same value" is wrong here.

[yroux]

Just a small comment, otherwise LGTM, thanks

llvm/lib/Target/ARM/ARMBaseInstrInfo.h

393

looks like a typo

[chill]

Thanks!

llvm/lib/Target/ARM/ARMBaseInstrInfo.h

 private:
   /// Returns an unused general-purpose register which can be used for
   /// constructing an outlined call if one exists. Returns 0 otherwise.
   unsigned findRegisterToSaveLRTo(const outliner::Candidate &C) const;
 
   // Adds an instruction which saves the link register on top of the stack into
   /// the MachineBasicBlock \p MBB at position \p It.
   void saveLROnStack(MachineBasicBlock &MBB,
-                     MachineBasicBlock::iterator &It) const;
+                     MachineBasicBlock::iterator It) const;
 
   /// Adds an instruction which restores the link register from the top the
   /// stack into the MachineBasicBlock \p MBB at position \p It.
   void restoreLRFromStack(MachineBasicBlock &MBB,
-                          MachineBasicBlock::iterator &It) const;
+                          MachineBasicBlock::iterator It) const;
+
+  /// Emit CFI instructions into the MachineBasicBlock \p MBB at poision \p It,
+  /// for the case when the LR is saved on the stack.
+  void emitCFIForLRSaveOnStack(MachineBasicBlock &MBB,
+                               MachineBasicBlock::iterator It) const;
+
+  /// Emit CFI instructions into the MachineBasicBlock \p MBB at poision \p It,
+  /// for the case when the LR is saved in the register \p Reg.
+  void emitCFIForLRSaveToReg(MachineBasicBlock &MBB,
+                             MachineBasicBlock::iterator It,
+                             Register Reg) const;
+
+  /// Emit CFI instructions into the MachineBasicBlock \p MBB at poision \p It,
+  /// after the LR is was restored from the stack.
+  void emitCFIForLRRestoreFromStack(MachineBasicBlock &MBB,
+                                    MachineBasicBlock::iterator It) const;
+
+  /// Emit CFI instructions into the MachineBasicBlock \p MBB at poision \p It,
+  /// after the LR is was restored from a register.
+  void emitCFIForLRRestoreFromReg(MachineBasicBlock &MBB,
+                                  MachineBasicBlock::iterator It) const;
 
   unsigned getInstBundleLength(const MachineInstr &MI) const;
 
   int getVLDMDefCycle(const InstrItineraryData *ItinData,
                       const MCInstrDesc &DefMCID,
                       unsigned DefClass,
                       unsigned DefIdx, unsigned DefAlign) const;
   int getLDMDefCycle(const InstrItineraryData *ItinData,

llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp

ARMBaseInstrInfo::getOutliningType(MachineBasicBlock::iterator &MIT,
   // Don't outline positions.
   if (MI.isPosition())
     return outliner::InstrType::Illegal;
 
   return outliner::InstrType::Legal;
 }
 
 void ARMBaseInstrInfo::saveLROnStack(MachineBasicBlock &MBB,
-                                     MachineBasicBlock::iterator &It) const {
+                                     MachineBasicBlock::iterator It) const {
   unsigned Opc = Subtarget.isThumb() ? ARM::t2STR_PRE : ARM::STR_PRE_IMM;
   int Align = -Subtarget.getStackAlignment().value();
   BuildMI(MBB, It, DebugLoc(), get(Opc), ARM::SP)
     .addReg(ARM::LR, RegState::Kill)
     .addReg(ARM::SP)
     .addImm(Align)
     .add(predOps(ARMCC::AL));
 }
 
+void ARMBaseInstrInfo::emitCFIForLRSaveOnStack(
+    MachineBasicBlock &MBB, MachineBasicBlock::iterator It) const {
+  MachineFunction &MF = *MBB.getParent();
+  const MCRegisterInfo *MRI = Subtarget.getRegisterInfo();
+  unsigned DwarfLR = MRI->getDwarfRegNum(ARM::LR, true);
+  int Align = Subtarget.getStackAlignment().value();
+  // Add a CFI saying the stack was moved down.
+  int64_t StackPosEntry =
+      MF.addFrameInst(MCCFIInstruction::cfiDefCfaOffset(nullptr, Align));
+  BuildMI(MBB, It, DebugLoc(), get(ARM::CFI_INSTRUCTION))
+      .addCFIIndex(StackPosEntry)
+      .setMIFlags(MachineInstr::FrameSetup);
+
+  // Add a CFI saying that the LR that we want to find is now higher than
+  // before.
+  int64_t LRPosEntry =
+      MF.addFrameInst(MCCFIInstruction::createOffset(nullptr, DwarfLR, Align));
+  BuildMI(MBB, It, DebugLoc(), get(ARM::CFI_INSTRUCTION))
+      .addCFIIndex(LRPosEntry)
+      .setMIFlags(MachineInstr::FrameSetup);
+}
+
+void ARMBaseInstrInfo::emitCFIForLRSaveToReg(MachineBasicBlock &MBB,
+                                             MachineBasicBlock::iterator It,
+                                             Register Reg) const {
+  MachineFunction &MF = *MBB.getParent();
+  const MCRegisterInfo *MRI = Subtarget.getRegisterInfo();
+  unsigned DwarfLR = MRI->getDwarfRegNum(ARM::LR, true);
+  unsigned DwarfReg = MRI->getDwarfRegNum(Reg, true);
+
+  int64_t LRPosEntry = MF.addFrameInst(
+      MCCFIInstruction::createRegister(nullptr, DwarfLR, DwarfReg));
+  BuildMI(MBB, It, DebugLoc(), get(ARM::CFI_INSTRUCTION))
+      .addCFIIndex(LRPosEntry)
+      .setMIFlags(MachineInstr::FrameSetup);
+}
+
 void ARMBaseInstrInfo::restoreLRFromStack(
-  MachineBasicBlock &MBB, MachineBasicBlock::iterator &It) const {
+    MachineBasicBlock &MBB, MachineBasicBlock::iterator It) const {
   unsigned Opc = Subtarget.isThumb() ? ARM::t2LDR_POST : ARM::LDR_POST_IMM;
   MachineInstrBuilder MIB = BuildMI(MBB, It, DebugLoc(), get(Opc), ARM::LR)
     .addReg(ARM::SP, RegState::Define)
     .addReg(ARM::SP);
   if (!Subtarget.isThumb())
     MIB.addReg(0);
   MIB.addImm(Subtarget.getStackAlignment().value()).add(predOps(ARMCC::AL));
 }
 
+void ARMBaseInstrInfo::emitCFIForLRRestoreFromStack(
+    MachineBasicBlock &MBB, MachineBasicBlock::iterator It) const {
+  // Now stack has moved back up...
+  MachineFunction &MF = *MBB.getParent();
+  const MCRegisterInfo *MRI = Subtarget.getRegisterInfo();
+  unsigned DwarfLR = MRI->getDwarfRegNum(ARM::LR, true);
+  int64_t StackPosEntry =
+      MF.addFrameInst(MCCFIInstruction::cfiDefCfaOffset(nullptr, 0));
+  BuildMI(MBB, It, DebugLoc(), get(ARM::CFI_INSTRUCTION))
+      .addCFIIndex(StackPosEntry)
+      .setMIFlags(MachineInstr::FrameDestroy);
+
+  // ... and LR is the same as in caller.
+  int64_t LRPosEntry =
+      MF.addFrameInst(MCCFIInstruction::createSameValue(nullptr, DwarfLR));
+  BuildMI(MBB, It, DebugLoc(), get(ARM::CFI_INSTRUCTION))
+      .addCFIIndex(LRPosEntry)
+      .setMIFlags(MachineInstr::FrameDestroy);
+}
+
+void ARMBaseInstrInfo::emitCFIForLRRestoreFromReg(
+    MachineBasicBlock &MBB, MachineBasicBlock::iterator It) const {
+  MachineFunction &MF = *MBB.getParent();
+  const MCRegisterInfo *MRI = Subtarget.getRegisterInfo();
+  unsigned DwarfLR = MRI->getDwarfRegNum(ARM::LR, true);
+
+  int64_t LRPosEntry =
+      MF.addFrameInst(MCCFIInstruction::createSameValue(nullptr, DwarfLR));
+  BuildMI(MBB, It, DebugLoc(), get(ARM::CFI_INSTRUCTION))
+      .addCFIIndex(LRPosEntry)
+      .setMIFlags(MachineInstr::FrameDestroy);
+}
+
 void ARMBaseInstrInfo::buildOutlinedFrame(
     MachineBasicBlock &MBB, MachineFunction &MF,
     const outliner::OutlinedFunction &OF) const {
   // For thunk outlining, rewrite the last instruction from a call to a
   // tail-call.
   if (OF.FrameConstructionID == MachineOutlinerThunk) {
     MachineInstr *Call = &*--MBB.instr_end();
     bool isThumb = Subtarget.isThumb();
if (std::any_of(MBB.instr_begin(), MBB.instr_end(), IsNonTailCall)) {
     // We have to save and restore LR, we need to add it to the liveins if it
     // is not already part of the set.  This is suffient since outlined
     // functions only have one block.
     if (!MBB.isLiveIn(ARM::LR))
       MBB.addLiveIn(ARM::LR);
 
     // Insert a save before the outlined region
     saveLROnStack(MBB, It);
-
-    unsigned StackAlignment = Subtarget.getStackAlignment().value();
-    const TargetSubtargetInfo &STI = MF.getSubtarget();
-    const MCRegisterInfo *MRI = STI.getRegisterInfo();
-    unsigned DwarfReg = MRI->getDwarfRegNum(ARM::LR, true);
-    // Add a CFI saying the stack was moved down.
-    int64_t StackPosEntry = MF.addFrameInst(
-        MCCFIInstruction::cfiDefCfaOffset(nullptr, StackAlignment));
-    BuildMI(MBB, It, DebugLoc(), get(ARM::CFI_INSTRUCTION))
-        .addCFIIndex(StackPosEntry)
-        .setMIFlags(MachineInstr::FrameSetup);
-
-    // Add a CFI saying that the LR that we want to find is now higher than
-    // before.
-    int64_t LRPosEntry = MF.addFrameInst(
-        MCCFIInstruction::createOffset(nullptr, DwarfReg, StackAlignment));
-    BuildMI(MBB, It, DebugLoc(), get(ARM::CFI_INSTRUCTION))
-        .addCFIIndex(LRPosEntry)
-        .setMIFlags(MachineInstr::FrameSetup);
+    emitCFIForLRSaveOnStack(MBB, It);
 
     // Insert a restore before the terminator for the function.  Restore LR.
     restoreLRFromStack(MBB, Et);
+    emitCFIForLRRestoreFromStack(MBB, Et);
   }
 
   // If this is a tail call outlined function, then there's already a return.
   if (OF.FrameConstructionID == MachineOutlinerTailCall ||
       OF.FrameConstructionID == MachineOutlinerThunk)
     return;
 
   // Here we have to insert the return ourselves.  Get the correct opcode from
MachineBasicBlock::iterator ARMBaseInstrInfo::insertOutlinedCall(
 
   if (C.CallConstructionID == MachineOutlinerNoLRSave ||
       C.CallConstructionID == MachineOutlinerThunk) {
     // No, so just insert the call.
     It = MBB.insert(It, CallMIB);
     return It;
   }
 
+  const ARMFunctionInfo &AFI = *C.getMF()->getInfo<ARMFunctionInfo>();
   // Can we save to a register?
   if (C.CallConstructionID == MachineOutlinerRegSave) {
     unsigned Reg = findRegisterToSaveLRTo(C);
     assert(Reg != 0 && "No callee-saved register available?");
 
     // Save and restore LR from that register.
     copyPhysReg(MBB, It, DebugLoc(), Reg, ARM::LR, true);
+    if (!AFI.isLRSpilled())
+      emitCFIForLRSaveToReg(MBB, It, Reg);
     CallPt = MBB.insert(It, CallMIB);
     copyPhysReg(MBB, It, DebugLoc(), ARM::LR, Reg, true);
+    if (!AFI.isLRSpilled())
+      emitCFIForLRRestoreFromReg(MBB, It);
     It--;
     return CallPt;
   }
   // We have the default case. Save and restore from SP.
   saveLROnStack(MBB, It);
+  if (!AFI.isLRSpilled())
+    emitCFIForLRSaveOnStack(MBB, It);
   CallPt = MBB.insert(It, CallMIB);
   restoreLRFromStack(MBB, It);
+  if (!AFI.isLRSpilled())
+    emitCFIForLRRestoreFromStack(MBB, It);
   It--;
   return CallPt;
 }
 
 bool ARMBaseInstrInfo::shouldOutlineFromFunctionByDefault(
     MachineFunction &MF) const {
   return Subtarget.isMClass() && MF.getFunction().hasMinSize();
 }

llvm/test/CodeGen/ARM/machine-outliner-cfi-1.ll

+; RUN: llc --verify-machineinstrs --force-dwarf-frame-section %s -o - | FileCheck %s
+target datalayout = "e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64"
+target triple = "thumbv7m-unknown-unknown-eabi"
+
+; Derived from
+; volatile int a, b, c, d, e, f, g, h;
+;
+; int x() {
+;   int r = (a + b) / (c + d) + e + f;
+;   return r + 1;
+; }
+;
+;
+; int y() {
+;   int r = (a + b) / (c + d) + e + f;
+;   return r + 2;
+; }
+; Checks that CFI instruction are emitted around saves/restores of
+; the LR on stack.
+
+@a = dso_local global i32 0, align 4
+@b = dso_local global i32 0, align 4
+@c = dso_local global i32 0, align 4
+@d = dso_local global i32 0, align 4
+@e = dso_local global i32 0, align 4
+@f = dso_local global i32 0, align 4
+
+define dso_local i32 @x() local_unnamed_addr #0 {
+entry:
+  %0 = load volatile i32, i32* @a, align 4
+  %1 = load volatile i32, i32* @b, align 4
+  %add = add nsw i32 %1, %0
+  %2 = load volatile i32, i32* @c, align 4
+  %3 = load volatile i32, i32* @d, align 4
+  %add1 = add nsw i32 %3, %2
+  %div = sdiv i32 %add, %add1
+  %4 = load volatile i32, i32* @e, align 4
+  %5 = load volatile i32, i32* @f, align 4
+  %add2 = add i32 %div, 1
+  %add3 = add i32 %add2, %4
+  %add4 = add i32 %add3, %5
+  ret i32 %add4
+}
+; CHECK-LABEL: x:
+; CHECK:       str lr, [sp, #-8]!
+; CHECK-NEXT:  .cfi_def_cfa_offset 8
+; CHECK-NEXT:  .cfi_offset lr, 8
+; CHECK-NEXT:  bl  OUTLINED_FUNCTION_0
+; CHECK-NEXT:  ldr lr, [sp], #8
+; CHECK-NEXT:  .cfi_def_cfa_offset 0
+; CHECK-NEXT:  .cfi_same_value lr
+
+define dso_local i32 @y() local_unnamed_addr #0 {
+entry:
+  %0 = load volatile i32, i32* @a, align 4
+  %1 = load volatile i32, i32* @b, align 4
+  %add = add nsw i32 %1, %0
+  %2 = load volatile i32, i32* @c, align 4
+  %3 = load volatile i32, i32* @d, align 4
+  %add1 = add nsw i32 %3, %2
+  %div = sdiv i32 %add, %add1
+  %4 = load volatile i32, i32* @e, align 4
+  %5 = load volatile i32, i32* @f, align 4
+  %add2 = add i32 %div, 2
+  %add3 = add i32 %add2, %4
+  %add4 = add i32 %add3, %5
+  ret i32 %add4
+}
+; CHECK-LABEL: y:
+; CHECK:       str lr, [sp, #-8]!
+; CHECK-NEXT:  .cfi_def_cfa_offset 8
+; CHECK-NEXT:  .cfi_offset lr, 8
+; CHECK-NEXT:  bl  OUTLINED_FUNCTION_0
+; CHECK-NEXT:  ldr lr, [sp], #8
+; CHECK-NEXT:  .cfi_def_cfa_offset 0
+; CHECK-NEXT:  .cfi_same_value lr
+
+attributes #0 = { minsize nofree norecurse nounwind optsize }

llvm/test/CodeGen/ARM/machine-outliner-cfi-2.ll

+; RUN: llc --verify-machineinstrs --force-dwarf-frame-section %s -o - | FileCheck %s
+target datalayout = "e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64"
+target triple = "thumbv7m-unknown-unknown-eabi"
+
+; Derived from
+; volatile int a, b, c, d, e, f, g, h;
+;
+; int x() {
+;   int r = a + b + c + d + e + f;
+;   return r + 1;
+; }
+;
+;
+; int y() {
+;   int r = a + b + c + d + e + f;
+;   return r + 2;
+; }
+; Check CFI instructions when LR is saved/restored to/from a register.
+
+@a = dso_local global i32 0, align 4
+@b = dso_local global i32 0, align 4
+@c = dso_local global i32 0, align 4
+@d = dso_local global i32 0, align 4
+@e = dso_local global i32 0, align 4
+@f = dso_local global i32 0, align 4
+
+define dso_local i32 @x() local_unnamed_addr #0 {
+entry:
+  %0 = load volatile i32, i32* @a, align 4
+  %1 = load volatile i32, i32* @b, align 4
+  %2 = load volatile i32, i32* @c, align 4
+  %3 = load volatile i32, i32* @d, align 4
+  %4 = load volatile i32, i32* @e, align 4
+  %5 = load volatile i32, i32* @f, align 4
+  %add = add i32 %0, 1
+  %add1 = add i32 %add, %1
+  %add2 = add i32 %add1, %2
+  %add3 = add i32 %add2, %3
+  %add4 = add i32 %add3, %4
+  %add5 = add i32 %add4, %5
+  ret i32 %add5
+}
+; CHECK-LABEL: x:
+; CHECK:      mov r3, lr
+; CHECK-NEXT: .cfi_register lr, r3
+; CHECK-NEXT: bl  OUTLINED_FUNCTION_0
+; CHECK-NEXT: mov lr, r3
+; CHECK-NEXT: .cfi_same_value lr
+
+define dso_local i32 @y() local_unnamed_addr #0 {
+entry:
+  %0 = load volatile i32, i32* @a, align 4
+  %1 = load volatile i32, i32* @b, align 4
+  %2 = load volatile i32, i32* @c, align 4
+  %3 = load volatile i32, i32* @d, align 4
+  %4 = load volatile i32, i32* @e, align 4
+  %5 = load volatile i32, i32* @f, align 4
+  %add = add i32 %0, 2
+  %add1 = add i32 %add, %1
+  %add2 = add i32 %add1, %2
+  %add3 = add i32 %add2, %3
+  %add4 = add i32 %add3, %4
+  %add5 = add i32 %add4, %5
+  ret i32 %add5
+}
+; CHECK-LABEL: y:
+; CHECK:      mov r3, lr
+; CHECK-NEXT: .cfi_register lr, r3
+; CHECK-NEXT: bl  OUTLINED_FUNCTION_0
+; CHECK-NEXT: mov lr, r3
+; CHECK-NEXT: .cfi_same_value lr
+
+attributes #0 = { minsize nofree norecurse nounwind optsize  }

llvm/test/CodeGen/ARM/machine-outliner-cfi-3.ll

+; RUN: llc --verify-machineinstrs --force-dwarf-frame-section %s -o - | FileCheck %s
+target datalayout = "e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64"
+target triple = "thumbv7m-unknown-unknown-eabi"
+
+; Derived from
+; __attribute__((noinline)) int h(int a, int b) { return a + b; }
+;
+; int f(int a, int b, int c, int d) {
+;     if (a < 0)
+;         return -1;
+;     a = h(a, b);
+;     return 2 + a * (a + b) / (c + d);
+; }
+;
+; int g(int a, int b, int c, int d) {
+;     if (a < 0)
+;         return -1;
+;     a = h(a, b);
+;     return 1 + a * (a + b) / (c + d);
+; }
+; Check CFI instructions inside the outlined function.
+
+define dso_local i32 @h(i32 %a, i32 %b) local_unnamed_addr #0 {
+entry:
+  %add = add nsw i32 %b, %a
+  ret i32 %add
+}
+
+define dso_local i32 @f(i32 %a, i32 %b, i32 %c, i32 %d) local_unnamed_addr #1 {
+entry:
+  %cmp = icmp slt i32 %a, 0
+  br i1 %cmp, label %return, label %if.end
+
+if.end:                                           ; preds = %entry
+  %call = tail call i32 @h(i32 %a, i32 %b) #2
+  %add = add nsw i32 %call, %b
+  %mul = mul nsw i32 %add, %call
+  %add1 = add nsw i32 %d, %c
+  %div = sdiv i32 %mul, %add1
+  %add2 = add nsw i32 %div, 2
+  br label %return
+
+return:                                           ; preds = %entry, %if.end
+  %retval.0 = phi i32 [ %add2, %if.end ], [ -1, %entry ]
+  ret i32 %retval.0
+}
+
+define dso_local i32 @g(i32 %a, i32 %b, i32 %c, i32 %d) local_unnamed_addr #1 {
+entry:
+  %cmp = icmp slt i32 %a, 0
+  br i1 %cmp, label %return, label %if.end
+
+if.end:                                           ; preds = %entry
+  %call = tail call i32 @h(i32 %a, i32 %b) #2
+  %add = add nsw i32 %call, %b
+  %mul = mul nsw i32 %add, %call
+  %add1 = add nsw i32 %d, %c
+  %div = sdiv i32 %mul, %add1
+  %add2 = add nsw i32 %div, 1
+  br label %return
+
+return:                                           ; preds = %entry, %if.end
+  %retval.0 = phi i32 [ %add2, %if.end ], [ -1, %entry ]
+  ret i32 %retval.0
+}
+
+; CHECK-LABEL: OUTLINED_FUNCTION_0:
+; CHECK:      str lr, [sp, #-8]!
+; CHECK-NEXT: .cfi_def_cfa_offset 8
+; CHECK-NEXT: .cfi_offset lr, 8
+; CHECK:      ldr lr, [sp], #8
+; CHECK-NEXT: .cfi_def_cfa_offset 0
+; CHECK-NEXT: .cfi_same_value lr
+; CHECK-NEXT: bx  lr
+
+attributes #0 = { minsize noinline norecurse nounwind optsize readnone }
+attributes #1 = { minsize norecurse nounwind optsize readnone }
+attributes #2 = { minsize optsize }