[CFIFixup] Fixup CFI for split functions with synchronous uwtables

Commit 6e54fcc disables CFI fixup for
functions with synchronous tables, breaking CFI for split functions.
Instead, we can disable *block-level* CFI fixup for functions with
synchronous tables.

Unwind tables can be:
- N/A (not present)
- Asynchronous
- Synchronous

Functions without unwind tables don't need CFI fixup (since they don't
care about CFI).

Functions with asynchronous unwind tables must be accurate for each
basic block, so full CFI fixup is necessary.

Functions with synchronous unwind tables only need to be accurate for
each function (specifically, the portion of a function in a given
section). Disabling CFI fixup entirely for functions with synchronous
uwtables may break CFI for a function split between two sections. The
portion in the first section may have valid CFI, while the portion in
the second section is missing a call frame.

Ex:
```
(.text.hot)
Foo (BB1):
  <Call frame information>
  ...
BB2:
  ...

(.text.split)
BB3:
  ...
BB4:
  <epilogue>
```

Even if `Foo` has a synchronous unwind table, we still need to insert
call frame information into `BB3` so that unwinding the call stack from
`BB3` or `BB4` works properly.

Author: dhoekwater

llvm/include/llvm/CodeGen/TargetFrameLowering.h

@@ -20,10 +20,10 @@
 #include <vector>
 
 namespace llvm {
-  class BitVector;
-  class CalleeSavedInfo;
-  class MachineFunction;
-  class RegScavenger;
+class BitVector;
+class CalleeSavedInfo;
+class MachineFunction;
+class RegScavenger;
 
 namespace TargetStackID {
 enum Value {
@@ -45,8 +45,8 @@ enum Value {
 class TargetFrameLowering {
 public:
   enum StackDirection {
-    StackGrowsUp,        // Adding to the stack increases the stack address
-    StackGrowsDown       // Adding to the stack decreases the stack address
+    StackGrowsUp,  // Adding to the stack increases the stack address
+    StackGrowsDown // Adding to the stack decreases the stack address
   };
 
   // Maps a callee saved register to a stack slot with a fixed offset.
@@ -78,6 +78,7 @@ class TargetFrameLowering {
   Align TransientStackAlignment;
   int LocalAreaOffset;
   bool StackRealignable;
+
 public:
   TargetFrameLowering(StackDirection D, Align StackAl, int LAO,
                       Align TransAl = Align(1), bool StackReal = true)
@@ -128,9 +129,7 @@ class TargetFrameLowering {
 
   /// isStackRealignable - This method returns whether the stack can be
   /// realigned.
-  bool isStackRealignable() const {
-    return StackRealignable;
-  }
+  bool isStackRealignable() const { return StackRealignable; }
 
   /// This method returns whether or not it is safe for an object with the
   /// given stack id to be bundled into the local area.
@@ -148,8 +147,8 @@ class TargetFrameLowering {
   ///
   /// If this returns true, the frame indexes used by the RegScavenger will be
   /// allocated closest to the incoming stack pointer.
-  virtual bool allocateScavengingFrameIndexesNearIncomingSP(
-    const MachineFunction &MF) const;
+  virtual bool
+  allocateScavengingFrameIndexesNearIncomingSP(const MachineFunction &MF) const;
 
   /// assignCalleeSavedSpillSlots - Allows target to override spill slot
   /// assignment logic.  If implemented, assignCalleeSavedSpillSlots() should
@@ -190,9 +189,7 @@ class TargetFrameLowering {
   /// targetHandlesStackFrameRounding - Returns true if the target is
   /// responsible for rounding up the stack frame (probably at emitPrologue
   /// time).
-  virtual bool targetHandlesStackFrameRounding() const {
-    return false;
-  }
+  virtual bool targetHandlesStackFrameRounding() const { return false; }
 
   /// Returns true if the target will correctly handle shrink wrapping.
   virtual bool enableShrinkWrapping(const MachineFunction &MF) const {
@@ -229,10 +226,16 @@ class TargetFrameLowering {
 
   /// Returns true if we may need to fix the unwind information for the
   /// function.
-  virtual bool enableCFIFixup(MachineFunction &MF) const;
+  virtual bool enableCFIFixup(const MachineFunction &MF) const;
+
+  /// enableBlockLevelCFIFixup - Returns true if we may need to fix up the
+  /// function at a basic block level (e.g. for async unwind tables).
+  virtual bool enableBlockLevelCFIFixup(const MachineFunction &MF) const {
+    return enableCFIFixup(MF);
+  };
 
   /// Emit CFI instructions that recreate the state of the unwind information
-  /// upon fucntion entry.
+  /// upon function entry.
   virtual void resetCFIToInitialState(MachineBasicBlock &MBB) const {}
 
   /// Replace a StackProbe stub (if any) with the actual probe code inline
@@ -352,7 +355,7 @@ class TargetFrameLowering {
   /// Returns the callee-saved registers as computed by determineCalleeSaves
   /// in the BitVector \p SavedRegs.
   virtual void getCalleeSaves(const MachineFunction &MF,
-                                  BitVector &SavedRegs) const;
+                              BitVector &SavedRegs) const;
 
   /// This method determines which of the registers reported by
   /// TargetRegisterInfo::getCalleeSavedRegs() should actually get saved.
@@ -372,9 +375,9 @@ class TargetFrameLowering {
   /// finalized.  Once the frame is finalized, MO_FrameIndex operands are
   /// replaced with direct constants.  This method is optional.
   ///
-  virtual void processFunctionBeforeFrameFinalized(MachineFunction &MF,
-                                             RegScavenger *RS = nullptr) const {
-  }
+  virtual void
+  processFunctionBeforeFrameFinalized(MachineFunction &MF,
+                                      RegScavenger *RS = nullptr) const {}
 
   /// processFunctionBeforeFrameIndicesReplaced - This method is called
   /// immediately before MO_FrameIndex operands are eliminated, but after the
@@ -394,14 +397,12 @@ class TargetFrameLowering {
   /// implemented if using call frame setup/destroy pseudo instructions.
   /// Returns an iterator pointing to the instruction after the replaced one.
   virtual MachineBasicBlock::iterator
-  eliminateCallFramePseudoInstr(MachineFunction &MF,
-                                MachineBasicBlock &MBB,
+  eliminateCallFramePseudoInstr(MachineFunction &MF, MachineBasicBlock &MBB,
                                 MachineBasicBlock::iterator MI) const {
     llvm_unreachable("Call Frame Pseudo Instructions do not exist on this "
                      "target!");
   }
 
-
   /// Order the symbols in the local stack frame.
   /// The list of objects that we want to order is in \p objectsToAllocate as
   /// indices into the MachineFrameInfo. The array can be reordered in any way
@@ -410,8 +411,7 @@ class TargetFrameLowering {
   /// By default, just maintain the original order.
   virtual void
   orderFrameObjects(const MachineFunction &MF,
-                    SmallVectorImpl<int> &objectsToAllocate) const {
-  }
+                    SmallVectorImpl<int> &objectsToAllocate) const {}
 
   /// Check whether or not the given \p MBB can be used as a prologue
   /// for the target.
@@ -457,9 +457,7 @@ class TargetFrameLowering {
   static bool isSafeForNoCSROpt(const Function &F);
 
   /// Check if the no-CSR optimisation is profitable for the given function.
-  virtual bool isProfitableForNoCSROpt(const Function &F) const {
-    return true;
-  }
+  virtual bool isProfitableForNoCSROpt(const Function &F) const { return true; }
 
   /// Return initial CFA offset value i.e. the one valid at the beginning of the
   /// function (before any stack operations).
@@ -486,6 +484,6 @@ class TargetFrameLowering {
   virtual bool hasFPImpl(const MachineFunction &MF) const = 0;
 };
 
-} // End llvm namespace
+} // namespace llvm
 
 #endif

llvm/lib/CodeGen/CFIFixup.cpp

@@ -80,6 +80,7 @@
 #include "llvm/CodeGen/TargetSubtargetInfo.h"
 #include "llvm/MC/MCAsmInfo.h"
 #include "llvm/MC/MCDwarf.h"
+#include "llvm/Support/CodeGen.h"
 #include "llvm/Target/TargetMachine.h"
 
 #include <iterator>
@@ -249,6 +250,10 @@ fixupBlock(MachineBasicBlock &CurrBB, const BlockFlagsVector &BlockInfo,
   if (!Info.Reachable)
     return false;
 
+  // If we don't need to fix up CFI at the block level, we only need to fix up the first basic block in the section.
+  if (!TFL.enableBlockLevelCFIFixup(MF) && !CurrBB.isBeginSection())
+    return false;
+
   // If the previous block and the current block are in the same section,
   // the frame info will propagate from the previous block to the current one.
   const BlockFlags &PrevInfo =

llvm/lib/CodeGen/TargetFrameLoweringImpl.cpp

@@ -35,7 +35,7 @@ bool TargetFrameLowering::enableCalleeSaveSkip(const MachineFunction &MF) const
   return false;
 }
 
-bool TargetFrameLowering::enableCFIFixup(MachineFunction &MF) const {
+bool TargetFrameLowering::enableCFIFixup(const MachineFunction &MF) const {
   return MF.needsFrameMoves() &&
          !MF.getTarget().getMCAsmInfo()->usesWindowsCFI();
 }

llvm/lib/Target/AArch64/AArch64FrameLowering.cpp

@@ -2609,8 +2609,14 @@ void AArch64FrameLowering::emitEpilogue(MachineFunction &MF,
   }
 }
 
-bool AArch64FrameLowering::enableCFIFixup(MachineFunction &MF) const {
+bool AArch64FrameLowering::enableCFIFixup(const MachineFunction &MF) const {
   return TargetFrameLowering::enableCFIFixup(MF) &&
+         MF.getInfo<AArch64FunctionInfo>()->needsDwarfUnwindInfo(MF);
+}
+
+bool AArch64FrameLowering::enableBlockLevelCFIFixup(
+    const MachineFunction &MF) const {
+  return enableCFIFixup(MF) &&
          MF.getInfo<AArch64FunctionInfo>()->needsAsyncDwarfUnwindInfo(MF);
 }
 

llvm/lib/Target/AArch64/AArch64FrameLowering.h

@@ -36,7 +36,9 @@ class AArch64FrameLowering : public TargetFrameLowering {
   void emitPrologue(MachineFunction &MF, MachineBasicBlock &MBB) const override;
   void emitEpilogue(MachineFunction &MF, MachineBasicBlock &MBB) const override;
 
-  bool enableCFIFixup(MachineFunction &MF) const override;
+  bool enableCFIFixup(const MachineFunction &MF) const override;
+
+  bool enableBlockLevelCFIFixup(const MachineFunction &MF) const override;
 
   bool canUseAsPrologue(const MachineBasicBlock &MBB) const override;
 

llvm/test/CodeGen/AArch64/cfi-fixup-multi-section.mir

@@ -285,6 +285,10 @@ body:             |
   ; CHECK-NEXT:   successors: %bb.5(0x80000000)
   ; CHECK-NEXT:   liveins: $w0
   ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   frame-setup CFI_INSTRUCTION escape 0x16, 0x12, 0x02, 0x82, 0x78
+  ; CHECK-NEXT:   frame-setup CFI_INSTRUCTION negate_ra_sign_state
+  ; CHECK-NEXT:   frame-setup CFI_INSTRUCTION def_cfa_offset 16
+  ; CHECK-NEXT:   frame-setup CFI_INSTRUCTION offset $w30, -16
   ; CHECK-NEXT:   renamable $w0 = nsw SUBWri killed renamable $w0, 1, 0
   ; CHECK-NEXT:   BL @g, csr_aarch64_aapcs_scs, implicit-def dead $lr, implicit $sp, implicit killed $w0, implicit-def $sp, implicit-def $w0
   ; CHECK-NEXT:   renamable $w0 = nsw ADDWri killed renamable $w0, 1, 0