Refactor AsmPrinterHandler callbacks. NFCI.
authorJames Y Knight <jyknight@google.com>
Tue, 22 Nov 2022 18:35:43 +0000 (13:35 -0500)
committerJames Y Knight <jyknight@google.com>
Tue, 22 Nov 2022 23:25:22 +0000 (18:25 -0500)
The existing behaviors and callbacks were overlapping and had very
confusing semantics: beginBasicBlock/endBasicBlock were not always
called, beginFragment/endFragment seemed like they were meant to mean
the same thing, but were slightly different, etc. This resulted in
confusing semantics, virtual method overloads, and control flow.

Remove the above, and replace with new beginBasicBlockSection and
endBasicBlockSection callbacks. And document them.

These are always called before the first and after the last blocks in
a function, even when basic-block-sections are disabled.

llvm/include/llvm/CodeGen/AsmPrinterHandler.h
llvm/include/llvm/CodeGen/DebugHandlerBase.h
llvm/lib/CodeGen/AsmPrinter/AIXException.cpp
llvm/lib/CodeGen/AsmPrinter/ARMException.cpp
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
llvm/lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp
llvm/lib/CodeGen/AsmPrinter/DwarfCFIException.cpp
llvm/lib/CodeGen/AsmPrinter/DwarfException.h

index dc81a30..5c06645 100644 (file)
@@ -53,13 +53,20 @@ public:
   virtual void markFunctionEnd();
 
   /// Gather post-function debug information.
-  /// Please note that some AsmPrinter implementations may not call
-  /// beginFunction at all.
   virtual void endFunction(const MachineFunction *MF) = 0;
 
-  virtual void beginFragment(const MachineBasicBlock *MBB,
-                             ExceptionSymbolProvider ESP) {}
-  virtual void endFragment() {}
+  /// Process the beginning of a new basic-block-section within a
+  /// function. Always called immediately after beginFunction for the first
+  /// basic-block. When basic-block-sections are enabled, called before the
+  /// first block of each such section.
+  virtual void beginBasicBlockSection(const MachineBasicBlock &MBB) {}
+
+  /// Process the end of a basic-block-section within a function. When
+  /// basic-block-sections are enabled, called after the last block in each such
+  /// section (including the last section in the function). When
+  /// basic-block-sections are disabled, called at the end of a function,
+  /// immediately prior to markFunctionEnd.
+  virtual void endBasicBlockSection(const MachineBasicBlock &MBB) {}
 
   /// Emit target-specific EH funclet machinery.
   virtual void beginFunclet(const MachineBasicBlock &MBB,
@@ -71,12 +78,6 @@ public:
 
   /// Process end of an instruction.
   virtual void endInstruction() = 0;
-
-  /// Process beginning of a basic block during basic block sections.
-  virtual void beginBasicBlock(const MachineBasicBlock &MBB) {}
-
-  /// Process end of a basic block during basic block sections.
-  virtual void endBasicBlock(const MachineBasicBlock &MBB) {}
 };
 
 } // End of namespace llvm
index 45823b2..5f6ea11 100644 (file)
@@ -123,8 +123,8 @@ public:
   void beginFunction(const MachineFunction *MF) override;
   void endFunction(const MachineFunction *MF) override;
 
-  void beginBasicBlock(const MachineBasicBlock &MBB) override;
-  void endBasicBlock(const MachineBasicBlock &MBB) override;
+  void beginBasicBlockSection(const MachineBasicBlock &MBB) override;
+  void endBasicBlockSection(const MachineBasicBlock &MBB) override;
 
   /// Return Label preceding the instruction.
   MCSymbol *getLabelBeforeInsn(const MachineInstr *MI);
index 1940f46..1cd877a 100644 (file)
@@ -23,7 +23,14 @@ namespace llvm {
 
 AIXException::AIXException(AsmPrinter *A) : DwarfCFIExceptionBase(A) {}
 
-void AIXException::markFunctionEnd() { endFragment(); }
+// This overrides 'DwarfCFIExceptionBase::markFunctionEnd', to avoid the call to
+// tidyLandingPads. This is necessary, because the
+// 'PPCAIXAsmPrinter::emitFunctionBodyEnd' function already checked whether we
+// need ehinfo, and emitted a traceback table with the bits set to indicate that
+// we will be emitting it, if so. Thus, if we remove it now -- so late in the
+// process -- we'll end up having emitted a reference to __ehinfo.N symbol, but
+// not emitting a definition for said symbol.
+void AIXException::markFunctionEnd() {}
 
 void AIXException::emitExceptionInfoTable(const MCSymbol *LSDA,
                                           const MCSymbol *PerSym) {
index e04a29f..aa56442 100644 (file)
@@ -48,6 +48,12 @@ void ARMException::beginFunction(const MachineFunction *MF) {
   }
 }
 
+void ARMException::markFunctionEnd() {
+  if (shouldEmitCFI)
+    Asm->OutStreamer->emitCFIEndProc();
+  DwarfCFIExceptionBase::markFunctionEnd();
+}
+
 /// endFunction - Gather and emit post-function exception information.
 ///
 void ARMException::endFunction(const MachineFunction *MF) {
index 10c0eea..dc3b07b 100644 (file)
@@ -1004,6 +1004,11 @@ void AsmPrinter::emitFunctionHeader() {
                        HI.TimerGroupDescription, TimePassesIsEnabled);
     HI.Handler->beginFunction(MF);
   }
+  for (const HandlerInfo &HI : Handlers) {
+    NamedRegionTimer T(HI.TimerName, HI.TimerDescription, HI.TimerGroupName,
+                       HI.TimerGroupDescription, TimePassesIsEnabled);
+    HI.Handler->beginBasicBlockSection(MF->front());
+  }
 
   // Emit the prologue data.
   if (F.hasPrologueData())
@@ -1803,6 +1808,15 @@ void AsmPrinter::emitFunctionBody() {
       OutStreamer->emitELFSize(CurrentFnBeginLocal, SizeExp);
   }
 
+  // Call endBasicBlockSection on the last block now, if it wasn't already
+  // called.
+  if (!MF->back().isEndSection()) {
+    for (const HandlerInfo &HI : Handlers) {
+      NamedRegionTimer T(HI.TimerName, HI.TimerDescription, HI.TimerGroupName,
+                         HI.TimerGroupDescription, TimePassesIsEnabled);
+      HI.Handler->endBasicBlockSection(MF->back());
+    }
+  }
   for (const HandlerInfo &HI : Handlers) {
     NamedRegionTimer T(HI.TimerName, HI.TimerDescription, HI.TimerGroupName,
                        HI.TimerGroupDescription, TimePassesIsEnabled);
@@ -3719,11 +3733,11 @@ void AsmPrinter::emitBasicBlockStart(const MachineBasicBlock &MBB) {
   }
 
   // With BB sections, each basic block must handle CFI information on its own
-  // if it begins a section (Entry block is handled separately by
-  // AsmPrinterHandler::beginFunction).
+  // if it begins a section (Entry block call is handled separately, next to
+  // beginFunction).
   if (MBB.isBeginSection() && !MBB.isEntryBlock())
     for (const HandlerInfo &HI : Handlers)
-      HI.Handler->beginBasicBlock(MBB);
+      HI.Handler->beginBasicBlockSection(MBB);
 }
 
 void AsmPrinter::emitBasicBlockEnd(const MachineBasicBlock &MBB) {
@@ -3731,7 +3745,7 @@ void AsmPrinter::emitBasicBlockEnd(const MachineBasicBlock &MBB) {
   // sections.
   if (MBB.isEndSection())
     for (const HandlerInfo &HI : Handlers)
-      HI.Handler->endBasicBlock(MBB);
+      HI.Handler->endBasicBlockSection(MBB);
 }
 
 void AsmPrinter::emitVisibility(MCSymbol *Sym, unsigned Visibility,
index 8ebbed9..f0b1f73 100644 (file)
@@ -416,16 +416,11 @@ void DebugHandlerBase::endFunction(const MachineFunction *MF) {
   InstOrdering.clear();
 }
 
-void DebugHandlerBase::beginBasicBlock(const MachineBasicBlock &MBB) {
-  if (!MBB.isBeginSection())
-    return;
-
-  PrevLabel = MBB.getSymbol();
+void DebugHandlerBase::beginBasicBlockSection(const MachineBasicBlock &MBB) {
+  if (!MBB.isEntryBlock())
+    PrevLabel = MBB.getSymbol();
 }
 
-void DebugHandlerBase::endBasicBlock(const MachineBasicBlock &MBB) {
-  if (!MBB.isEndSection())
-    return;
-
+void DebugHandlerBase::endBasicBlockSection(const MachineBasicBlock &MBB) {
   PrevLabel = nullptr;
 }
index 5f187ac..1c1c5da 100644 (file)
@@ -26,8 +26,6 @@ using namespace llvm;
 DwarfCFIExceptionBase::DwarfCFIExceptionBase(AsmPrinter *A) : EHStreamer(A) {}
 
 void DwarfCFIExceptionBase::markFunctionEnd() {
-  endFragment();
-
   // Map all labels and get rid of any dead landing pads.
   if (!Asm->MF->getLandingPads().empty()) {
     MachineFunction *NonConstMF = const_cast<MachineFunction*>(Asm->MF);
@@ -35,11 +33,6 @@ void DwarfCFIExceptionBase::markFunctionEnd() {
   }
 }
 
-void DwarfCFIExceptionBase::endFragment() {
-  if (shouldEmitCFI && !Asm->MF->hasBBSections())
-    Asm->OutStreamer->emitCFIEndProc();
-}
-
 DwarfCFIException::DwarfCFIException(AsmPrinter *A)
     : DwarfCFIExceptionBase(A) {}
 
@@ -68,11 +61,6 @@ void DwarfCFIException::endModule() {
   }
 }
 
-static MCSymbol *getExceptionSym(AsmPrinter *Asm,
-                                 const MachineBasicBlock *MBB) {
-  return Asm->getMBBExceptionSym(*MBB);
-}
-
 void DwarfCFIException::beginFunction(const MachineFunction *MF) {
   shouldEmitPersonality = shouldEmitLSDA = false;
   const Function &F = MF->getFunction();
@@ -114,12 +102,9 @@ void DwarfCFIException::beginFunction(const MachineFunction *MF) {
         MAI.usesCFIForEH() && (shouldEmitPersonality || shouldEmitMoves);
   else
     shouldEmitCFI = Asm->needsCFIForDebug() && shouldEmitMoves;
-
-  beginFragment(&*MF->begin(), getExceptionSym);
 }
 
-void DwarfCFIException::beginFragment(const MachineBasicBlock *MBB,
-                                      ExceptionSymbolProvider ESP) {
+void DwarfCFIException::beginBasicBlockSection(const MachineBasicBlock &MBB) {
   if (!shouldEmitCFI)
     return;
 
@@ -141,7 +126,7 @@ void DwarfCFIException::beginFragment(const MachineBasicBlock *MBB,
   if (!shouldEmitPersonality)
     return;
 
-  auto &F = MBB->getParent()->getFunction();
+  auto &F = MBB.getParent()->getFunction();
   auto *P = dyn_cast<Function>(F.getPersonalityFn()->stripPointerCasts());
   assert(P && "Expected personality function");
 
@@ -157,7 +142,13 @@ void DwarfCFIException::beginFragment(const MachineBasicBlock *MBB,
 
   // Provide LSDA information.
   if (shouldEmitLSDA)
-    Asm->OutStreamer->emitCFILsda(ESP(Asm, MBB), TLOF.getLSDAEncoding());
+    Asm->OutStreamer->emitCFILsda(Asm->getMBBExceptionSym(MBB),
+                                  TLOF.getLSDAEncoding());
+}
+
+void DwarfCFIException::endBasicBlockSection(const MachineBasicBlock &MBB) {
+  if (shouldEmitCFI)
+    Asm->OutStreamer->emitCFIEndProc();
 }
 
 /// endFunction - Gather and emit post-function exception information.
@@ -168,12 +159,3 @@ void DwarfCFIException::endFunction(const MachineFunction *MF) {
 
   emitExceptionTable();
 }
-
-void DwarfCFIException::beginBasicBlock(const MachineBasicBlock &MBB) {
-  beginFragment(&MBB, getExceptionSym);
-}
-
-void DwarfCFIException::endBasicBlock(const MachineBasicBlock &MBB) {
-  if (shouldEmitCFI)
-    Asm->OutStreamer->emitCFIEndProc();
-}
index e5cda47..4285b40 100644 (file)
@@ -31,7 +31,6 @@ protected:
   bool hasEmittedCFISections = false;
 
   void markFunctionEnd() override;
-  void endFragment() override;
 };
 
 class LLVM_LIBRARY_VISIBILITY DwarfCFIException : public DwarfCFIExceptionBase {
@@ -61,11 +60,8 @@ public:
   /// Gather and emit post-function exception information.
   void endFunction(const MachineFunction *) override;
 
-  void beginFragment(const MachineBasicBlock *MBB,
-                     ExceptionSymbolProvider ESP) override;
-
-  void beginBasicBlock(const MachineBasicBlock &MBB) override;
-  void endBasicBlock(const MachineBasicBlock &MBB) override;
+  void beginBasicBlockSection(const MachineBasicBlock &MBB) override;
+  void endBasicBlockSection(const MachineBasicBlock &MBB) override;
 };
 
 class LLVM_LIBRARY_VISIBILITY ARMException : public DwarfCFIExceptionBase {
@@ -88,6 +84,8 @@ public:
 
   /// Gather and emit post-function exception information.
   void endFunction(const MachineFunction *) override;
+
+  void markFunctionEnd() override;
 };
 
 class LLVM_LIBRARY_VISIBILITY AIXException : public DwarfCFIExceptionBase {
@@ -102,7 +100,6 @@ public:
 
   void endModule() override {}
   void beginFunction(const MachineFunction *MF) override {}
-
   void endFunction(const MachineFunction *MF) override;
 };
 } // End of namespace llvm