[llvm] 4af73d7 - Refactor AsmPrinterHandler callbacks. NFCI.

James Y Knight via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 22 15:27:43 PST 2022


Author: James Y Knight
Date: 2022-11-22T18:25:22-05:00
New Revision: 4af73d7ebb5bf4bc2045d15ae0f4ebce56f2d86f

URL: https://github.com/llvm/llvm-project/commit/4af73d7ebb5bf4bc2045d15ae0f4ebce56f2d86f
DIFF: https://github.com/llvm/llvm-project/commit/4af73d7ebb5bf4bc2045d15ae0f4ebce56f2d86f.diff

LOG: Refactor AsmPrinterHandler callbacks. NFCI.

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.

Added: 
    

Modified: 
    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

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/CodeGen/AsmPrinterHandler.h b/llvm/include/llvm/CodeGen/AsmPrinterHandler.h
index dc81a30400977..5c06645f767eb 100644
--- a/llvm/include/llvm/CodeGen/AsmPrinterHandler.h
+++ b/llvm/include/llvm/CodeGen/AsmPrinterHandler.h
@@ -53,13 +53,20 @@ class AsmPrinterHandler {
   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 @@ class AsmPrinterHandler {
 
   /// 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

diff  --git a/llvm/include/llvm/CodeGen/DebugHandlerBase.h b/llvm/include/llvm/CodeGen/DebugHandlerBase.h
index 45823b2ba349e..5f6ea115525fd 100644
--- a/llvm/include/llvm/CodeGen/DebugHandlerBase.h
+++ b/llvm/include/llvm/CodeGen/DebugHandlerBase.h
@@ -123,8 +123,8 @@ class DebugHandlerBase : public AsmPrinterHandler {
   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);

diff  --git a/llvm/lib/CodeGen/AsmPrinter/AIXException.cpp b/llvm/lib/CodeGen/AsmPrinter/AIXException.cpp
index 1940f46232d3b..1cd877aa3a2fd 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AIXException.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AIXException.cpp
@@ -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) {

diff  --git a/llvm/lib/CodeGen/AsmPrinter/ARMException.cpp b/llvm/lib/CodeGen/AsmPrinter/ARMException.cpp
index e04a29fbb42b8..aa56442881d46 100644
--- a/llvm/lib/CodeGen/AsmPrinter/ARMException.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/ARMException.cpp
@@ -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) {

diff  --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
index 10c0eea44f0dd..dc3b07b1de2f1 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -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,

diff  --git a/llvm/lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp b/llvm/lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp
index 8ebbed974abbd..f0b1f73b089d2 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp
@@ -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;
 }

diff  --git a/llvm/lib/CodeGen/AsmPrinter/DwarfCFIException.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfCFIException.cpp
index 5f187acf13dc9..1c1c5dadfb699 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfCFIException.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfCFIException.cpp
@@ -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();
-}

diff  --git a/llvm/lib/CodeGen/AsmPrinter/DwarfException.h b/llvm/lib/CodeGen/AsmPrinter/DwarfException.h
index e5cda4739fde7..4285b408c0f98 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfException.h
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfException.h
@@ -31,7 +31,6 @@ class LLVM_LIBRARY_VISIBILITY DwarfCFIExceptionBase : public EHStreamer {
   bool hasEmittedCFISections = false;
 
   void markFunctionEnd() override;
-  void endFragment() override;
 };
 
 class LLVM_LIBRARY_VISIBILITY DwarfCFIException : public DwarfCFIExceptionBase {
@@ -61,11 +60,8 @@ class LLVM_LIBRARY_VISIBILITY DwarfCFIException : public DwarfCFIExceptionBase {
   /// 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 @@ class LLVM_LIBRARY_VISIBILITY ARMException : public DwarfCFIExceptionBase {
 
   /// 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 @@ class LLVM_LIBRARY_VISIBILITY AIXException : public DwarfCFIExceptionBase {
 
   void endModule() override {}
   void beginFunction(const MachineFunction *MF) override {}
-
   void endFunction(const MachineFunction *MF) override;
 };
 } // End of namespace llvm


        


More information about the llvm-commits mailing list