Skip to content

[AsmPrinter] Reduce AsmPrinterHandlers virt. fn calls #96785

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jul 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 16 additions & 6 deletions llvm/include/llvm/CodeGen/AsmPrinter.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "llvm/ADT/SmallVector.h"
#include "llvm/BinaryFormat/Dwarf.h"
#include "llvm/CodeGen/AsmPrinterHandler.h"
#include "llvm/CodeGen/DebugHandlerBase.h"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be avoided somehow? It pulls the entire kitchen sink into AsmPrinter.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right now, I don't think so, HandlerInfo is templated and unique_ptr requires the definition for the destructor. After #97046, which removes HandlerInfo, it should be possible with an out-of-line destructor of AsmPrinter. I'll check next week (I won't merge before then anyway) -- thanks for pointing this out.

#include "llvm/CodeGen/DwarfStringPoolEntry.h"
#include "llvm/CodeGen/MachineFunctionPass.h"
#include "llvm/CodeGen/StackMaps.h"
Expand Down Expand Up @@ -145,14 +146,14 @@ class AsmPrinter : public MachineFunctionPass {

/// struct HandlerInfo and Handlers permit users or target extended
/// AsmPrinter to add their own handlers.
struct HandlerInfo {
std::unique_ptr<AsmPrinterHandler> Handler;
template <class H> struct HandlerInfo {
std::unique_ptr<H> Handler;
StringRef TimerName;
StringRef TimerDescription;
StringRef TimerGroupName;
StringRef TimerGroupDescription;

HandlerInfo(std::unique_ptr<AsmPrinterHandler> Handler, StringRef TimerName,
HandlerInfo(std::unique_ptr<H> Handler, StringRef TimerName,
StringRef TimerDescription, StringRef TimerGroupName,
StringRef TimerGroupDescription)
: Handler(std::move(Handler)), TimerName(TimerName),
Expand Down Expand Up @@ -205,9 +206,13 @@ class AsmPrinter : public MachineFunctionPass {

/// A vector of all debug/EH info emitters we should use. This vector
/// maintains ownership of the emitters.
std::vector<HandlerInfo> Handlers;
SmallVector<HandlerInfo<AsmPrinterHandler>, 2> Handlers;
size_t NumUserHandlers = 0;

/// Debuginfo handler. Protected so that targets can add their own.
SmallVector<HandlerInfo<DebugHandlerBase>, 1> DebugHandlers;
size_t NumUserDebugHandlers = 0;

StackMaps SM;

private:
Expand All @@ -222,7 +227,7 @@ class AsmPrinter : public MachineFunctionPass {

/// A handler that supports pseudo probe emission with embedded inline
/// context.
PseudoProbeHandler *PP = nullptr;
std::unique_ptr<PseudoProbeHandler> PP;

/// CFISection type the module needs i.e. either .eh_frame or .debug_frame.
CFISection ModuleCFISection = CFISection::None;
Expand Down Expand Up @@ -531,11 +536,16 @@ class AsmPrinter : public MachineFunctionPass {
// Overridable Hooks
//===------------------------------------------------------------------===//

void addAsmPrinterHandler(HandlerInfo Handler) {
void addAsmPrinterHandler(HandlerInfo<AsmPrinterHandler> Handler) {
Handlers.insert(Handlers.begin(), std::move(Handler));
NumUserHandlers++;
}

void addDebugHandler(HandlerInfo<DebugHandlerBase> Handler) {
DebugHandlers.insert(DebugHandlers.begin(), std::move(Handler));
NumUserDebugHandlers++;
}

// Targets can, or in the case of EmitInstruction, must implement these to
// customize output.

Expand Down
10 changes: 0 additions & 10 deletions llvm/include/llvm/CodeGen/AsmPrinterHandler.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,6 @@ class AsmPrinterHandler {
public:
virtual ~AsmPrinterHandler();

/// For symbols that have a size designated (e.g. common symbols),
/// 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.
Expand Down Expand Up @@ -72,12 +68,6 @@ class AsmPrinterHandler {
virtual void beginFunclet(const MachineBasicBlock &MBB,
MCSymbol *Sym = nullptr) {}
virtual void endFunclet() {}

/// Process beginning of an instruction.
virtual void beginInstruction(const MachineInstr *MI) = 0;

/// Process end of an instruction.
virtual void endInstruction() = 0;
};

} // End of namespace llvm
Expand Down
26 changes: 17 additions & 9 deletions llvm/include/llvm/CodeGen/DebugHandlerBase.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -116,18 +120,22 @@ class DebugHandlerBase : public AsmPrinterHandler {
private:
InstructionOrdering InstOrdering;

// AsmPrinterHandler overrides.
public:
void beginModule(Module *M) 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) {}

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);
Expand Down
94 changes: 60 additions & 34 deletions llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -166,10 +166,6 @@ const char CFGuardName[] = "Control Flow Guard";
const char CFGuardDescription[] = "Control Flow Guard";
const char CodeViewLineTablesGroupName[] = "linetables";
const char CodeViewLineTablesGroupDescription[] = "CodeView Line Tables";
const char PPTimerName[] = "emit";
const char PPTimerDescription[] = "Pseudo Probe Emission";
const char PPGroupName[] = "pseudo probe";
const char PPGroupDescription[] = "Pseudo Probe Emission";

STATISTIC(EmittedInsts, "Number of machine instrs printed");

Expand Down Expand Up @@ -553,27 +549,24 @@ bool AsmPrinter::doInitialization(Module &M) {
if (MAI->doesSupportDebugInformation()) {
bool EmitCodeView = M.getCodeViewFlag();
if (EmitCodeView && TM.getTargetTriple().isOSWindows()) {
Handlers.emplace_back(std::make_unique<CodeViewDebug>(this),
DbgTimerName, DbgTimerDescription,
CodeViewLineTablesGroupName,
CodeViewLineTablesGroupDescription);
DebugHandlers.emplace_back(std::make_unique<CodeViewDebug>(this),
DbgTimerName, DbgTimerDescription,
CodeViewLineTablesGroupName,
CodeViewLineTablesGroupDescription);
}
if (!EmitCodeView || M.getDwarfVersion()) {
assert(MMI && "MMI could not be nullptr here!");
if (MMI->hasDebugInfo()) {
DD = new DwarfDebug(this);
Handlers.emplace_back(std::unique_ptr<DwarfDebug>(DD), DbgTimerName,
DbgTimerDescription, DWARFGroupName,
DWARFGroupDescription);
DebugHandlers.emplace_back(std::unique_ptr<DwarfDebug>(DD),
DbgTimerName, DbgTimerDescription,
DWARFGroupName, DWARFGroupDescription);
}
}
}

if (M.getNamedMetadata(PseudoProbeDescMetadataName)) {
PP = new PseudoProbeHandler(this);
Handlers.emplace_back(std::unique_ptr<PseudoProbeHandler>(PP), PPTimerName,
PPTimerDescription, PPGroupName, PPGroupDescription);
}
if (M.getNamedMetadata(PseudoProbeDescMetadataName))
PP = std::make_unique<PseudoProbeHandler>(this);

switch (MAI->getExceptionHandlingType()) {
case ExceptionHandling::None:
Expand Down Expand Up @@ -640,7 +633,12 @@ bool AsmPrinter::doInitialization(Module &M) {
CFGuardDescription, DWARFGroupName,
DWARFGroupDescription);

for (const HandlerInfo &HI : Handlers) {
for (const auto &HI : DebugHandlers) {
NamedRegionTimer T(HI.TimerName, HI.TimerDescription, HI.TimerGroupName,
HI.TimerGroupDescription, TimePassesIsEnabled);
HI.Handler->beginModule(&M);
}
for (const auto &HI : Handlers) {
NamedRegionTimer T(HI.TimerName, HI.TimerDescription, HI.TimerGroupName,
HI.TimerGroupDescription, TimePassesIsEnabled);
HI.Handler->beginModule(&M);
Expand Down Expand Up @@ -791,10 +789,9 @@ void AsmPrinter::emitGlobalVariable(const GlobalVariable *GV) {
// sections and expected to be contiguous (e.g. ObjC metadata).
const Align Alignment = getGVAlignment(GV, DL);

for (const HandlerInfo &HI : Handlers) {
NamedRegionTimer T(HI.TimerName, HI.TimerDescription,
HI.TimerGroupName, HI.TimerGroupDescription,
TimePassesIsEnabled);
for (auto &HI : DebugHandlers) {
NamedRegionTimer T(HI.TimerName, HI.TimerDescription, HI.TimerGroupName,
HI.TimerGroupDescription, TimePassesIsEnabled);
HI.Handler->setSymbolSize(GVSym, Size);
}

Expand Down Expand Up @@ -1067,12 +1064,18 @@ void AsmPrinter::emitFunctionHeader() {
}

// Emit pre-function debug and/or EH information.
for (const HandlerInfo &HI : Handlers) {
for (const auto &HI : DebugHandlers) {
NamedRegionTimer T(HI.TimerName, HI.TimerDescription, HI.TimerGroupName,
HI.TimerGroupDescription, TimePassesIsEnabled);
HI.Handler->beginFunction(MF);
HI.Handler->beginBasicBlockSection(MF->front());
}
for (const HandlerInfo &HI : Handlers) {
for (const auto &HI : Handlers) {
NamedRegionTimer T(HI.TimerName, HI.TimerDescription, HI.TimerGroupName,
HI.TimerGroupDescription, TimePassesIsEnabled);
HI.Handler->beginFunction(MF);
}
for (const auto &HI : Handlers) {
NamedRegionTimer T(HI.TimerName, HI.TimerDescription, HI.TimerGroupName,
HI.TimerGroupDescription, TimePassesIsEnabled);
HI.Handler->beginBasicBlockSection(MF->front());
Expand Down Expand Up @@ -1770,7 +1773,7 @@ void AsmPrinter::emitFunctionBody() {
if (MDNode *MD = MI.getPCSections())
emitPCSectionsLabel(*MF, *MD);

for (const HandlerInfo &HI : Handlers) {
for (const auto &HI : DebugHandlers) {
NamedRegionTimer T(HI.TimerName, HI.TimerDescription, HI.TimerGroupName,
HI.TimerGroupDescription, TimePassesIsEnabled);
HI.Handler->beginInstruction(&MI);
Expand Down Expand Up @@ -1868,7 +1871,7 @@ void AsmPrinter::emitFunctionBody() {
if (MCSymbol *S = MI.getPostInstrSymbol())
OutStreamer->emitLabel(S);

for (const HandlerInfo &HI : Handlers) {
for (const auto &HI : DebugHandlers) {
NamedRegionTimer T(HI.TimerName, HI.TimerDescription, HI.TimerGroupName,
HI.TimerGroupDescription, TimePassesIsEnabled);
HI.Handler->endInstruction();
Expand Down Expand Up @@ -2003,13 +2006,18 @@ void AsmPrinter::emitFunctionBody() {
// Call endBasicBlockSection on the last block now, if it wasn't already
// called.
if (!MF->back().isEndSection()) {
for (const HandlerInfo &HI : Handlers) {
for (const auto &HI : DebugHandlers) {
NamedRegionTimer T(HI.TimerName, HI.TimerDescription, HI.TimerGroupName,
HI.TimerGroupDescription, TimePassesIsEnabled);
HI.Handler->endBasicBlockSection(MF->back());
}
for (const auto &HI : Handlers) {
NamedRegionTimer T(HI.TimerName, HI.TimerDescription, HI.TimerGroupName,
HI.TimerGroupDescription, TimePassesIsEnabled);
HI.Handler->endBasicBlockSection(MF->back());
}
}
for (const HandlerInfo &HI : Handlers) {
for (const auto &HI : Handlers) {
NamedRegionTimer T(HI.TimerName, HI.TimerDescription, HI.TimerGroupName,
HI.TimerGroupDescription, TimePassesIsEnabled);
HI.Handler->markFunctionEnd();
Expand All @@ -2022,7 +2030,12 @@ void AsmPrinter::emitFunctionBody() {
emitJumpTableInfo();

// Emit post-function debug and/or EH information.
for (const HandlerInfo &HI : Handlers) {
for (const auto &HI : DebugHandlers) {
NamedRegionTimer T(HI.TimerName, HI.TimerDescription, HI.TimerGroupName,
HI.TimerGroupDescription, TimePassesIsEnabled);
HI.Handler->endFunction(MF);
}
for (const auto &HI : Handlers) {
NamedRegionTimer T(HI.TimerName, HI.TimerDescription, HI.TimerGroupName,
HI.TimerGroupDescription, TimePassesIsEnabled);
HI.Handler->endFunction(MF);
Expand Down Expand Up @@ -2463,7 +2476,12 @@ bool AsmPrinter::doFinalization(Module &M) {
emitGlobalIFunc(M, IFunc);

// Finalize debug and EH information.
for (const HandlerInfo &HI : Handlers) {
for (const auto &HI : DebugHandlers) {
NamedRegionTimer T(HI.TimerName, HI.TimerDescription, HI.TimerGroupName,
HI.TimerGroupDescription, TimePassesIsEnabled);
HI.Handler->endModule();
}
for (const auto &HI : Handlers) {
NamedRegionTimer T(HI.TimerName, HI.TimerDescription, HI.TimerGroupName,
HI.TimerGroupDescription, TimePassesIsEnabled);
HI.Handler->endModule();
Expand All @@ -2473,6 +2491,8 @@ bool AsmPrinter::doFinalization(Module &M) {
// keeping all the user-added handlers alive until the AsmPrinter is
// destroyed.
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.
Expand Down Expand Up @@ -3987,7 +4007,7 @@ static void emitBasicBlockLoopComments(const MachineBasicBlock &MBB,
void AsmPrinter::emitBasicBlockStart(const MachineBasicBlock &MBB) {
// End the previous funclet and start a new one.
if (MBB.isEHFuncletEntry()) {
for (const HandlerInfo &HI : Handlers) {
for (const auto &HI : Handlers) {
HI.Handler->endFunclet();
HI.Handler->beginFunclet(MBB);
}
Expand Down Expand Up @@ -4059,17 +4079,23 @@ 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 call is handled separately, next to
// beginFunction).
if (MBB.isBeginSection() && !MBB.isEntryBlock())
for (const HandlerInfo &HI : Handlers)
if (MBB.isBeginSection() && !MBB.isEntryBlock()) {
for (const auto &HI : DebugHandlers)
HI.Handler->beginBasicBlockSection(MBB);
for (const auto &HI : Handlers)
HI.Handler->beginBasicBlockSection(MBB);
}
}

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 (const HandlerInfo &HI : Handlers)
if (MBB.isEndSection()) {
for (const auto &HI : DebugHandlers)
HI.Handler->endBasicBlockSection(MBB);
for (const auto &HI : Handlers)
HI.Handler->endBasicBlockSection(MBB);
}
}

void AsmPrinter::emitVisibility(MCSymbol *Sym, unsigned Visibility,
Expand Down
2 changes: 0 additions & 2 deletions llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h
Original file line number Diff line number Diff line change
Expand Up @@ -517,8 +517,6 @@ class LLVM_LIBRARY_VISIBILITY CodeViewDebug : public DebugHandlerBase {

void beginModule(Module *M) override;

void setSymbolSize(const MCSymbol *, uint64_t) override {}

/// Emit the COFF section that holds the line table information.
void endModule() override;

Expand Down
2 changes: 2 additions & 0 deletions llvm/lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,8 @@ DbgVariableLocation::extractFromMachineInstruction(

DebugHandlerBase::DebugHandlerBase(AsmPrinter *A) : Asm(A), MMI(Asm->MMI) {}

DebugHandlerBase::~DebugHandlerBase() = default;

void DebugHandlerBase::beginModule(Module *M) {
if (M->debug_compile_units().empty())
Asm = nullptr;
Expand Down
5 changes: 0 additions & 5 deletions llvm/lib/CodeGen/AsmPrinter/EHStreamer.h
Original file line number Diff line number Diff line change
Expand Up @@ -150,11 +150,6 @@ class LLVM_LIBRARY_VISIBILITY EHStreamer : public AsmPrinterHandler {
EHStreamer(AsmPrinter *A);
~EHStreamer() override;

// Unused.
void setSymbolSize(const MCSymbol *Sym, uint64_t Size) override {}
void beginInstruction(const MachineInstr *MI) override {}
void endInstruction() override {}

/// Return `true' if this is a call to a function marked `nounwind'. Return
/// `false' otherwise.
static bool callToNoUnwindFunction(const MachineInstr *MI);
Expand Down
2 changes: 0 additions & 2 deletions llvm/lib/CodeGen/AsmPrinter/PseudoProbePrinter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@

using namespace llvm;

PseudoProbeHandler::~PseudoProbeHandler() = default;

void PseudoProbeHandler::emitPseudoProbe(uint64_t Guid, uint64_t Index,
uint64_t Type, uint64_t Attr,
const DILocation *DebugLoc) {
Expand Down
Loading
Loading