[AArch64] Enable CFIInstrInserter

[MaskRay]

Summary

A basic block following an exit basic block (with ret) generally has
incorrect CFI.
CFIInstrInserter introduced by Correct dwarf unwind information in function epilogue can fix the problem.
I have skepticism whether the fix-up approach is the best, but currently appears
to be the only realistic approach to fix a basic block with epilogue and a basic
block following an epilogue, before we come up with a better scheme.

[SjoerdMeijer]

Just wanted to double check because it is not mentioned in the description, this is addressing https://bugs.llvm.org/show_bug.cgi?id=47142 ?

If this is how X86 works around this issue, then this patch seems trivial and not controversial, but perhaps someone else can comment.

[MaskRay]

Ping:)

[chill]

I would request to not enable CFIInstrInserter for AArch64. I have a patch (that I was planning to submit tomorrow, 2021-11-08), that is
IMHO, for a lack of better word, better.

Until I get aroun d to submit it, here's the commit message, describing it:

[CodeGen] Async unwind (7/7) - add a pass to fix CFI information
 
 This pass inserts the necessary `.cfi_remember_state` and
 `.cfi_restore_state` CFI instructions to adjust for the inconsistency
 of the call-frame information caused by final machine basic block
 layout.
 
 Unlike the `CFIInstrInserer` pass, this one uses emits only
 `.cfi_remember_state`/`.cfi_restore_state`, which results in smaller
 unwind tables and also transparently handles custom unwind info
 extensions, CFA offset adjustement and save locations of SVE
 registers.
 
 The pass takes advantage on the contraints LLVM imposes on the
 placement of save/restore points (cf. `ShrinkWrap.cpp`):
 
 * there is a single basic block, containing the function prologue
 
 * possibly multiple epilogue blocks, where each epilogue block is
   complete and self-contained, i.e. CSR restore instructions (and the
   corresponding CFI instructions are not split across two or more
   blocks.
 
 * prologue and epilogue blocks are outside of any loops
 
 Thus at the beginning and at the end of each basic block we can be in
 one of two states - "has frame" or "does not have frame", which can be
 computed by a single RPO traversal.
 
 Then, for each epilogue block (one that transitions from "has frame"
 to "does not have frame"), which is physically followed by a block
 with a "has frame", we insert a `.cfi_remember_state`.
 
 Similarly, for each block thas "has frame", but is preceded an
 epilogue block, we insert a `.cfi_restore_state".
 
 A block may need to have both instructions, in which case the restore
 one has to precede the remember one.

[xgupta]

(@chill, I am also waiting for your patch as these two CFI directives are needed to fix the stack unwinding issue with shrink wrapping in RISCV)

[chill]

In D109254#3148338, @xgupta wrote:

(@chill, I am also waiting for your patch as these two CFI directives are needed to fix the stack unwinding issue with shrink wrapping in RISCV)

Noted. Unfortunately, the initial algorithm (as described above) was wrong. I've come up with another one I'm working on it right now.

[xgupta]

In D109254#3148360, @chill wrote:

In D109254#3148338, @xgupta wrote:

(@chill, I am also waiting for your patch as these two CFI directives are needed to fix the stack unwinding issue with shrink wrapping in RISCV)

Noted. Unfortunately, the initial algorithm (as described above) was wrong. I've come up with another one I'm working on it right now.

Sure no hurry, Happy to help in reviewing if I can.

llvm/lib/Target/AArch64/AArch64FrameLowering.h

bool isStackIdSafeForLocalArea(unsigned StackId) const override {
     // frame block at the moment.
     return StackId != TargetStackID::ScalableVector;
   }
 
   void
   orderFrameObjects(const MachineFunction &MF,
                     SmallVectorImpl<int> &ObjectsToAllocate) const override;
 
+  int getInitialCFAOffset(const MachineFunction &MF) const override {
+    return 0;
+  }
+  Register getInitialCFARegister(const MachineFunction &MF) const override;
+
 private:
   /// Returns true if a homogeneous prolog or epilog code can be emitted
   /// for the size optimization. If so, HOM_Prolog/HOM_Epilog pseudo
   /// instructions are emitted in place. When Exit block is given, this check is
   /// for epilog.
   bool homogeneousPrologEpilog(MachineFunction &MF,
                                MachineBasicBlock *Exit = nullptr) const;
 

llvm/lib/Target/AArch64/AArch64FrameLowering.cpp

if (IsTailCallReturn) {
     // LowerFormalArguments. This will, of course, be zero for the C calling
     // convention.
     ArgumentPopSize = AFI->getArgumentStackToRestore();
   }
 
   return ArgumentPopSize;
 }
 
+Register
+AArch64FrameLowering::getInitialCFARegister(const MachineFunction &MF) const {
+  return MF.getSubtarget().getRegisterInfo()->getDwarfRegNum(AArch64::SP, true);
+}
+
 static bool produceCompactUnwindFrame(MachineFunction &MF);
 static bool needsWinCFI(const MachineFunction &MF);
 static StackOffset getSVEStackSize(const MachineFunction &MF);
 
 /// Returns true if a homogeneous prolog or epilog code can be emitted
 /// for the size optimization. If possible, a frame helper call is injected.
 /// When Exit block is given, this check is for epilog.
 bool AArch64FrameLowering::homogeneousPrologEpilog(

llvm/lib/Target/AArch64/AArch64TargetMachine.cpp

if (TM->getOptLevel() != CodeGenOpt::None && EnableCollectLOH &&
       TM->getTargetTriple().isOSBinFormatMachO())
     addPass(createAArch64CollectLOHPass());
 }
 
 void AArch64PassConfig::addPreEmitPass2() {
   // SVE bundles move prefixes with destructive operations. BLR_RVMARKER pseudo
   // instructions are lowered to bundles as well.
   addPass(createUnpackMachineBundles(nullptr));
+
+  const Triple &TT = TM->getTargetTriple();
+  if (TT.isOSBinFormatELF())
+    addPass(createCFIInstrInserter());
 }
 
 yaml::MachineFunctionInfo *
 AArch64TargetMachine::createDefaultFuncInfoYAML() const {
   return new yaml::AArch64FunctionInfo();
 }
 
 yaml::MachineFunctionInfo *

llvm/test/CodeGen/AArch64/O0-pipeline.ll

 ; CHECK-NEXT:       Insert XRay ops
 ; CHECK-NEXT:       Implement the 'patchable-function' attribute
 ; CHECK-NEXT:       AArch64 Branch Targets
 ; CHECK-NEXT:       Branch relaxation pass
 ; CHECK-NEXT:       Contiguously Lay Out Funclets
 ; CHECK-NEXT:       StackMap Liveness Analysis
 ; CHECK-NEXT:       Live DEBUG_VALUE analysis
 ; CHECK-NEXT:       Unpack machine instruction bundles
+; CHECK-NEXT:       Check CFA info and insert CFI instructions if needed
 ; CHECK-NEXT:       Lazy Machine Block Frequency Analysis
 ; CHECK-NEXT:       Machine Optimization Remark Emitter
 ; CHECK-NEXT:       AArch64 Assembly Printer
 ; CHECK-NEXT:       Free MachineFunction
 
 define void @f() {
   ret void
 }

llvm/test/CodeGen/AArch64/O3-pipeline.ll

 ; CHECK-NEXT:       Branch relaxation pass
 ; CHECK-NEXT:       AArch64 Compress Jump Tables
 ; CHECK-NEXT:       Contiguously Lay Out Funclets
 ; CHECK-NEXT:       StackMap Liveness Analysis
 ; CHECK-NEXT:       Live DEBUG_VALUE analysis
 ; CHECK-NEXT:     Machine Outliner
 ; CHECK-NEXT:     FunctionPass Manager
 ; CHECK-NEXT:       Unpack machine instruction bundles
+; CHECK-NEXT:       Check CFA info and insert CFI instructions if needed
 ; CHECK-NEXT:       Lazy Machine Block Frequency Analysis
 ; CHECK-NEXT:       Machine Optimization Remark Emitter
 ; CHECK-NEXT:       AArch64 Assembly Printer
 ; CHECK-NEXT:       Free MachineFunction
 ; CHECK-NEXT: Pass Arguments:  -domtree
 ; CHECK-NEXT:   FunctionPass Manager
 ; CHECK-NEXT:     Dominator Tree Construction
 ; CHECK-NEXT: Pass Arguments:  -assumption-cache-tracker -targetlibinfo -domtree -loops -scalar-evolution -stack-safety-local