From 4d27cafc23312c7e584e1d80c4ba6ee16ff375e8 Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Thu, 16 Jan 2025 11:00:06 -0500 Subject: [PATCH 1/7] [AsmPrinter] Reintroduce full AsmPrinterHandler API (#122297) This restores the functionality of AsmPrinterHandlers to what it was prior to https://github.com/llvm/llvm-project/pull/96785. The attempted hack there of adding a duplicate DebugHandlerBase handling added a lot of hidden state and assumptions, which just segfaulted when we tried to continuing using this API. Instead, this just goes back to the old design, but adds a separate array for the basic EH handles. The duplicate array is identical to the other array of handler, but which doesn't get their begin/endInstruction callbacks called. This still saves the negligible but measurable amount of virtual function calls as was the goal of #96785, while restoring the API to the pre-LLVM-19 status quo. (cherry picked from commit f6b0555a433cea1d32a6904c120516cd94b8f3db) --- llvm/include/llvm/CodeGen/AsmPrinter.h | 17 ++--- llvm/include/llvm/CodeGen/AsmPrinterHandler.h | 12 ++++ llvm/include/llvm/CodeGen/DebugHandlerBase.h | 26 +++---- llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp | 68 +++++++++---------- .../CodeGen/AsmPrinter/PseudoProbePrinter.h | 1 - llvm/lib/Target/BPF/BPFAsmPrinter.cpp | 2 +- .../unittests/CodeGen/AsmPrinterDwarfTest.cpp | 53 +-------------- 7 files changed, 69 insertions(+), 110 deletions(-) diff --git a/llvm/include/llvm/CodeGen/AsmPrinter.h b/llvm/include/llvm/CodeGen/AsmPrinter.h index f57be39076a78..c7a4d3c3bfe24 100644 --- a/llvm/include/llvm/CodeGen/AsmPrinter.h +++ b/llvm/include/llvm/CodeGen/AsmPrinter.h @@ -44,6 +44,7 @@ class DebugHandlerBase; class DIE; class DIEAbbrev; class DwarfDebug; +class EHStreamer; class GCMetadataPrinter; class GCStrategy; class GlobalAlias; @@ -187,15 +188,17 @@ class AsmPrinter : public MachineFunctionPass { /// For dso_local functions, the current $local alias for the function. MCSymbol *CurrentFnBeginLocal = nullptr; - /// A vector of all debug/EH info emitters we should use. This vector - /// maintains ownership of the emitters. + /// A handle to the EH info emitter (if present). + // Only for EHStreamer subtypes, but some C++ compilers will incorrectly warn + // us if we declare that directly. + SmallVector, 1> EHHandlers; + + // A vector of all Debuginfo emitters we should use. Protected so that + // targets can add their own. This vector maintains ownership of the + // emitters. SmallVector, 2> Handlers; size_t NumUserHandlers = 0; - /// Debuginfo handler. Protected so that targets can add their own. - SmallVector, 1> DebugHandlers; - size_t NumUserDebugHandlers = 0; - StackMaps SM; private: @@ -521,8 +524,6 @@ class AsmPrinter : public MachineFunctionPass { void addAsmPrinterHandler(std::unique_ptr Handler); - void addDebugHandler(std::unique_ptr Handler); - // 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 ed73e618431de..bf3f6c53027a7 100644 --- a/llvm/include/llvm/CodeGen/AsmPrinterHandler.h +++ b/llvm/include/llvm/CodeGen/AsmPrinterHandler.h @@ -64,6 +64,18 @@ class AsmPrinterHandler { /// immediately prior to markFunctionEnd. virtual void endBasicBlockSection(const MachineBasicBlock &MBB) {} + /// For symbols that have a size designated (e.g. common symbols), + /// this tracks that size. + virtual void setSymbolSize(const MCSymbol *Sym, uint64_t Size) {} + + /// Process beginning of an instruction. + virtual void beginInstruction(const MachineInstr *MI) {} + + /// Process end of an instruction. + virtual void endInstruction() {} + + virtual void beginCodeAlignment(const MachineBasicBlock &MBB) {} + /// Emit target-specific EH funclet machinery. virtual void beginFunclet(const MachineBasicBlock &MBB, MCSymbol *Sym = nullptr) {} diff --git a/llvm/include/llvm/CodeGen/DebugHandlerBase.h b/llvm/include/llvm/CodeGen/DebugHandlerBase.h index 36a844e7087fa..17764b31f30eb 100644 --- a/llvm/include/llvm/CodeGen/DebugHandlerBase.h +++ b/llvm/include/llvm/CodeGen/DebugHandlerBase.h @@ -50,14 +50,10 @@ struct DbgVariableLocation { /// Base class for debug information backends. Common functionality related to /// tracking which variables and scopes are alive at a given PC live here. -class DebugHandlerBase { +class DebugHandlerBase : public AsmPrinterHandler { protected: DebugHandlerBase(AsmPrinter *A); -public: - virtual ~DebugHandlerBase(); - -protected: /// Target of debug info emission. AsmPrinter *Asm = nullptr; @@ -120,22 +116,20 @@ class DebugHandlerBase { private: InstructionOrdering InstOrdering; + // AsmPrinterHandler overrides. public: - /// For symbols that have a size designated (e.g. common symbols), - /// this tracks that size. Only used by DWARF. - virtual void setSymbolSize(const MCSymbol *Sym, uint64_t Size) {} + virtual ~DebugHandlerBase() override; - virtual void beginModule(Module *M); - virtual void endModule() = 0; + void beginModule(Module *M) override; - virtual void beginInstruction(const MachineInstr *MI); - virtual void endInstruction(); + void beginInstruction(const MachineInstr *MI) override; + void endInstruction() override; - void beginFunction(const MachineFunction *MF); - void endFunction(const MachineFunction *MF); + void beginFunction(const MachineFunction *MF) override; + void endFunction(const MachineFunction *MF) override; - void beginBasicBlockSection(const MachineBasicBlock &MBB); - void endBasicBlockSection(const MachineBasicBlock &MBB); + 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/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp index 2297b27ffdc07..6d510d11c38e6 100644 --- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp +++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp @@ -539,12 +539,12 @@ bool AsmPrinter::doInitialization(Module &M) { if (MAI->doesSupportDebugInformation()) { bool EmitCodeView = M.getCodeViewFlag(); if (EmitCodeView && TM.getTargetTriple().isOSWindows()) - DebugHandlers.push_back(std::make_unique(this)); + Handlers.push_back(std::make_unique(this)); if (!EmitCodeView || M.getDwarfVersion()) { assert(MMI && "MMI could not be nullptr here!"); if (MMI->hasDebugInfo()) { DD = new DwarfDebug(this); - DebugHandlers.push_back(std::unique_ptr(DD)); + Handlers.push_back(std::unique_ptr(DD)); } } } @@ -611,12 +611,12 @@ bool AsmPrinter::doInitialization(Module &M) { // Emit tables for any value of cfguard flag (i.e. cfguard=1 or cfguard=2). if (mdconst::extract_or_null(M.getModuleFlag("cfguard"))) - Handlers.push_back(std::make_unique(this)); + EHHandlers.push_back(std::make_unique(this)); - for (auto &Handler : DebugHandlers) - Handler->beginModule(&M); for (auto &Handler : Handlers) Handler->beginModule(&M); + for (auto &Handler : EHHandlers) + Handler->beginModule(&M); return false; } @@ -763,7 +763,7 @@ void AsmPrinter::emitGlobalVariable(const GlobalVariable *GV) { // sections and expected to be contiguous (e.g. ObjC metadata). const Align Alignment = getGVAlignment(GV, DL); - for (auto &Handler : DebugHandlers) + for (auto &Handler : Handlers) Handler->setSymbolSize(GVSym, Size); // Handle common symbols @@ -1035,14 +1035,14 @@ void AsmPrinter::emitFunctionHeader() { } // Emit pre-function debug and/or EH information. - for (auto &Handler : DebugHandlers) { + for (auto &Handler : Handlers) { Handler->beginFunction(MF); Handler->beginBasicBlockSection(MF->front()); } - for (auto &Handler : Handlers) + for (auto &Handler : EHHandlers) { Handler->beginFunction(MF); - for (auto &Handler : Handlers) Handler->beginBasicBlockSection(MF->front()); + } // Emit the prologue data. if (F.hasPrologueData()) @@ -1737,7 +1737,7 @@ void AsmPrinter::emitFunctionBody() { if (MDNode *MD = MI.getPCSections()) emitPCSectionsLabel(*MF, *MD); - for (auto &Handler : DebugHandlers) + for (auto &Handler : Handlers) Handler->beginInstruction(&MI); if (isVerbose()) @@ -1832,7 +1832,7 @@ void AsmPrinter::emitFunctionBody() { if (MCSymbol *S = MI.getPostInstrSymbol()) OutStreamer->emitLabel(S); - for (auto &Handler : DebugHandlers) + for (auto &Handler : Handlers) Handler->endInstruction(); } @@ -1966,27 +1966,26 @@ void AsmPrinter::emitFunctionBody() { // Call endBasicBlockSection on the last block now, if it wasn't already // called. if (!MF->back().isEndSection()) { - for (auto &Handler : DebugHandlers) - Handler->endBasicBlockSection(MF->back()); for (auto &Handler : Handlers) Handler->endBasicBlockSection(MF->back()); + for (auto &Handler : EHHandlers) + Handler->endBasicBlockSection(MF->back()); } for (auto &Handler : Handlers) Handler->markFunctionEnd(); - - assert(!MBBSectionRanges.contains(MF->front().getSectionID()) && - "Overwrite section range"); - MBBSectionRanges[MF->front().getSectionID()] = - MBBSectionRange{CurrentFnBegin, CurrentFnEnd}; + for (auto &Handler : EHHandlers) + Handler->markFunctionEnd(); + // Update the end label of the entry block's section. + MBBSectionRanges[MF->front().getSectionID()].EndLabel = CurrentFnEnd; // Print out jump tables referenced by the function. emitJumpTableInfo(); // Emit post-function debug and/or EH information. - for (auto &Handler : DebugHandlers) - Handler->endFunction(MF); for (auto &Handler : Handlers) Handler->endFunction(MF); + for (auto &Handler : EHHandlers) + Handler->endFunction(MF); // Emit section containing BB address offsets and their metadata, when // BB labels are requested for this function. Skip empty functions. @@ -2425,17 +2424,16 @@ bool AsmPrinter::doFinalization(Module &M) { emitGlobalIFunc(M, IFunc); // Finalize debug and EH information. - for (auto &Handler : DebugHandlers) - Handler->endModule(); for (auto &Handler : Handlers) Handler->endModule(); + for (auto &Handler : EHHandlers) + Handler->endModule(); // This deletes all the ephemeral handlers that AsmPrinter added, while // keeping all the user-added handlers alive until the AsmPrinter is // destroyed. + EHHandlers.clear(); Handlers.erase(Handlers.begin() + NumUserHandlers, Handlers.end()); - DebugHandlers.erase(DebugHandlers.begin() + NumUserDebugHandlers, - DebugHandlers.end()); DD = nullptr; // If the target wants to know about weak references, print them all. @@ -3957,6 +3955,10 @@ void AsmPrinter::emitBasicBlockStart(const MachineBasicBlock &MBB) { Handler->endFunclet(); Handler->beginFunclet(MBB); } + for (auto &Handler : EHHandlers) { + Handler->endFunclet(); + Handler->beginFunclet(MBB); + } } // Switch to a new section if this basic block must begin a section. The @@ -3969,6 +3971,9 @@ void AsmPrinter::emitBasicBlockStart(const MachineBasicBlock &MBB) { CurrentSectionBeginSym = MBB.getSymbol(); } + for (auto &Handler : Handlers) + Handler->beginCodeAlignment(MBB); + // Emit an alignment directive for this block, if needed. const Align Alignment = MBB.getAlignment(); if (Alignment != Align(1)) @@ -4026,10 +4031,10 @@ void AsmPrinter::emitBasicBlockStart(const MachineBasicBlock &MBB) { // if it begins a section (Entry block call is handled separately, next to // beginFunction). if (MBB.isBeginSection() && !MBB.isEntryBlock()) { - for (auto &Handler : DebugHandlers) - Handler->beginBasicBlockSection(MBB); for (auto &Handler : Handlers) Handler->beginBasicBlockSection(MBB); + for (auto &Handler : EHHandlers) + Handler->beginBasicBlockSection(MBB); } } @@ -4037,10 +4042,10 @@ void AsmPrinter::emitBasicBlockEnd(const MachineBasicBlock &MBB) { // Check if CFI information needs to be updated for this MBB with basic block // sections. if (MBB.isEndSection()) { - for (auto &Handler : DebugHandlers) - Handler->endBasicBlockSection(MBB); for (auto &Handler : Handlers) Handler->endBasicBlockSection(MBB); + for (auto &Handler : EHHandlers) + Handler->endBasicBlockSection(MBB); } } @@ -4174,12 +4179,7 @@ void AsmPrinter::addAsmPrinterHandler( NumUserHandlers++; } -void AsmPrinter::addDebugHandler(std::unique_ptr Handler) { - DebugHandlers.insert(DebugHandlers.begin(), std::move(Handler)); - NumUserDebugHandlers++; -} - -/// Pin vtable to this file. +/// Pin vtables to this file. AsmPrinterHandler::~AsmPrinterHandler() = default; void AsmPrinterHandler::markFunctionEnd() {} diff --git a/llvm/lib/CodeGen/AsmPrinter/PseudoProbePrinter.h b/llvm/lib/CodeGen/AsmPrinter/PseudoProbePrinter.h index 35461e53fbf19..f11b552387501 100644 --- a/llvm/lib/CodeGen/AsmPrinter/PseudoProbePrinter.h +++ b/llvm/lib/CodeGen/AsmPrinter/PseudoProbePrinter.h @@ -14,7 +14,6 @@ #define LLVM_LIB_CODEGEN_ASMPRINTER_PSEUDOPROBEPRINTER_H #include "llvm/ADT/DenseMap.h" -#include "llvm/CodeGen/AsmPrinterHandler.h" namespace llvm { diff --git a/llvm/lib/Target/BPF/BPFAsmPrinter.cpp b/llvm/lib/Target/BPF/BPFAsmPrinter.cpp index a9ac4f651ea27..b99988a07607e 100644 --- a/llvm/lib/Target/BPF/BPFAsmPrinter.cpp +++ b/llvm/lib/Target/BPF/BPFAsmPrinter.cpp @@ -62,7 +62,7 @@ bool BPFAsmPrinter::doInitialization(Module &M) { // Only emit BTF when debuginfo available. if (MAI->doesSupportDebugInformation() && !M.debug_compile_units().empty()) { BTF = new BTFDebug(this); - DebugHandlers.push_back(std::unique_ptr(BTF)); + Handlers.push_back(std::unique_ptr(BTF)); } return false; diff --git a/llvm/unittests/CodeGen/AsmPrinterDwarfTest.cpp b/llvm/unittests/CodeGen/AsmPrinterDwarfTest.cpp index 1f3d7a55ee854..00c11dd741335 100644 --- a/llvm/unittests/CodeGen/AsmPrinterDwarfTest.cpp +++ b/llvm/unittests/CodeGen/AsmPrinterDwarfTest.cpp @@ -384,10 +384,13 @@ class AsmPrinterHandlerTest : public AsmPrinterFixtureBase { 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: @@ -424,54 +427,4 @@ TEST_F(AsmPrinterHandlerTest, Basic) { ASSERT_EQ(EndCount, 3); } -class AsmPrinterDebugHandlerTest : public AsmPrinterFixtureBase { - class TestDebugHandler : public DebugHandlerBase { - AsmPrinterDebugHandlerTest &Test; - - public: - TestDebugHandler(AsmPrinterDebugHandlerTest &Test, AsmPrinter *AP) - : DebugHandlerBase(AP), Test(Test) {} - virtual ~TestDebugHandler() {} - virtual void beginModule(Module *M) override { Test.BeginCount++; } - virtual void endModule() override { Test.EndCount++; } - virtual void beginFunctionImpl(const MachineFunction *MF) override {} - virtual void endFunctionImpl(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->addDebugHandler(std::make_unique(*this, AP)); - 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->addDebugHandler(std::make_unique(*this, AP)); - PM.run(*M); - return true; - } - - int BeginCount = 0; - int EndCount = 0; -}; - -TEST_F(AsmPrinterDebugHandlerTest, Basic) { - if (!init("x86_64-pc-linux", /*DwarfVersion=*/4, dwarf::DWARF32)) - GTEST_SKIP(); - - ASSERT_EQ(BeginCount, 3); - ASSERT_EQ(EndCount, 3); -} - } // end namespace From 9e772173566297e21c8d1b35cd5a73548ca46ec7 Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Fri, 7 Feb 2025 02:01:39 -0500 Subject: [PATCH 2/7] [analyzer] Do not destruct fields of unions (#122330) The C++ standard prohibits this implicit destructor call, leading to incorrect reports from clang-analyzer. This causes projects that use std::option (including llvm) to fail the cplusplus.NewDelete test incorrectly when run through the analyzer. Fixes #119415 (cherry picked from commit 9b8297bc7ea8f217a3f701afedd2c953a4ad7867) --- clang/lib/Analysis/CFG.cpp | 2 + .../test/Analysis/NewDelete-checker-test.cpp | 28 ++++++++++++++ clang/test/Analysis/dtor-union.cpp | 38 +++++++++++++++++++ 3 files changed, 68 insertions(+) create mode 100644 clang/test/Analysis/dtor-union.cpp diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp index 64e6155de090c..3030589769553 100644 --- a/clang/lib/Analysis/CFG.cpp +++ b/clang/lib/Analysis/CFG.cpp @@ -2034,6 +2034,8 @@ void CFGBuilder::addImplicitDtorsForDestructor(const CXXDestructorDecl *DD) { } // First destroy member objects. + if (RD->isUnion()) + return; for (auto *FI : RD->fields()) { // Check for constant size array. Set type to array element type. QualType QT = FI->getType(); diff --git a/clang/test/Analysis/NewDelete-checker-test.cpp b/clang/test/Analysis/NewDelete-checker-test.cpp index 1100b49e84d3a..5b0ccf921d4ea 100644 --- a/clang/test/Analysis/NewDelete-checker-test.cpp +++ b/clang/test/Analysis/NewDelete-checker-test.cpp @@ -458,3 +458,31 @@ void testLeakBecauseNTTPIsNotDeallocation() { void* p = ::operator new(10); deallocate_via_nttp(p); } // leak-warning{{Potential leak of memory pointed to by 'p'}} + +namespace optional_union { + template + class unique_ptr { + T *q; + public: + unique_ptr() : q(new T) {} + ~unique_ptr() { + delete q; + } + }; + + union custom_union_t { + unique_ptr present; + char notpresent; + custom_union_t() : present(unique_ptr()) {} + ~custom_union_t() {} + }; + + void testUnionCorrect() { + custom_union_t a; + a.present.~unique_ptr(); + } + + void testUnionLeak() { + custom_union_t a; + } // leak-warning{{Potential leak of memory pointed to by 'a.present.q'}} +} diff --git a/clang/test/Analysis/dtor-union.cpp b/clang/test/Analysis/dtor-union.cpp new file mode 100644 index 0000000000000..dac366e6f9df8 --- /dev/null +++ b/clang/test/Analysis/dtor-union.cpp @@ -0,0 +1,38 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config c++-inlining=destructors -verify -std=c++11 %s +// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config c++-inlining=destructors -verify -std=c++17 %s + +void clang_analyzer_eval(bool); + +struct InlineDtor { + static int cnt; + static int dtorCalled; + ~InlineDtor() { + ++dtorCalled; + } +}; + +int InlineDtor::cnt = 0; +int InlineDtor::dtorCalled = 0; + +void testUnionDtor() { + static int unionDtorCalled; + InlineDtor::cnt = 0; + InlineDtor::dtorCalled = 0; + unionDtorCalled = 0; + { + union UnionDtor { + InlineDtor kind1; + char kind2; + ~UnionDtor() { unionDtorCalled++; } + }; + UnionDtor u1{.kind1{}}; + UnionDtor u2{.kind2{}}; + auto u3 = new UnionDtor{.kind1{}}; + auto u4 = new UnionDtor{.kind2{}}; + delete u3; + delete u4; + } + + clang_analyzer_eval(unionDtorCalled == 4); // expected-warning {{TRUE}} + clang_analyzer_eval(InlineDtor::dtorCalled == 0); // expected-warning {{TRUE}} +} From bdbfd514634b40d456a48d0b7fb6f35939b3a7ea Mon Sep 17 00:00:00 2001 From: Zentrik Date: Tue, 4 Feb 2025 00:23:54 +0000 Subject: [PATCH 3/7] Fix build if ITTAPI_SOURCE_DIR is specified (#106924) de92615d68f allows specifying the source directory of ittapi. This change allows configuring the source directory of ittapi here as well. (cherry picked from commit f8287f6c373fcf993643dd6f0e30dde304c1be73) --- llvm/lib/ExecutionEngine/IntelJITEvents/CMakeLists.txt | 5 ++++- llvm/lib/ExecutionEngine/Orc/TargetProcess/CMakeLists.txt | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/llvm/lib/ExecutionEngine/IntelJITEvents/CMakeLists.txt b/llvm/lib/ExecutionEngine/IntelJITEvents/CMakeLists.txt index b4fd04d65e263..56c529c08937c 100644 --- a/llvm/lib/ExecutionEngine/IntelJITEvents/CMakeLists.txt +++ b/llvm/lib/ExecutionEngine/IntelJITEvents/CMakeLists.txt @@ -1,5 +1,8 @@ include_directories( ${CMAKE_CURRENT_SOURCE_DIR}/.. ) -include_directories( ${PROJECT_BINARY_DIR}/ittapi/include/ ) +if(NOT DEFINED ITTAPI_SOURCE_DIR) + set(ITTAPI_SOURCE_DIR ${PROJECT_BINARY_DIR}) +endif() +include_directories( ${ITTAPI_SOURCE_DIR}/ittapi/include/ ) add_llvm_component_library(LLVMIntelJITEvents IntelJITEventListener.cpp diff --git a/llvm/lib/ExecutionEngine/Orc/TargetProcess/CMakeLists.txt b/llvm/lib/ExecutionEngine/Orc/TargetProcess/CMakeLists.txt index 3d1dfe758c79d..03677d610cbb7 100644 --- a/llvm/lib/ExecutionEngine/Orc/TargetProcess/CMakeLists.txt +++ b/llvm/lib/ExecutionEngine/Orc/TargetProcess/CMakeLists.txt @@ -6,7 +6,10 @@ set(intel_jit_profiling ) if( LLVM_USE_INTEL_JITEVENTS ) set(intel_jit_profiling IntelJITProfiling) include_directories(${CMAKE_CURRENT_SOURCE_DIR}/../../IntelJITProfiling) - include_directories(${PROJECT_BINARY_DIR}/ittapi/include/ ) + if(NOT DEFINED ITTAPI_SOURCE_DIR) + set(ITTAPI_SOURCE_DIR ${PROJECT_BINARY_DIR}) + endif() + include_directories(${ITTAPI_SOURCE_DIR}/ittapi/include/ ) endif() add_llvm_component_library(LLVMOrcTargetProcess From 6e8ff5767e9466d73be85f1eaa760e2c94cdbd86 Mon Sep 17 00:00:00 2001 From: Zentrik Date: Wed, 9 Oct 2024 20:39:04 +0100 Subject: [PATCH 4/7] [ittapi] Check out branch only if ITTAPI is cloned In claude we trust Fix by not setting working directory as ittapi source dir might not exist. Also, quote paths in case of whitespace --- .../IntelJITEvents/CMakeLists.txt | 4 +-- .../IntelJITProfiling/CMakeLists.txt | 30 ++++++++++--------- .../Orc/TargetProcess/CMakeLists.txt | 4 +-- 3 files changed, 20 insertions(+), 18 deletions(-) diff --git a/llvm/lib/ExecutionEngine/IntelJITEvents/CMakeLists.txt b/llvm/lib/ExecutionEngine/IntelJITEvents/CMakeLists.txt index 56c529c08937c..08ce4c9399f17 100644 --- a/llvm/lib/ExecutionEngine/IntelJITEvents/CMakeLists.txt +++ b/llvm/lib/ExecutionEngine/IntelJITEvents/CMakeLists.txt @@ -1,8 +1,8 @@ include_directories( ${CMAKE_CURRENT_SOURCE_DIR}/.. ) if(NOT DEFINED ITTAPI_SOURCE_DIR) - set(ITTAPI_SOURCE_DIR ${PROJECT_BINARY_DIR}) + set(ITTAPI_SOURCE_DIR ${PROJECT_BINARY_DIR}/ittapi) endif() -include_directories( ${ITTAPI_SOURCE_DIR}/ittapi/include/ ) +include_directories( ${ITTAPI_SOURCE_DIR}/include/ ) add_llvm_component_library(LLVMIntelJITEvents IntelJITEventListener.cpp diff --git a/llvm/lib/ExecutionEngine/IntelJITProfiling/CMakeLists.txt b/llvm/lib/ExecutionEngine/IntelJITProfiling/CMakeLists.txt index 0aedadc65df05..bf6416b7a48e2 100644 --- a/llvm/lib/ExecutionEngine/IntelJITProfiling/CMakeLists.txt +++ b/llvm/lib/ExecutionEngine/IntelJITProfiling/CMakeLists.txt @@ -9,26 +9,28 @@ if(NOT DEFINED ITTAPI_GIT_TAG) endif() if(NOT DEFINED ITTAPI_SOURCE_DIR) - set(ITTAPI_SOURCE_DIR ${PROJECT_BINARY_DIR}) + set(ITTAPI_SOURCE_DIR ${PROJECT_BINARY_DIR}/ittapi) endif() -if(NOT EXISTS ${ITTAPI_SOURCE_DIR}/ittapi) - execute_process(COMMAND ${GIT_EXECUTABLE} clone ${ITTAPI_GIT_REPOSITORY} - WORKING_DIRECTORY ${ITTAPI_SOURCE_DIR} - RESULT_VARIABLE GIT_CLONE_RESULT) +if(NOT EXISTS "${ITTAPI_SOURCE_DIR}") + execute_process( + COMMAND "${GIT_EXECUTABLE}" clone "${ITTAPI_GIT_REPOSITORY}" "${ITTAPI_SOURCE_DIR}" + RESULT_VARIABLE GIT_CLONE_RESULT + ) if(NOT GIT_CLONE_RESULT EQUAL "0") - message(FATAL_ERROR "git clone ${ITTAPI_GIT_REPOSITORY} failed with ${GIT_CLONE_RESULT}, please clone ${ITTAPI_GIT_REPOSITORY}") + message(FATAL_ERROR "git clone ${ITTAPI_GIT_REPOSITORY} failed with ${GIT_CLONE_RESULT}, please clone ${ITTAPI_GIT_REPOSITORY} manually") endif() -endif() -execute_process(COMMAND ${GIT_EXECUTABLE} checkout ${ITTAPI_GIT_TAG} - WORKING_DIRECTORY ${ITTAPI_SOURCE_DIR}/ittapi - RESULT_VARIABLE GIT_CHECKOUT_RESULT) -if(NOT GIT_CHECKOUT_RESULT EQUAL "0") - message(FATAL_ERROR "git checkout ${ITTAPI_GIT_TAG} failed with ${GIT_CHECKOUT_RESULT}, please checkout ${ITTAPI_GIT_TAG} at ${ITTAPI_SOURCE_DIR}/ittapi") + execute_process( + COMMAND "${GIT_EXECUTABLE}" -C "${ITTAPI_SOURCE_DIR}" checkout "${ITTAPI_GIT_TAG}" + RESULT_VARIABLE GIT_CHECKOUT_RESULT + ) + if(NOT GIT_CHECKOUT_RESULT EQUAL "0") + message(FATAL_ERROR "git checkout ${ITTAPI_GIT_TAG} failed with ${GIT_CHECKOUT_RESULT}, please checkout ${ITTAPI_GIT_TAG} at ${ITTAPI_SOURCE_DIR} manually") + endif() endif() -include_directories( ${ITTAPI_SOURCE_DIR}/ittapi/include/ ) +include_directories( ${ITTAPI_SOURCE_DIR}/include/ ) if( HAVE_LIBDL ) set(LLVM_INTEL_JIT_LIBS ${CMAKE_DL_LIBS}) @@ -39,7 +41,7 @@ set(LLVM_INTEL_JIT_LIBS ${LLVM_PTHREAD_LIB} ${LLVM_INTEL_JIT_LIBS}) add_llvm_component_library(LLVMIntelJITProfiling jitprofiling.c - ${ITTAPI_SOURCE_DIR}/ittapi/src/ittnotify/ittnotify_static.c + ${ITTAPI_SOURCE_DIR}/src/ittnotify/ittnotify_static.c LINK_LIBS ${LLVM_INTEL_JIT_LIBS} diff --git a/llvm/lib/ExecutionEngine/Orc/TargetProcess/CMakeLists.txt b/llvm/lib/ExecutionEngine/Orc/TargetProcess/CMakeLists.txt index 03677d610cbb7..308d7012c4231 100644 --- a/llvm/lib/ExecutionEngine/Orc/TargetProcess/CMakeLists.txt +++ b/llvm/lib/ExecutionEngine/Orc/TargetProcess/CMakeLists.txt @@ -7,9 +7,9 @@ if( LLVM_USE_INTEL_JITEVENTS ) set(intel_jit_profiling IntelJITProfiling) include_directories(${CMAKE_CURRENT_SOURCE_DIR}/../../IntelJITProfiling) if(NOT DEFINED ITTAPI_SOURCE_DIR) - set(ITTAPI_SOURCE_DIR ${PROJECT_BINARY_DIR}) + set(ITTAPI_SOURCE_DIR ${PROJECT_BINARY_DIR}/ittapi) endif() - include_directories(${ITTAPI_SOURCE_DIR}/ittapi/include/ ) + include_directories(${ITTAPI_SOURCE_DIR}/include/ ) endif() add_llvm_component_library(LLVMOrcTargetProcess From f65733cda5916bac4a32bee698067255163c53c4 Mon Sep 17 00:00:00 2001 From: Zentrik Date: Sat, 8 Feb 2025 19:53:09 +0000 Subject: [PATCH 5/7] Revert "[AsmPrinter] Reintroduce full AsmPrinterHandler API (#122297)" This reverts commit 4d27cafc23312c7e584e1d80c4ba6ee16ff375e8. --- llvm/include/llvm/CodeGen/AsmPrinter.h | 17 +++-- llvm/include/llvm/CodeGen/AsmPrinterHandler.h | 12 ---- llvm/include/llvm/CodeGen/DebugHandlerBase.h | 26 ++++--- llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp | 68 +++++++++---------- .../CodeGen/AsmPrinter/PseudoProbePrinter.h | 1 + llvm/lib/Target/BPF/BPFAsmPrinter.cpp | 2 +- .../unittests/CodeGen/AsmPrinterDwarfTest.cpp | 53 ++++++++++++++- 7 files changed, 110 insertions(+), 69 deletions(-) diff --git a/llvm/include/llvm/CodeGen/AsmPrinter.h b/llvm/include/llvm/CodeGen/AsmPrinter.h index c7a4d3c3bfe24..f57be39076a78 100644 --- a/llvm/include/llvm/CodeGen/AsmPrinter.h +++ b/llvm/include/llvm/CodeGen/AsmPrinter.h @@ -44,7 +44,6 @@ class DebugHandlerBase; class DIE; class DIEAbbrev; class DwarfDebug; -class EHStreamer; class GCMetadataPrinter; class GCStrategy; class GlobalAlias; @@ -188,17 +187,15 @@ class AsmPrinter : public MachineFunctionPass { /// For dso_local functions, the current $local alias for the function. MCSymbol *CurrentFnBeginLocal = nullptr; - /// A handle to the EH info emitter (if present). - // Only for EHStreamer subtypes, but some C++ compilers will incorrectly warn - // us if we declare that directly. - SmallVector, 1> EHHandlers; - - // A vector of all Debuginfo emitters we should use. Protected so that - // targets can add their own. This vector maintains ownership of the - // emitters. + /// A vector of all debug/EH info emitters we should use. This vector + /// maintains ownership of the emitters. SmallVector, 2> Handlers; size_t NumUserHandlers = 0; + /// Debuginfo handler. Protected so that targets can add their own. + SmallVector, 1> DebugHandlers; + size_t NumUserDebugHandlers = 0; + StackMaps SM; private: @@ -524,6 +521,8 @@ class AsmPrinter : public MachineFunctionPass { void addAsmPrinterHandler(std::unique_ptr Handler); + void addDebugHandler(std::unique_ptr Handler); + // 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 bf3f6c53027a7..ed73e618431de 100644 --- a/llvm/include/llvm/CodeGen/AsmPrinterHandler.h +++ b/llvm/include/llvm/CodeGen/AsmPrinterHandler.h @@ -64,18 +64,6 @@ class AsmPrinterHandler { /// immediately prior to markFunctionEnd. virtual void endBasicBlockSection(const MachineBasicBlock &MBB) {} - /// For symbols that have a size designated (e.g. common symbols), - /// this tracks that size. - virtual void setSymbolSize(const MCSymbol *Sym, uint64_t Size) {} - - /// Process beginning of an instruction. - virtual void beginInstruction(const MachineInstr *MI) {} - - /// Process end of an instruction. - virtual void endInstruction() {} - - virtual void beginCodeAlignment(const MachineBasicBlock &MBB) {} - /// Emit target-specific EH funclet machinery. virtual void beginFunclet(const MachineBasicBlock &MBB, MCSymbol *Sym = nullptr) {} diff --git a/llvm/include/llvm/CodeGen/DebugHandlerBase.h b/llvm/include/llvm/CodeGen/DebugHandlerBase.h index 17764b31f30eb..36a844e7087fa 100644 --- a/llvm/include/llvm/CodeGen/DebugHandlerBase.h +++ b/llvm/include/llvm/CodeGen/DebugHandlerBase.h @@ -50,10 +50,14 @@ struct DbgVariableLocation { /// Base class for debug information backends. Common functionality related to /// tracking which variables and scopes are alive at a given PC live here. -class DebugHandlerBase : public AsmPrinterHandler { +class DebugHandlerBase { protected: DebugHandlerBase(AsmPrinter *A); +public: + virtual ~DebugHandlerBase(); + +protected: /// Target of debug info emission. AsmPrinter *Asm = nullptr; @@ -116,20 +120,22 @@ class DebugHandlerBase : public AsmPrinterHandler { private: InstructionOrdering InstOrdering; - // AsmPrinterHandler overrides. public: - virtual ~DebugHandlerBase() override; + /// For symbols that have a size designated (e.g. common symbols), + /// this tracks that size. Only used by DWARF. + virtual void setSymbolSize(const MCSymbol *Sym, uint64_t Size) {} - void beginModule(Module *M) override; + virtual void beginModule(Module *M); + virtual void endModule() = 0; - void beginInstruction(const MachineInstr *MI) override; - void endInstruction() override; + virtual void beginInstruction(const MachineInstr *MI); + virtual void endInstruction(); - void beginFunction(const MachineFunction *MF) override; - void endFunction(const MachineFunction *MF) override; + void beginFunction(const MachineFunction *MF); + void endFunction(const MachineFunction *MF); - void beginBasicBlockSection(const MachineBasicBlock &MBB) override; - void endBasicBlockSection(const MachineBasicBlock &MBB) override; + void beginBasicBlockSection(const MachineBasicBlock &MBB); + void endBasicBlockSection(const MachineBasicBlock &MBB); /// Return Label preceding the instruction. MCSymbol *getLabelBeforeInsn(const MachineInstr *MI); diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp index 6d510d11c38e6..2297b27ffdc07 100644 --- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp +++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp @@ -539,12 +539,12 @@ bool AsmPrinter::doInitialization(Module &M) { if (MAI->doesSupportDebugInformation()) { bool EmitCodeView = M.getCodeViewFlag(); if (EmitCodeView && TM.getTargetTriple().isOSWindows()) - Handlers.push_back(std::make_unique(this)); + DebugHandlers.push_back(std::make_unique(this)); if (!EmitCodeView || M.getDwarfVersion()) { assert(MMI && "MMI could not be nullptr here!"); if (MMI->hasDebugInfo()) { DD = new DwarfDebug(this); - Handlers.push_back(std::unique_ptr(DD)); + DebugHandlers.push_back(std::unique_ptr(DD)); } } } @@ -611,11 +611,11 @@ bool AsmPrinter::doInitialization(Module &M) { // Emit tables for any value of cfguard flag (i.e. cfguard=1 or cfguard=2). if (mdconst::extract_or_null(M.getModuleFlag("cfguard"))) - EHHandlers.push_back(std::make_unique(this)); + Handlers.push_back(std::make_unique(this)); - for (auto &Handler : Handlers) + for (auto &Handler : DebugHandlers) Handler->beginModule(&M); - for (auto &Handler : EHHandlers) + for (auto &Handler : Handlers) Handler->beginModule(&M); return false; @@ -763,7 +763,7 @@ void AsmPrinter::emitGlobalVariable(const GlobalVariable *GV) { // sections and expected to be contiguous (e.g. ObjC metadata). const Align Alignment = getGVAlignment(GV, DL); - for (auto &Handler : Handlers) + for (auto &Handler : DebugHandlers) Handler->setSymbolSize(GVSym, Size); // Handle common symbols @@ -1035,14 +1035,14 @@ void AsmPrinter::emitFunctionHeader() { } // Emit pre-function debug and/or EH information. - for (auto &Handler : Handlers) { + for (auto &Handler : DebugHandlers) { Handler->beginFunction(MF); Handler->beginBasicBlockSection(MF->front()); } - for (auto &Handler : EHHandlers) { + for (auto &Handler : Handlers) Handler->beginFunction(MF); + for (auto &Handler : Handlers) Handler->beginBasicBlockSection(MF->front()); - } // Emit the prologue data. if (F.hasPrologueData()) @@ -1737,7 +1737,7 @@ void AsmPrinter::emitFunctionBody() { if (MDNode *MD = MI.getPCSections()) emitPCSectionsLabel(*MF, *MD); - for (auto &Handler : Handlers) + for (auto &Handler : DebugHandlers) Handler->beginInstruction(&MI); if (isVerbose()) @@ -1832,7 +1832,7 @@ void AsmPrinter::emitFunctionBody() { if (MCSymbol *S = MI.getPostInstrSymbol()) OutStreamer->emitLabel(S); - for (auto &Handler : Handlers) + for (auto &Handler : DebugHandlers) Handler->endInstruction(); } @@ -1966,25 +1966,26 @@ void AsmPrinter::emitFunctionBody() { // Call endBasicBlockSection on the last block now, if it wasn't already // called. if (!MF->back().isEndSection()) { - for (auto &Handler : Handlers) + for (auto &Handler : DebugHandlers) Handler->endBasicBlockSection(MF->back()); - for (auto &Handler : EHHandlers) + for (auto &Handler : Handlers) Handler->endBasicBlockSection(MF->back()); } for (auto &Handler : Handlers) Handler->markFunctionEnd(); - for (auto &Handler : EHHandlers) - Handler->markFunctionEnd(); - // Update the end label of the entry block's section. - MBBSectionRanges[MF->front().getSectionID()].EndLabel = CurrentFnEnd; + + assert(!MBBSectionRanges.contains(MF->front().getSectionID()) && + "Overwrite section range"); + MBBSectionRanges[MF->front().getSectionID()] = + MBBSectionRange{CurrentFnBegin, CurrentFnEnd}; // Print out jump tables referenced by the function. emitJumpTableInfo(); // Emit post-function debug and/or EH information. - for (auto &Handler : Handlers) + for (auto &Handler : DebugHandlers) Handler->endFunction(MF); - for (auto &Handler : EHHandlers) + for (auto &Handler : Handlers) Handler->endFunction(MF); // Emit section containing BB address offsets and their metadata, when @@ -2424,16 +2425,17 @@ bool AsmPrinter::doFinalization(Module &M) { emitGlobalIFunc(M, IFunc); // Finalize debug and EH information. - for (auto &Handler : Handlers) + for (auto &Handler : DebugHandlers) Handler->endModule(); - for (auto &Handler : EHHandlers) + for (auto &Handler : Handlers) Handler->endModule(); // This deletes all the ephemeral handlers that AsmPrinter added, while // keeping all the user-added handlers alive until the AsmPrinter is // destroyed. - EHHandlers.clear(); Handlers.erase(Handlers.begin() + NumUserHandlers, Handlers.end()); + DebugHandlers.erase(DebugHandlers.begin() + NumUserDebugHandlers, + DebugHandlers.end()); DD = nullptr; // If the target wants to know about weak references, print them all. @@ -3955,10 +3957,6 @@ void AsmPrinter::emitBasicBlockStart(const MachineBasicBlock &MBB) { Handler->endFunclet(); Handler->beginFunclet(MBB); } - for (auto &Handler : EHHandlers) { - Handler->endFunclet(); - Handler->beginFunclet(MBB); - } } // Switch to a new section if this basic block must begin a section. The @@ -3971,9 +3969,6 @@ void AsmPrinter::emitBasicBlockStart(const MachineBasicBlock &MBB) { CurrentSectionBeginSym = MBB.getSymbol(); } - for (auto &Handler : Handlers) - Handler->beginCodeAlignment(MBB); - // Emit an alignment directive for this block, if needed. const Align Alignment = MBB.getAlignment(); if (Alignment != Align(1)) @@ -4031,9 +4026,9 @@ void AsmPrinter::emitBasicBlockStart(const MachineBasicBlock &MBB) { // if it begins a section (Entry block call is handled separately, next to // beginFunction). if (MBB.isBeginSection() && !MBB.isEntryBlock()) { - for (auto &Handler : Handlers) + for (auto &Handler : DebugHandlers) Handler->beginBasicBlockSection(MBB); - for (auto &Handler : EHHandlers) + for (auto &Handler : Handlers) Handler->beginBasicBlockSection(MBB); } } @@ -4042,9 +4037,9 @@ void AsmPrinter::emitBasicBlockEnd(const MachineBasicBlock &MBB) { // Check if CFI information needs to be updated for this MBB with basic block // sections. if (MBB.isEndSection()) { - for (auto &Handler : Handlers) + for (auto &Handler : DebugHandlers) Handler->endBasicBlockSection(MBB); - for (auto &Handler : EHHandlers) + for (auto &Handler : Handlers) Handler->endBasicBlockSection(MBB); } } @@ -4179,7 +4174,12 @@ void AsmPrinter::addAsmPrinterHandler( NumUserHandlers++; } -/// Pin vtables to this file. +void AsmPrinter::addDebugHandler(std::unique_ptr Handler) { + DebugHandlers.insert(DebugHandlers.begin(), std::move(Handler)); + NumUserDebugHandlers++; +} + +/// Pin vtable to this file. AsmPrinterHandler::~AsmPrinterHandler() = default; void AsmPrinterHandler::markFunctionEnd() {} diff --git a/llvm/lib/CodeGen/AsmPrinter/PseudoProbePrinter.h b/llvm/lib/CodeGen/AsmPrinter/PseudoProbePrinter.h index f11b552387501..35461e53fbf19 100644 --- a/llvm/lib/CodeGen/AsmPrinter/PseudoProbePrinter.h +++ b/llvm/lib/CodeGen/AsmPrinter/PseudoProbePrinter.h @@ -14,6 +14,7 @@ #define LLVM_LIB_CODEGEN_ASMPRINTER_PSEUDOPROBEPRINTER_H #include "llvm/ADT/DenseMap.h" +#include "llvm/CodeGen/AsmPrinterHandler.h" namespace llvm { diff --git a/llvm/lib/Target/BPF/BPFAsmPrinter.cpp b/llvm/lib/Target/BPF/BPFAsmPrinter.cpp index b99988a07607e..a9ac4f651ea27 100644 --- a/llvm/lib/Target/BPF/BPFAsmPrinter.cpp +++ b/llvm/lib/Target/BPF/BPFAsmPrinter.cpp @@ -62,7 +62,7 @@ bool BPFAsmPrinter::doInitialization(Module &M) { // Only emit BTF when debuginfo available. if (MAI->doesSupportDebugInformation() && !M.debug_compile_units().empty()) { BTF = new BTFDebug(this); - Handlers.push_back(std::unique_ptr(BTF)); + DebugHandlers.push_back(std::unique_ptr(BTF)); } return false; diff --git a/llvm/unittests/CodeGen/AsmPrinterDwarfTest.cpp b/llvm/unittests/CodeGen/AsmPrinterDwarfTest.cpp index 00c11dd741335..1f3d7a55ee854 100644 --- a/llvm/unittests/CodeGen/AsmPrinterDwarfTest.cpp +++ b/llvm/unittests/CodeGen/AsmPrinterDwarfTest.cpp @@ -384,13 +384,10 @@ class AsmPrinterHandlerTest : public AsmPrinterFixtureBase { 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: @@ -427,4 +424,54 @@ TEST_F(AsmPrinterHandlerTest, Basic) { ASSERT_EQ(EndCount, 3); } +class AsmPrinterDebugHandlerTest : public AsmPrinterFixtureBase { + class TestDebugHandler : public DebugHandlerBase { + AsmPrinterDebugHandlerTest &Test; + + public: + TestDebugHandler(AsmPrinterDebugHandlerTest &Test, AsmPrinter *AP) + : DebugHandlerBase(AP), Test(Test) {} + virtual ~TestDebugHandler() {} + virtual void beginModule(Module *M) override { Test.BeginCount++; } + virtual void endModule() override { Test.EndCount++; } + virtual void beginFunctionImpl(const MachineFunction *MF) override {} + virtual void endFunctionImpl(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->addDebugHandler(std::make_unique(*this, AP)); + 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->addDebugHandler(std::make_unique(*this, AP)); + PM.run(*M); + return true; + } + + int BeginCount = 0; + int EndCount = 0; +}; + +TEST_F(AsmPrinterDebugHandlerTest, Basic) { + if (!init("x86_64-pc-linux", /*DwarfVersion=*/4, dwarf::DWARF32)) + GTEST_SKIP(); + + ASSERT_EQ(BeginCount, 3); + ASSERT_EQ(EndCount, 3); +} + } // end namespace From d33fa68813f970c123e6a2da064b580d9e90b58d Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Thu, 16 Jan 2025 11:00:06 -0500 Subject: [PATCH 6/7] [AsmPrinter] Reintroduce full AsmPrinterHandler API (#122297) This restores the functionality of AsmPrinterHandlers to what it was prior to https://github.com/llvm/llvm-project/pull/96785. The attempted hack there of adding a duplicate DebugHandlerBase handling added a lot of hidden state and assumptions, which just segfaulted when we tried to continuing using this API. Instead, this just goes back to the old design, but adds a separate array for the basic EH handles. The duplicate array is identical to the other array of handler, but which doesn't get their begin/endInstruction callbacks called. This still saves the negligible but measurable amount of virtual function calls as was the goal of #96785, while restoring the API to the pre-LLVM-19 status quo. --- llvm/include/llvm/CodeGen/AsmPrinter.h | 17 +++--- llvm/include/llvm/CodeGen/AsmPrinterHandler.h | 10 ++++ llvm/include/llvm/CodeGen/DebugHandlerBase.h | 27 ++++----- llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp | 58 +++++++++---------- .../CodeGen/AsmPrinter/PseudoProbePrinter.h | 1 - llvm/lib/Target/BPF/BPFAsmPrinter.cpp | 2 +- .../unittests/CodeGen/AsmPrinterDwarfTest.cpp | 53 +---------------- 7 files changed, 63 insertions(+), 105 deletions(-) diff --git a/llvm/include/llvm/CodeGen/AsmPrinter.h b/llvm/include/llvm/CodeGen/AsmPrinter.h index f57be39076a78..c7a4d3c3bfe24 100644 --- a/llvm/include/llvm/CodeGen/AsmPrinter.h +++ b/llvm/include/llvm/CodeGen/AsmPrinter.h @@ -44,6 +44,7 @@ class DebugHandlerBase; class DIE; class DIEAbbrev; class DwarfDebug; +class EHStreamer; class GCMetadataPrinter; class GCStrategy; class GlobalAlias; @@ -187,15 +188,17 @@ class AsmPrinter : public MachineFunctionPass { /// For dso_local functions, the current $local alias for the function. MCSymbol *CurrentFnBeginLocal = nullptr; - /// A vector of all debug/EH info emitters we should use. This vector - /// maintains ownership of the emitters. + /// A handle to the EH info emitter (if present). + // Only for EHStreamer subtypes, but some C++ compilers will incorrectly warn + // us if we declare that directly. + SmallVector, 1> EHHandlers; + + // A vector of all Debuginfo emitters we should use. Protected so that + // targets can add their own. This vector maintains ownership of the + // emitters. SmallVector, 2> Handlers; size_t NumUserHandlers = 0; - /// Debuginfo handler. Protected so that targets can add their own. - SmallVector, 1> DebugHandlers; - size_t NumUserDebugHandlers = 0; - StackMaps SM; private: @@ -521,8 +524,6 @@ class AsmPrinter : public MachineFunctionPass { void addAsmPrinterHandler(std::unique_ptr Handler); - void addDebugHandler(std::unique_ptr Handler); - // 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 ed73e618431de..0d13f8be0865a 100644 --- a/llvm/include/llvm/CodeGen/AsmPrinterHandler.h +++ b/llvm/include/llvm/CodeGen/AsmPrinterHandler.h @@ -64,6 +64,16 @@ class AsmPrinterHandler { /// immediately prior to markFunctionEnd. virtual void endBasicBlockSection(const MachineBasicBlock &MBB) {} + /// For symbols that have a size designated (e.g. common symbols), + /// this tracks that size. + virtual void setSymbolSize(const MCSymbol *Sym, uint64_t Size) {} + + /// Process beginning of an instruction. + virtual void beginInstruction(const MachineInstr *MI) {} + + /// Process end of an instruction. + virtual void endInstruction() {} + /// Emit target-specific EH funclet machinery. virtual void beginFunclet(const MachineBasicBlock &MBB, MCSymbol *Sym = nullptr) {} diff --git a/llvm/include/llvm/CodeGen/DebugHandlerBase.h b/llvm/include/llvm/CodeGen/DebugHandlerBase.h index 36a844e7087fa..59a67c245455c 100644 --- a/llvm/include/llvm/CodeGen/DebugHandlerBase.h +++ b/llvm/include/llvm/CodeGen/DebugHandlerBase.h @@ -50,14 +50,10 @@ struct DbgVariableLocation { /// Base class for debug information backends. Common functionality related to /// tracking which variables and scopes are alive at a given PC live here. -class DebugHandlerBase { +class DebugHandlerBase : public AsmPrinterHandler { protected: DebugHandlerBase(AsmPrinter *A); -public: - virtual ~DebugHandlerBase(); - -protected: /// Target of debug info emission. AsmPrinter *Asm = nullptr; @@ -120,22 +116,21 @@ class DebugHandlerBase { private: InstructionOrdering InstOrdering; + // AsmPrinterHandler overrides. public: - /// For symbols that have a size designated (e.g. common symbols), - /// this tracks that size. Only used by DWARF. - virtual void setSymbolSize(const MCSymbol *Sym, uint64_t Size) {} + virtual ~DebugHandlerBase() override; + + void beginModule(Module *M) override; - virtual void beginModule(Module *M); - virtual void endModule() = 0; + void beginInstruction(const MachineInstr *MI) override; + void endInstruction() override; - virtual void beginInstruction(const MachineInstr *MI); - virtual void endInstruction(); + void beginFunction(const MachineFunction *MF) override; + void endFunction(const MachineFunction *MF) override; - void beginFunction(const MachineFunction *MF); - void endFunction(const MachineFunction *MF); - void beginBasicBlockSection(const MachineBasicBlock &MBB); - void endBasicBlockSection(const MachineBasicBlock &MBB); + 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/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp index 2297b27ffdc07..676d99fb8e64c 100644 --- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp +++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp @@ -539,12 +539,12 @@ bool AsmPrinter::doInitialization(Module &M) { if (MAI->doesSupportDebugInformation()) { bool EmitCodeView = M.getCodeViewFlag(); if (EmitCodeView && TM.getTargetTriple().isOSWindows()) - DebugHandlers.push_back(std::make_unique(this)); + Handlers.push_back(std::make_unique(this)); if (!EmitCodeView || M.getDwarfVersion()) { assert(MMI && "MMI could not be nullptr here!"); if (MMI->hasDebugInfo()) { DD = new DwarfDebug(this); - DebugHandlers.push_back(std::unique_ptr(DD)); + Handlers.push_back(std::unique_ptr(DD)); } } } @@ -611,12 +611,12 @@ bool AsmPrinter::doInitialization(Module &M) { // Emit tables for any value of cfguard flag (i.e. cfguard=1 or cfguard=2). if (mdconst::extract_or_null(M.getModuleFlag("cfguard"))) - Handlers.push_back(std::make_unique(this)); + EHHandlers.push_back(std::make_unique(this)); - for (auto &Handler : DebugHandlers) - Handler->beginModule(&M); for (auto &Handler : Handlers) Handler->beginModule(&M); + for (auto &Handler : EHHandlers) + Handler->beginModule(&M); return false; } @@ -763,7 +763,7 @@ void AsmPrinter::emitGlobalVariable(const GlobalVariable *GV) { // sections and expected to be contiguous (e.g. ObjC metadata). const Align Alignment = getGVAlignment(GV, DL); - for (auto &Handler : DebugHandlers) + for (auto &Handler : Handlers) Handler->setSymbolSize(GVSym, Size); // Handle common symbols @@ -1035,14 +1035,14 @@ void AsmPrinter::emitFunctionHeader() { } // Emit pre-function debug and/or EH information. - for (auto &Handler : DebugHandlers) { + for (auto &Handler : Handlers) { Handler->beginFunction(MF); Handler->beginBasicBlockSection(MF->front()); } - for (auto &Handler : Handlers) + for (auto &Handler : EHHandlers) { Handler->beginFunction(MF); - for (auto &Handler : Handlers) Handler->beginBasicBlockSection(MF->front()); + } // Emit the prologue data. if (F.hasPrologueData()) @@ -1737,7 +1737,7 @@ void AsmPrinter::emitFunctionBody() { if (MDNode *MD = MI.getPCSections()) emitPCSectionsLabel(*MF, *MD); - for (auto &Handler : DebugHandlers) + for (auto &Handler : Handlers) Handler->beginInstruction(&MI); if (isVerbose()) @@ -1832,7 +1832,7 @@ void AsmPrinter::emitFunctionBody() { if (MCSymbol *S = MI.getPostInstrSymbol()) OutStreamer->emitLabel(S); - for (auto &Handler : DebugHandlers) + for (auto &Handler : Handlers) Handler->endInstruction(); } @@ -1966,13 +1966,15 @@ void AsmPrinter::emitFunctionBody() { // Call endBasicBlockSection on the last block now, if it wasn't already // called. if (!MF->back().isEndSection()) { - for (auto &Handler : DebugHandlers) - Handler->endBasicBlockSection(MF->back()); for (auto &Handler : Handlers) Handler->endBasicBlockSection(MF->back()); + for (auto &Handler : EHHandlers) + Handler->endBasicBlockSection(MF->back()); } for (auto &Handler : Handlers) Handler->markFunctionEnd(); + for (auto &Handler : EHHandlers) + Handler->markFunctionEnd(); assert(!MBBSectionRanges.contains(MF->front().getSectionID()) && "Overwrite section range"); @@ -1983,10 +1985,10 @@ void AsmPrinter::emitFunctionBody() { emitJumpTableInfo(); // Emit post-function debug and/or EH information. - for (auto &Handler : DebugHandlers) - Handler->endFunction(MF); for (auto &Handler : Handlers) Handler->endFunction(MF); + for (auto &Handler : EHHandlers) + Handler->endFunction(MF); // Emit section containing BB address offsets and their metadata, when // BB labels are requested for this function. Skip empty functions. @@ -2425,17 +2427,16 @@ bool AsmPrinter::doFinalization(Module &M) { emitGlobalIFunc(M, IFunc); // Finalize debug and EH information. - for (auto &Handler : DebugHandlers) - Handler->endModule(); for (auto &Handler : Handlers) Handler->endModule(); + for (auto &Handler : EHHandlers) + Handler->endModule(); // This deletes all the ephemeral handlers that AsmPrinter added, while // keeping all the user-added handlers alive until the AsmPrinter is // destroyed. + EHHandlers.clear(); Handlers.erase(Handlers.begin() + NumUserHandlers, Handlers.end()); - DebugHandlers.erase(DebugHandlers.begin() + NumUserDebugHandlers, - DebugHandlers.end()); DD = nullptr; // If the target wants to know about weak references, print them all. @@ -3957,6 +3958,10 @@ void AsmPrinter::emitBasicBlockStart(const MachineBasicBlock &MBB) { Handler->endFunclet(); Handler->beginFunclet(MBB); } + for (auto &Handler : EHHandlers) { + Handler->endFunclet(); + Handler->beginFunclet(MBB); + } } // Switch to a new section if this basic block must begin a section. The @@ -4026,10 +4031,10 @@ void AsmPrinter::emitBasicBlockStart(const MachineBasicBlock &MBB) { // if it begins a section (Entry block call is handled separately, next to // beginFunction). if (MBB.isBeginSection() && !MBB.isEntryBlock()) { - for (auto &Handler : DebugHandlers) - Handler->beginBasicBlockSection(MBB); for (auto &Handler : Handlers) Handler->beginBasicBlockSection(MBB); + for (auto &Handler : EHHandlers) + Handler->beginBasicBlockSection(MBB); } } @@ -4037,10 +4042,10 @@ void AsmPrinter::emitBasicBlockEnd(const MachineBasicBlock &MBB) { // Check if CFI information needs to be updated for this MBB with basic block // sections. if (MBB.isEndSection()) { - for (auto &Handler : DebugHandlers) - Handler->endBasicBlockSection(MBB); for (auto &Handler : Handlers) Handler->endBasicBlockSection(MBB); + for (auto &Handler : EHHandlers) + Handler->endBasicBlockSection(MBB); } } @@ -4174,12 +4179,7 @@ void AsmPrinter::addAsmPrinterHandler( NumUserHandlers++; } -void AsmPrinter::addDebugHandler(std::unique_ptr Handler) { - DebugHandlers.insert(DebugHandlers.begin(), std::move(Handler)); - NumUserDebugHandlers++; -} - -/// Pin vtable to this file. +/// Pin vtables to this file. AsmPrinterHandler::~AsmPrinterHandler() = default; void AsmPrinterHandler::markFunctionEnd() {} diff --git a/llvm/lib/CodeGen/AsmPrinter/PseudoProbePrinter.h b/llvm/lib/CodeGen/AsmPrinter/PseudoProbePrinter.h index 35461e53fbf19..f11b552387501 100644 --- a/llvm/lib/CodeGen/AsmPrinter/PseudoProbePrinter.h +++ b/llvm/lib/CodeGen/AsmPrinter/PseudoProbePrinter.h @@ -14,7 +14,6 @@ #define LLVM_LIB_CODEGEN_ASMPRINTER_PSEUDOPROBEPRINTER_H #include "llvm/ADT/DenseMap.h" -#include "llvm/CodeGen/AsmPrinterHandler.h" namespace llvm { diff --git a/llvm/lib/Target/BPF/BPFAsmPrinter.cpp b/llvm/lib/Target/BPF/BPFAsmPrinter.cpp index a9ac4f651ea27..b99988a07607e 100644 --- a/llvm/lib/Target/BPF/BPFAsmPrinter.cpp +++ b/llvm/lib/Target/BPF/BPFAsmPrinter.cpp @@ -62,7 +62,7 @@ bool BPFAsmPrinter::doInitialization(Module &M) { // Only emit BTF when debuginfo available. if (MAI->doesSupportDebugInformation() && !M.debug_compile_units().empty()) { BTF = new BTFDebug(this); - DebugHandlers.push_back(std::unique_ptr(BTF)); + Handlers.push_back(std::unique_ptr(BTF)); } return false; diff --git a/llvm/unittests/CodeGen/AsmPrinterDwarfTest.cpp b/llvm/unittests/CodeGen/AsmPrinterDwarfTest.cpp index 1f3d7a55ee854..00c11dd741335 100644 --- a/llvm/unittests/CodeGen/AsmPrinterDwarfTest.cpp +++ b/llvm/unittests/CodeGen/AsmPrinterDwarfTest.cpp @@ -384,10 +384,13 @@ class AsmPrinterHandlerTest : public AsmPrinterFixtureBase { 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: @@ -424,54 +427,4 @@ TEST_F(AsmPrinterHandlerTest, Basic) { ASSERT_EQ(EndCount, 3); } -class AsmPrinterDebugHandlerTest : public AsmPrinterFixtureBase { - class TestDebugHandler : public DebugHandlerBase { - AsmPrinterDebugHandlerTest &Test; - - public: - TestDebugHandler(AsmPrinterDebugHandlerTest &Test, AsmPrinter *AP) - : DebugHandlerBase(AP), Test(Test) {} - virtual ~TestDebugHandler() {} - virtual void beginModule(Module *M) override { Test.BeginCount++; } - virtual void endModule() override { Test.EndCount++; } - virtual void beginFunctionImpl(const MachineFunction *MF) override {} - virtual void endFunctionImpl(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->addDebugHandler(std::make_unique(*this, AP)); - 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->addDebugHandler(std::make_unique(*this, AP)); - PM.run(*M); - return true; - } - - int BeginCount = 0; - int EndCount = 0; -}; - -TEST_F(AsmPrinterDebugHandlerTest, Basic) { - if (!init("x86_64-pc-linux", /*DwarfVersion=*/4, dwarf::DWARF32)) - GTEST_SKIP(); - - ASSERT_EQ(BeginCount, 3); - ASSERT_EQ(EndCount, 3); -} - } // end namespace From 82ee50754d4797500beaf24da05bf0118c544c70 Mon Sep 17 00:00:00 2001 From: Zentrik Date: Sat, 8 Feb 2025 20:17:20 +0000 Subject: [PATCH 7/7] Remove errant whitespace change --- llvm/include/llvm/CodeGen/DebugHandlerBase.h | 1 - 1 file changed, 1 deletion(-) diff --git a/llvm/include/llvm/CodeGen/DebugHandlerBase.h b/llvm/include/llvm/CodeGen/DebugHandlerBase.h index 59a67c245455c..17764b31f30eb 100644 --- a/llvm/include/llvm/CodeGen/DebugHandlerBase.h +++ b/llvm/include/llvm/CodeGen/DebugHandlerBase.h @@ -128,7 +128,6 @@ class DebugHandlerBase : public AsmPrinterHandler { void beginFunction(const MachineFunction *MF) override; void endFunction(const MachineFunction *MF) override; - void beginBasicBlockSection(const MachineBasicBlock &MBB) override; void endBasicBlockSection(const MachineBasicBlock &MBB) override;