From 4242df14708cb84b3732ba1a22fb77146833340b Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Fri, 16 Oct 2020 17:22:07 -0400 Subject: [PATCH] Revert "make the AsmPrinterHandler array public" I messed up one of the tests. --- llvm/include/llvm/CodeGen/AsmPrinter.h | 42 ++++++------- llvm/include/llvm/CodeGen/AsmPrinterHandler.h | 4 -- llvm/include/llvm/CodeGen/DebugHandlerBase.h | 2 - llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp | 46 +++++++-------- llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp | 3 + .../CodeGen/AsmPrinter/DebugHandlerBase.cpp | 11 +--- llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp | 29 +++++---- llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h | 4 +- llvm/lib/CodeGen/MachineModuleInfo.cpp | 4 +- llvm/lib/Target/BPF/BTFDebug.cpp | 3 - .../unittests/CodeGen/AsmPrinterDwarfTest.cpp | 59 ------------------- llvm/unittests/CodeGen/TestAsmPrinter.h | 1 - 12 files changed, 66 insertions(+), 142 deletions(-) diff --git a/llvm/include/llvm/CodeGen/AsmPrinter.h b/llvm/include/llvm/CodeGen/AsmPrinter.h index f6efdc664e18..3056568ccf98 100644 --- a/llvm/include/llvm/CodeGen/AsmPrinter.h +++ b/llvm/include/llvm/CodeGen/AsmPrinter.h @@ -139,23 +139,6 @@ public: using GOTEquivUsePair = std::pair; MapVector GlobalGOTEquivs; - /// struct HandlerInfo and Handlers permit users or target extended - /// AsmPrinter to add their own handlers. - struct HandlerInfo { - std::unique_ptr Handler; - const char *TimerName; - const char *TimerDescription; - const char *TimerGroupName; - const char *TimerGroupDescription; - - HandlerInfo(std::unique_ptr Handler, - const char *TimerName, const char *TimerDescription, - const char *TimerGroupName, const char *TimerGroupDescription) - : Handler(std::move(Handler)), TimerName(TimerName), - TimerDescription(TimerDescription), TimerGroupName(TimerGroupName), - TimerGroupDescription(TimerGroupDescription) {} - }; - private: MCSymbol *CurrentFnEnd = nullptr; @@ -179,10 +162,26 @@ private: protected: MCSymbol *CurrentFnBegin = nullptr; + /// Protected struct HandlerInfo and Handlers permit target extended + /// AsmPrinter adds their own handlers. + struct HandlerInfo { + std::unique_ptr Handler; + const char *TimerName; + const char *TimerDescription; + const char *TimerGroupName; + const char *TimerGroupDescription; + + HandlerInfo(std::unique_ptr Handler, + const char *TimerName, const char *TimerDescription, + const char *TimerGroupName, const char *TimerGroupDescription) + : Handler(std::move(Handler)), TimerName(TimerName), + TimerDescription(TimerDescription), TimerGroupName(TimerGroupName), + TimerGroupDescription(TimerGroupDescription) {} + }; + /// A vector of all debug/EH info emitters we should use. This vector /// maintains ownership of the emitters. - std::vector Handlers; - size_t NumUserHandlers = 0; + SmallVector Handlers; public: struct SrcMgrDiagInfo { @@ -447,11 +446,6 @@ public: // Overridable Hooks //===------------------------------------------------------------------===// - void addAsmPrinterHandler(HandlerInfo Handler) { - Handlers.insert(Handlers.begin(), std::move(Handler)); - NumUserHandlers++; - } - // Targets can, or in the case of EmitInstruction, must implement these to // customize output. diff --git a/llvm/include/llvm/CodeGen/AsmPrinterHandler.h b/llvm/include/llvm/CodeGen/AsmPrinterHandler.h index dc81a3040097..b9837dc168e9 100644 --- a/llvm/include/llvm/CodeGen/AsmPrinterHandler.h +++ b/llvm/include/llvm/CodeGen/AsmPrinterHandler.h @@ -23,7 +23,6 @@ class MachineBasicBlock; class MachineFunction; class MachineInstr; class MCSymbol; -class Module; typedef MCSymbol *ExceptionSymbolProvider(AsmPrinter *Asm, const MachineBasicBlock *MBB); @@ -38,8 +37,6 @@ public: /// this tracks that size. virtual void setSymbolSize(const MCSymbol *Sym, uint64_t Size) = 0; - virtual void beginModule(Module *M) {} - /// Emit all sections that should come after the content. virtual void endModule() = 0; @@ -78,7 +75,6 @@ public: /// Process end of a basic block during basic block sections. virtual void endBasicBlock(const MachineBasicBlock &MBB) {} }; - } // End of namespace llvm #endif diff --git a/llvm/include/llvm/CodeGen/DebugHandlerBase.h b/llvm/include/llvm/CodeGen/DebugHandlerBase.h index 02dba1b9e6de..b488979f458c 100644 --- a/llvm/include/llvm/CodeGen/DebugHandlerBase.h +++ b/llvm/include/llvm/CodeGen/DebugHandlerBase.h @@ -115,8 +115,6 @@ private: // AsmPrinterHandler overrides. public: - void beginModule(Module *M) override; - void beginInstruction(const MachineInstr *MI) override; void endInstruction() override; diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp index 3353c6243387..f5587c9ae7dc 100644 --- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp +++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp @@ -188,8 +188,7 @@ AsmPrinter::AsmPrinter(TargetMachine &tm, std::unique_ptr Streamer) } AsmPrinter::~AsmPrinter() { - assert(!DD && Handlers.size() == NumUserHandlers && - "Debug/EH info didn't get finalized"); + assert(!DD && Handlers.empty() && "Debug/EH info didn't get finalized"); if (GCMetadataPrinters) { gcp_map_type &GCMap = getGCMap(GCMetadataPrinters); @@ -253,9 +252,6 @@ bool AsmPrinter::doInitialization(Module &M) { auto *MMIWP = getAnalysisIfAvailable(); MMI = MMIWP ? &MMIWP->getMMI() : nullptr; - if (!M.debug_compile_units().empty()) - MMI->setDebugInfoAvailability(true); - // Initialize TargetLoweringObjectFile. const_cast(getObjFileLowering()) .Initialize(OutContext, TM); @@ -317,7 +313,8 @@ bool AsmPrinter::doInitialization(Module &M) { CodeViewLineTablesGroupDescription); } if (!EmitCodeView || M.getDwarfVersion()) { - DD = new DwarfDebug(this); + DD = new DwarfDebug(this, &M); + DD->beginModule(); Handlers.emplace_back(std::unique_ptr(DD), DbgTimerName, DbgTimerDescription, DWARFGroupName, DWARFGroupDescription); @@ -382,13 +379,6 @@ bool AsmPrinter::doInitialization(Module &M) { Handlers.emplace_back(std::make_unique(this), CFGuardName, CFGuardDescription, DWARFGroupName, DWARFGroupDescription); - - for (const HandlerInfo &HI : Handlers) { - NamedRegionTimer T(HI.TimerName, HI.TimerDescription, HI.TimerGroupName, - HI.TimerGroupDescription, TimePassesIsEnabled); - HI.Handler->beginModule(&M); - } - return false; } @@ -1120,6 +1110,8 @@ void AsmPrinter::emitFunctionBody() { // Emit target-specific gunk before the function body. emitFunctionBodyStart(); + bool ShouldPrintDebugScopes = MMI->hasDebugInfo(); + if (isVerbose()) { // Get MachineDominatorTree or compute it on the fly if it's unavailable MDT = getAnalysisIfAvailable(); @@ -1157,10 +1149,13 @@ void AsmPrinter::emitFunctionBody() { if (MCSymbol *S = MI.getPreInstrSymbol()) OutStreamer->emitLabel(S); - for (const HandlerInfo &HI : Handlers) { - NamedRegionTimer T(HI.TimerName, HI.TimerDescription, HI.TimerGroupName, - HI.TimerGroupDescription, TimePassesIsEnabled); - HI.Handler->beginInstruction(&MI); + if (ShouldPrintDebugScopes) { + for (const HandlerInfo &HI : Handlers) { + NamedRegionTimer T(HI.TimerName, HI.TimerDescription, + HI.TimerGroupName, HI.TimerGroupDescription, + TimePassesIsEnabled); + HI.Handler->beginInstruction(&MI); + } } if (isVerbose()) @@ -1214,10 +1209,13 @@ void AsmPrinter::emitFunctionBody() { if (MCSymbol *S = MI.getPostInstrSymbol()) OutStreamer->emitLabel(S); - for (const HandlerInfo &HI : Handlers) { - NamedRegionTimer T(HI.TimerName, HI.TimerDescription, HI.TimerGroupName, - HI.TimerGroupDescription, TimePassesIsEnabled); - HI.Handler->endInstruction(); + if (ShouldPrintDebugScopes) { + for (const HandlerInfo &HI : Handlers) { + NamedRegionTimer T(HI.TimerName, HI.TimerDescription, + HI.TimerGroupName, HI.TimerGroupDescription, + TimePassesIsEnabled); + HI.Handler->endInstruction(); + } } } @@ -1639,11 +1637,7 @@ bool AsmPrinter::doFinalization(Module &M) { HI.TimerGroupDescription, TimePassesIsEnabled); HI.Handler->endModule(); } - - // This deletes all the ephemeral handlers that AsmPrinter added, while - // keeping all the user-added handlers alive until the AsmPrinter is - // destroyed. - Handlers.erase(Handlers.begin() + NumUserHandlers, Handlers.end()); + Handlers.clear(); DD = nullptr; // If the target wants to know about weak references, print them all. diff --git a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp index a24deb53a6a6..bcace6264cd0 100644 --- a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp +++ b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp @@ -141,6 +141,7 @@ CodeViewDebug::CodeViewDebug(AsmPrinter *AP) if (!MMI->getModule()->getNamedMetadata("llvm.dbg.cu") || !AP->getObjFileLowering().getCOFFDebugSymbolsSection()) { Asm = nullptr; + MMI->setDebugInfoAvailability(false); return; } // Tell MMI that we have debug info. @@ -563,6 +564,8 @@ void CodeViewDebug::endModule() { if (!Asm || !MMI->hasDebugInfo()) return; + assert(Asm != nullptr); + // The COFF .debug$S section consists of several subsections, each starting // with a 4-byte control code (e.g. 0xF1, 0xF2, etc) and then a 4-byte length // of the payload followed by the payload itself. The subsections are 4-byte diff --git a/llvm/lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp b/llvm/lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp index ab53540ed2c0..826c5078ed50 100644 --- a/llvm/lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp +++ b/llvm/lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp @@ -91,11 +91,6 @@ DbgVariableLocation::extractFromMachineInstruction( DebugHandlerBase::DebugHandlerBase(AsmPrinter *A) : Asm(A), MMI(Asm->MMI) {} -void DebugHandlerBase::beginModule(Module *M) { - if (M->debug_compile_units().empty()) - Asm = nullptr; -} - // Each LexicalScope has first instruction and last instruction to mark // beginning and end of a scope respectively. Create an inverse map that list // scopes starts (and ends) with an instruction. One instruction may start (or @@ -281,7 +276,7 @@ void DebugHandlerBase::beginFunction(const MachineFunction *MF) { } void DebugHandlerBase::beginInstruction(const MachineInstr *MI) { - if (!Asm || !MMI->hasDebugInfo()) + if (!MMI->hasDebugInfo()) return; assert(CurMI == nullptr); @@ -307,7 +302,7 @@ void DebugHandlerBase::beginInstruction(const MachineInstr *MI) { } void DebugHandlerBase::endInstruction() { - if (!Asm || !MMI->hasDebugInfo()) + if (!MMI->hasDebugInfo()) return; assert(CurMI != nullptr); @@ -339,7 +334,7 @@ void DebugHandlerBase::endInstruction() { } void DebugHandlerBase::endFunction(const MachineFunction *MF) { - if (Asm && hasDebugInfo(MMI, MF)) + if (hasDebugInfo(MMI, MF)) endFunctionImpl(MF); DbgValues.clear(); DbgLabels.clear(); diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp index 2dc0a762736a..a315a7e2ca02 100644 --- a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp +++ b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp @@ -148,6 +148,10 @@ static cl::opt "Abstract subprograms")), cl::init(DefaultLinkageNames)); +static const char *const DWARFGroupName = "dwarf"; +static const char *const DWARFGroupDescription = "DWARF Emission"; +static const char *const DbgTimerName = "writer"; +static const char *const DbgTimerDescription = "DWARF Debug Writer"; static constexpr unsigned ULEB128PadSize = 4; void DebugLocDwarfExpression::emitOp(uint8_t Op, const char *Comment) { @@ -326,7 +330,7 @@ static AccelTableKind computeAccelTableKind(unsigned DwarfVersion, return AccelTableKind::None; } -DwarfDebug::DwarfDebug(AsmPrinter *A) +DwarfDebug::DwarfDebug(AsmPrinter *A, Module *M) : DebugHandlerBase(A), DebugLocs(A->OutStreamer->isVerboseAsm()), InfoHolder(A, "info_string", DIEValueAllocator), SkeletonHolder(A, "skel_string", DIEValueAllocator), @@ -1107,17 +1111,21 @@ sortGlobalExprs(SmallVectorImpl &GVEs) { // Emit all Dwarf sections that should come prior to the content. Create // global DIEs and emit initial debug info sections. This is invoked by // the target AsmPrinter. -void DwarfDebug::beginModule(Module *M) { - DebugHandlerBase::beginModule(M); - - if (!Asm || !MMI->hasDebugInfo() || DisableDebugInfoPrinting) +void DwarfDebug::beginModule() { + NamedRegionTimer T(DbgTimerName, DbgTimerDescription, DWARFGroupName, + DWARFGroupDescription, TimePassesIsEnabled); + if (DisableDebugInfoPrinting) { + MMI->setDebugInfoAvailability(false); return; + } + + const Module *M = MMI->getModule(); unsigned NumDebugCUs = std::distance(M->debug_compile_units_begin(), M->debug_compile_units_end()); - assert(NumDebugCUs > 0 && "Asm unexpectedly initialized"); - assert(MMI->hasDebugInfo() && - "DebugInfoAvailabilty unexpectedly not initialized"); + // Tell MMI whether we have debug info. + assert(MMI->hasDebugInfo() == (NumDebugCUs > 0) && + "DebugInfoAvailabilty initialized unexpectedly"); SingleCU = NumDebugCUs == 1; DenseMap> GVMap; @@ -1380,7 +1388,7 @@ void DwarfDebug::endModule() { // If we aren't actually generating debug info (check beginModule - // conditionalized on !DisableDebugInfoPrinting and the presence of the // llvm.dbg.cu metadata node) - if (!Asm || !MMI->hasDebugInfo() || DisableDebugInfoPrinting) + if (!MMI->hasDebugInfo()) return; // Finalize the debug info for the module. @@ -1891,8 +1899,7 @@ void DwarfDebug::beginInstruction(const MachineInstr *MI) { } DebugHandlerBase::beginInstruction(MI); - if (!CurMI) - return; + assert(CurMI); if (NoDebug) return; diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h index a542fdd89f8a..34c88f1a9c60 100644 --- a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h +++ b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h @@ -621,13 +621,13 @@ public: //===--------------------------------------------------------------------===// // Main entry points. // - DwarfDebug(AsmPrinter *A); + DwarfDebug(AsmPrinter *A, Module *M); ~DwarfDebug() override; /// Emit all Dwarf sections that should come prior to the /// content. - void beginModule(Module *M) override; + void beginModule(); /// Emit all Dwarf sections that should come after the content. void endModule() override; diff --git a/llvm/lib/CodeGen/MachineModuleInfo.cpp b/llvm/lib/CodeGen/MachineModuleInfo.cpp index ecae4961d5a9..be08f0ae3117 100644 --- a/llvm/lib/CodeGen/MachineModuleInfo.cpp +++ b/llvm/lib/CodeGen/MachineModuleInfo.cpp @@ -304,7 +304,7 @@ char MachineModuleInfoWrapperPass::ID = 0; bool MachineModuleInfoWrapperPass::doInitialization(Module &M) { MMI.initialize(); MMI.TheModule = &M; - MMI.DbgInfoAvailable = false; + MMI.DbgInfoAvailable = !M.debug_compile_units().empty(); return false; } @@ -319,6 +319,6 @@ MachineModuleInfo MachineModuleAnalysis::run(Module &M, ModuleAnalysisManager &) { MachineModuleInfo MMI(TM); MMI.TheModule = &M; - MMI.DbgInfoAvailable = false; + MMI.DbgInfoAvailable = !M.debug_compile_units().empty(); return MMI; } diff --git a/llvm/lib/Target/BPF/BTFDebug.cpp b/llvm/lib/Target/BPF/BTFDebug.cpp index 30d86e02f367..709e599cd6b8 100644 --- a/llvm/lib/Target/BPF/BTFDebug.cpp +++ b/llvm/lib/Target/BPF/BTFDebug.cpp @@ -1076,9 +1076,6 @@ void BTFDebug::beginInstruction(const MachineInstr *MI) { } } - if (!CurMI) // no debug info - return; - // Skip this instruction if no DebugLoc or the DebugLoc // is the same as the previous instruction. const DebugLoc &DL = MI->getDebugLoc(); diff --git a/llvm/unittests/CodeGen/AsmPrinterDwarfTest.cpp b/llvm/unittests/CodeGen/AsmPrinterDwarfTest.cpp index c9e489ed2d8a..5c53f39fd9a3 100644 --- a/llvm/unittests/CodeGen/AsmPrinterDwarfTest.cpp +++ b/llvm/unittests/CodeGen/AsmPrinterDwarfTest.cpp @@ -8,13 +8,8 @@ #include "TestAsmPrinter.h" #include "llvm/CodeGen/AsmPrinter.h" -#include "llvm/CodeGen/MachineModuleInfo.h" -#include "llvm/IR/LegacyPassManager.h" -#include "llvm/IR/Module.h" -#include "llvm/IR/PassManager.h" #include "llvm/MC/MCContext.h" #include "llvm/MC/MCSectionELF.h" -#include "llvm/Target/TargetMachine.h" #include "llvm/Testing/Support/Error.h" using namespace llvm; @@ -372,58 +367,4 @@ TEST_F(AsmPrinterEmitDwarfUnitLengthAsHiLoDiffTest, DWARF64) { TestPrinter->getAP()->emitDwarfUnitLength(Hi, Lo, ""); } -class AsmPrinterHandlerTest : public AsmPrinterFixtureBase { - class TestHandler : public AsmPrinterHandler { - AsmPrinterHandlerTest &Test; - - public: - TestHandler(AsmPrinterHandlerTest &Test) : Test(Test) {} - virtual ~TestHandler() {} - virtual void setSymbolSize(const MCSymbol *Sym, uint64_t Size) override {} - virtual void beginModule(Module *M) override { Test.BeginCount++; } - virtual void endModule() override { Test.EndCount++; } - virtual void beginFunction(const MachineFunction *MF) override {} - virtual void endFunction(const MachineFunction *MF) override {} - virtual void beginInstruction(const MachineInstr *MI) override {} - virtual void endInstruction() override {} - }; - -protected: - bool init(const std::string &TripleStr, unsigned DwarfVersion, - dwarf::DwarfFormat DwarfFormat) { - if (!AsmPrinterFixtureBase::init(TripleStr, DwarfVersion, DwarfFormat)) - return false; - - auto *AP = TestPrinter->getAP(); - AP->addAsmPrinterHandler(AsmPrinter::HandlerInfo( - std::unique_ptr(new TestHandler(*this)), - "TestTimerName", "TestTimerDesc", "TestGroupName", "TestGroupDesc")); - LLVMTargetMachine *LLVMTM = static_cast(&AP->TM); - legacy::PassManager PM; - PM.add(new MachineModuleInfoWrapperPass(LLVMTM)); - PM.add(TestPrinter->releaseAP()); // Takes ownership of destroying AP - LLVMContext Context; - std::unique_ptr M(new Module("TestModule", Context)); - M->setDataLayout(LLVMTM->createDataLayout()); - PM.run(*M); - // Now check that we can run it twice. - AP->addAsmPrinterHandler(AsmPrinter::HandlerInfo( - std::unique_ptr(new TestHandler(*this)), - "TestTimerName", "TestTimerDesc", "TestGroupName", "TestGroupDesc")); - PM.run(*M); - return true; - } - - int BeginCount = 0; - int EndCount = 0; -}; - -TEST_F(AsmPrinterHandlerTest, Basic) { - if (!init("x86_64-pc-linux", /*DwarfVersion=*/4, dwarf::DWARF32)) - return; - - ASSERT_EQ(BeginCount, 3); - ASSERT_EQ(EndCount, 3); -} - } // end namespace diff --git a/llvm/unittests/CodeGen/TestAsmPrinter.h b/llvm/unittests/CodeGen/TestAsmPrinter.h index b69cd34247a6..65e557b9b4a6 100644 --- a/llvm/unittests/CodeGen/TestAsmPrinter.h +++ b/llvm/unittests/CodeGen/TestAsmPrinter.h @@ -73,7 +73,6 @@ public: void setDwarfUsesRelocationsAcrossSections(bool Enable); AsmPrinter *getAP() const { return Asm.get(); } - AsmPrinter *releaseAP() { return Asm.release(); } MCContext &getCtx() const { return *MC; } MockMCStreamer &getMS() const { return *MS; } };