Skip to content

Commit 9b9a40a

Browse files
committed
[CodeGen] Reduce AsmPrinterHandlers virt. fn calls
Currently, an AsmPrinterHandler has several methods that allow to dynamically hook in unwind or debug info emission, e.g. at begin/end of every function or instruction. The class hierarchy and the actually overriden functions are as follows: (SymSz=setSymbolSize, mFE=markFunctionEnd, BBS=BasicBlockSection, FL=Funclet; b=beginX, e=endX) SymSz Mod Fn mFE BBS FL Inst AsmPrinterHandler - - - - - - - ` PseudoProbeHandler - - - - - - - ` WinCFGuard - e e - - - - ` EHStreamer - - - - - - - ` DwarfCFIException - e be - be - - ` ARMException - - be e - - - ` AIXException - - e - - - - ` WinException - e be e - be - ` WasmException - e be - - - - ` DebugHandlerBase - b be - be - be ` BTFDebug - e - - - - b ` CodeViewDebug - be - - - - b ` DWARFDebug yes be - - - - b Doing virtual function calls per instruction is costly and useless when the called function does nothing. This commit performs the following clean-up/improvements: - PseudoProbeHandler is no longer an AsmPrinterHandler -- it used nothing of its functionality to hook in at the possible points. This avoids virtual function calls when a pseudo probe printer is present. - DebugHandlerBase is no longer an AsmPrinterHandler, but a separate base class. DebugHandlerBase is the only remaining "hook" for begin/end instruction and setSymbolSize (only used by DWARFDebug). begin/end for function and basic block sections are never overriden and therefore are no longer virtual. (Originally I intended there to be only one debug handler, but BPF as the only target supports two at the same time: DWARF and BTF.) - AsmPrinterHandler no longer has begin/end instruction and setSymbolSize hooks -- these were only used by DebugHandlerBase. This avoid iterating over handlers in every instruction. - Remove NamedRegionTimer from instruction loop. Checking a global variable for every instruction (and doing an out-of-line function call) is too expensive for a profiling functionality. AsmPrinterHandler Mod Fn mFE BBS FL ` WinCFGuard e e - - - ` EHStreamer - - - - - ` DwarfCFIException e be - be - ` ARMException - be e - - ` AIXException - e - - - ` WinException e be e - be ` WasmException e be - - - SymSz Mod Fn BBS Inst DebugHandlerBase - b be be be ` BTFDebug - e b ` CodeViewDebug - be b ` DWARFDebug yes be b PseudoProbeHandler (no shared methods) This results in a performance improvement, especially in the -O0 -g0 case with unwind information (e.g., JIT baseline).
1 parent 54cb5ca commit 9b9a40a

File tree

13 files changed

+98
-94
lines changed

13 files changed

+98
-94
lines changed

llvm/include/llvm/CodeGen/AsmPrinter.h

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include "llvm/ADT/SmallVector.h"
2121
#include "llvm/BinaryFormat/Dwarf.h"
2222
#include "llvm/CodeGen/AsmPrinterHandler.h"
23+
#include "llvm/CodeGen/DebugHandlerBase.h"
2324
#include "llvm/CodeGen/DwarfStringPoolEntry.h"
2425
#include "llvm/CodeGen/MachineFunctionPass.h"
2526
#include "llvm/CodeGen/StackMaps.h"
@@ -145,14 +146,14 @@ class AsmPrinter : public MachineFunctionPass {
145146

146147
/// struct HandlerInfo and Handlers permit users or target extended
147148
/// AsmPrinter to add their own handlers.
148-
struct HandlerInfo {
149-
std::unique_ptr<AsmPrinterHandler> Handler;
149+
template <class H> struct HandlerInfo {
150+
std::unique_ptr<H> Handler;
150151
StringRef TimerName;
151152
StringRef TimerDescription;
152153
StringRef TimerGroupName;
153154
StringRef TimerGroupDescription;
154155

155-
HandlerInfo(std::unique_ptr<AsmPrinterHandler> Handler, StringRef TimerName,
156+
HandlerInfo(std::unique_ptr<H> Handler, StringRef TimerName,
156157
StringRef TimerDescription, StringRef TimerGroupName,
157158
StringRef TimerGroupDescription)
158159
: Handler(std::move(Handler)), TimerName(TimerName),
@@ -205,9 +206,13 @@ class AsmPrinter : public MachineFunctionPass {
205206

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

212+
/// Debuginfo handler. Protected so that targets can add their own.
213+
SmallVector<HandlerInfo<DebugHandlerBase>, 1> DebugHandlers;
214+
size_t NumUserDebugHandlers = 0;
215+
211216
StackMaps SM;
212217

213218
private:
@@ -222,7 +227,7 @@ class AsmPrinter : public MachineFunctionPass {
222227

223228
/// A handler that supports pseudo probe emission with embedded inline
224229
/// context.
225-
PseudoProbeHandler *PP = nullptr;
230+
std::unique_ptr<PseudoProbeHandler> PP;
226231

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

534-
void addAsmPrinterHandler(HandlerInfo Handler) {
539+
void addAsmPrinterHandler(HandlerInfo<AsmPrinterHandler> Handler) {
535540
Handlers.insert(Handlers.begin(), std::move(Handler));
536541
NumUserHandlers++;
537542
}
538543

544+
void addDebugHandler(HandlerInfo<DebugHandlerBase> Handler) {
545+
DebugHandlers.insert(DebugHandlers.begin(), std::move(Handler));
546+
NumUserDebugHandlers++;
547+
}
548+
539549
// Targets can, or in the case of EmitInstruction, must implement these to
540550
// customize output.
541551

llvm/include/llvm/CodeGen/AsmPrinterHandler.h

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,6 @@ class AsmPrinterHandler {
3434
public:
3535
virtual ~AsmPrinterHandler();
3636

37-
/// For symbols that have a size designated (e.g. common symbols),
38-
/// this tracks that size.
39-
virtual void setSymbolSize(const MCSymbol *Sym, uint64_t Size) = 0;
40-
4137
virtual void beginModule(Module *M) {}
4238

4339
/// Emit all sections that should come after the content.
@@ -72,12 +68,6 @@ class AsmPrinterHandler {
7268
virtual void beginFunclet(const MachineBasicBlock &MBB,
7369
MCSymbol *Sym = nullptr) {}
7470
virtual void endFunclet() {}
75-
76-
/// Process beginning of an instruction.
77-
virtual void beginInstruction(const MachineInstr *MI) = 0;
78-
79-
/// Process end of an instruction.
80-
virtual void endInstruction() = 0;
8171
};
8272

8373
} // End of namespace llvm

llvm/include/llvm/CodeGen/DebugHandlerBase.h

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,14 @@ struct DbgVariableLocation {
5050

5151
/// Base class for debug information backends. Common functionality related to
5252
/// tracking which variables and scopes are alive at a given PC live here.
53-
class DebugHandlerBase : public AsmPrinterHandler {
53+
class DebugHandlerBase {
5454
protected:
5555
DebugHandlerBase(AsmPrinter *A);
5656

57+
public:
58+
virtual ~DebugHandlerBase();
59+
60+
protected:
5761
/// Target of debug info emission.
5862
AsmPrinter *Asm = nullptr;
5963

@@ -116,18 +120,22 @@ class DebugHandlerBase : public AsmPrinterHandler {
116120
private:
117121
InstructionOrdering InstOrdering;
118122

119-
// AsmPrinterHandler overrides.
120123
public:
121-
void beginModule(Module *M) override;
124+
/// For symbols that have a size designated (e.g. common symbols),
125+
/// this tracks that size. Only used by DWARF.
126+
virtual void setSymbolSize(const MCSymbol *Sym, uint64_t Size) {}
127+
128+
virtual void beginModule(Module *M);
129+
virtual void endModule() = 0;
122130

123-
void beginInstruction(const MachineInstr *MI) override;
124-
void endInstruction() override;
131+
virtual void beginInstruction(const MachineInstr *MI);
132+
virtual void endInstruction();
125133

126-
void beginFunction(const MachineFunction *MF) override;
127-
void endFunction(const MachineFunction *MF) override;
134+
void beginFunction(const MachineFunction *MF);
135+
void endFunction(const MachineFunction *MF);
128136

129-
void beginBasicBlockSection(const MachineBasicBlock &MBB) override;
130-
void endBasicBlockSection(const MachineBasicBlock &MBB) override;
137+
void beginBasicBlockSection(const MachineBasicBlock &MBB);
138+
void endBasicBlockSection(const MachineBasicBlock &MBB);
131139

132140
/// Return Label preceding the instruction.
133141
MCSymbol *getLabelBeforeInsn(const MachineInstr *MI);

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

Lines changed: 60 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -166,10 +166,6 @@ const char CFGuardName[] = "Control Flow Guard";
166166
const char CFGuardDescription[] = "Control Flow Guard";
167167
const char CodeViewLineTablesGroupName[] = "linetables";
168168
const char CodeViewLineTablesGroupDescription[] = "CodeView Line Tables";
169-
const char PPTimerName[] = "emit";
170-
const char PPTimerDescription[] = "Pseudo Probe Emission";
171-
const char PPGroupName[] = "pseudo probe";
172-
const char PPGroupDescription[] = "Pseudo Probe Emission";
173169

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

@@ -553,27 +549,24 @@ bool AsmPrinter::doInitialization(Module &M) {
553549
if (MAI->doesSupportDebugInformation()) {
554550
bool EmitCodeView = M.getCodeViewFlag();
555551
if (EmitCodeView && TM.getTargetTriple().isOSWindows()) {
556-
Handlers.emplace_back(std::make_unique<CodeViewDebug>(this),
557-
DbgTimerName, DbgTimerDescription,
558-
CodeViewLineTablesGroupName,
559-
CodeViewLineTablesGroupDescription);
552+
DebugHandlers.emplace_back(std::make_unique<CodeViewDebug>(this),
553+
DbgTimerName, DbgTimerDescription,
554+
CodeViewLineTablesGroupName,
555+
CodeViewLineTablesGroupDescription);
560556
}
561557
if (!EmitCodeView || M.getDwarfVersion()) {
562558
assert(MMI && "MMI could not be nullptr here!");
563559
if (MMI->hasDebugInfo()) {
564560
DD = new DwarfDebug(this);
565-
Handlers.emplace_back(std::unique_ptr<DwarfDebug>(DD), DbgTimerName,
566-
DbgTimerDescription, DWARFGroupName,
567-
DWARFGroupDescription);
561+
DebugHandlers.emplace_back(std::unique_ptr<DwarfDebug>(DD),
562+
DbgTimerName, DbgTimerDescription,
563+
DWARFGroupName, DWARFGroupDescription);
568564
}
569565
}
570566
}
571567

572-
if (M.getNamedMetadata(PseudoProbeDescMetadataName)) {
573-
PP = new PseudoProbeHandler(this);
574-
Handlers.emplace_back(std::unique_ptr<PseudoProbeHandler>(PP), PPTimerName,
575-
PPTimerDescription, PPGroupName, PPGroupDescription);
576-
}
568+
if (M.getNamedMetadata(PseudoProbeDescMetadataName))
569+
PP = std::make_unique<PseudoProbeHandler>(this);
577570

578571
switch (MAI->getExceptionHandlingType()) {
579572
case ExceptionHandling::None:
@@ -640,7 +633,12 @@ bool AsmPrinter::doInitialization(Module &M) {
640633
CFGuardDescription, DWARFGroupName,
641634
DWARFGroupDescription);
642635

643-
for (const HandlerInfo &HI : Handlers) {
636+
for (const auto &HI : DebugHandlers) {
637+
NamedRegionTimer T(HI.TimerName, HI.TimerDescription, HI.TimerGroupName,
638+
HI.TimerGroupDescription, TimePassesIsEnabled);
639+
HI.Handler->beginModule(&M);
640+
}
641+
for (const auto &HI : Handlers) {
644642
NamedRegionTimer T(HI.TimerName, HI.TimerDescription, HI.TimerGroupName,
645643
HI.TimerGroupDescription, TimePassesIsEnabled);
646644
HI.Handler->beginModule(&M);
@@ -791,10 +789,9 @@ void AsmPrinter::emitGlobalVariable(const GlobalVariable *GV) {
791789
// sections and expected to be contiguous (e.g. ObjC metadata).
792790
const Align Alignment = getGVAlignment(GV, DL);
793791

794-
for (const HandlerInfo &HI : Handlers) {
795-
NamedRegionTimer T(HI.TimerName, HI.TimerDescription,
796-
HI.TimerGroupName, HI.TimerGroupDescription,
797-
TimePassesIsEnabled);
792+
for (auto &HI : DebugHandlers) {
793+
NamedRegionTimer T(HI.TimerName, HI.TimerDescription, HI.TimerGroupName,
794+
HI.TimerGroupDescription, TimePassesIsEnabled);
798795
HI.Handler->setSymbolSize(GVSym, Size);
799796
}
800797

@@ -1067,12 +1064,18 @@ void AsmPrinter::emitFunctionHeader() {
10671064
}
10681065

10691066
// Emit pre-function debug and/or EH information.
1070-
for (const HandlerInfo &HI : Handlers) {
1067+
for (const auto &HI : DebugHandlers) {
10711068
NamedRegionTimer T(HI.TimerName, HI.TimerDescription, HI.TimerGroupName,
10721069
HI.TimerGroupDescription, TimePassesIsEnabled);
10731070
HI.Handler->beginFunction(MF);
1071+
HI.Handler->beginBasicBlockSection(MF->front());
10741072
}
1075-
for (const HandlerInfo &HI : Handlers) {
1073+
for (const auto &HI : Handlers) {
1074+
NamedRegionTimer T(HI.TimerName, HI.TimerDescription, HI.TimerGroupName,
1075+
HI.TimerGroupDescription, TimePassesIsEnabled);
1076+
HI.Handler->beginFunction(MF);
1077+
}
1078+
for (const auto &HI : Handlers) {
10761079
NamedRegionTimer T(HI.TimerName, HI.TimerDescription, HI.TimerGroupName,
10771080
HI.TimerGroupDescription, TimePassesIsEnabled);
10781081
HI.Handler->beginBasicBlockSection(MF->front());
@@ -1770,7 +1773,7 @@ void AsmPrinter::emitFunctionBody() {
17701773
if (MDNode *MD = MI.getPCSections())
17711774
emitPCSectionsLabel(*MF, *MD);
17721775

1773-
for (const HandlerInfo &HI : Handlers) {
1776+
for (const auto &HI : DebugHandlers) {
17741777
NamedRegionTimer T(HI.TimerName, HI.TimerDescription, HI.TimerGroupName,
17751778
HI.TimerGroupDescription, TimePassesIsEnabled);
17761779
HI.Handler->beginInstruction(&MI);
@@ -1868,7 +1871,7 @@ void AsmPrinter::emitFunctionBody() {
18681871
if (MCSymbol *S = MI.getPostInstrSymbol())
18691872
OutStreamer->emitLabel(S);
18701873

1871-
for (const HandlerInfo &HI : Handlers) {
1874+
for (const auto &HI : DebugHandlers) {
18721875
NamedRegionTimer T(HI.TimerName, HI.TimerDescription, HI.TimerGroupName,
18731876
HI.TimerGroupDescription, TimePassesIsEnabled);
18741877
HI.Handler->endInstruction();
@@ -2003,13 +2006,18 @@ void AsmPrinter::emitFunctionBody() {
20032006
// Call endBasicBlockSection on the last block now, if it wasn't already
20042007
// called.
20052008
if (!MF->back().isEndSection()) {
2006-
for (const HandlerInfo &HI : Handlers) {
2009+
for (const auto &HI : DebugHandlers) {
2010+
NamedRegionTimer T(HI.TimerName, HI.TimerDescription, HI.TimerGroupName,
2011+
HI.TimerGroupDescription, TimePassesIsEnabled);
2012+
HI.Handler->endBasicBlockSection(MF->back());
2013+
}
2014+
for (const auto &HI : Handlers) {
20072015
NamedRegionTimer T(HI.TimerName, HI.TimerDescription, HI.TimerGroupName,
20082016
HI.TimerGroupDescription, TimePassesIsEnabled);
20092017
HI.Handler->endBasicBlockSection(MF->back());
20102018
}
20112019
}
2012-
for (const HandlerInfo &HI : Handlers) {
2020+
for (const auto &HI : Handlers) {
20132021
NamedRegionTimer T(HI.TimerName, HI.TimerDescription, HI.TimerGroupName,
20142022
HI.TimerGroupDescription, TimePassesIsEnabled);
20152023
HI.Handler->markFunctionEnd();
@@ -2022,7 +2030,12 @@ void AsmPrinter::emitFunctionBody() {
20222030
emitJumpTableInfo();
20232031

20242032
// Emit post-function debug and/or EH information.
2025-
for (const HandlerInfo &HI : Handlers) {
2033+
for (const auto &HI : DebugHandlers) {
2034+
NamedRegionTimer T(HI.TimerName, HI.TimerDescription, HI.TimerGroupName,
2035+
HI.TimerGroupDescription, TimePassesIsEnabled);
2036+
HI.Handler->endFunction(MF);
2037+
}
2038+
for (const auto &HI : Handlers) {
20262039
NamedRegionTimer T(HI.TimerName, HI.TimerDescription, HI.TimerGroupName,
20272040
HI.TimerGroupDescription, TimePassesIsEnabled);
20282041
HI.Handler->endFunction(MF);
@@ -2463,7 +2476,12 @@ bool AsmPrinter::doFinalization(Module &M) {
24632476
emitGlobalIFunc(M, IFunc);
24642477

24652478
// Finalize debug and EH information.
2466-
for (const HandlerInfo &HI : Handlers) {
2479+
for (const auto &HI : DebugHandlers) {
2480+
NamedRegionTimer T(HI.TimerName, HI.TimerDescription, HI.TimerGroupName,
2481+
HI.TimerGroupDescription, TimePassesIsEnabled);
2482+
HI.Handler->endModule();
2483+
}
2484+
for (const auto &HI : Handlers) {
24672485
NamedRegionTimer T(HI.TimerName, HI.TimerDescription, HI.TimerGroupName,
24682486
HI.TimerGroupDescription, TimePassesIsEnabled);
24692487
HI.Handler->endModule();
@@ -2473,6 +2491,8 @@ bool AsmPrinter::doFinalization(Module &M) {
24732491
// keeping all the user-added handlers alive until the AsmPrinter is
24742492
// destroyed.
24752493
Handlers.erase(Handlers.begin() + NumUserHandlers, Handlers.end());
2494+
DebugHandlers.erase(DebugHandlers.begin() + NumUserDebugHandlers,
2495+
DebugHandlers.end());
24762496
DD = nullptr;
24772497

24782498
// If the target wants to know about weak references, print them all.
@@ -3987,7 +4007,7 @@ static void emitBasicBlockLoopComments(const MachineBasicBlock &MBB,
39874007
void AsmPrinter::emitBasicBlockStart(const MachineBasicBlock &MBB) {
39884008
// End the previous funclet and start a new one.
39894009
if (MBB.isEHFuncletEntry()) {
3990-
for (const HandlerInfo &HI : Handlers) {
4010+
for (const auto &HI : Handlers) {
39914011
HI.Handler->endFunclet();
39924012
HI.Handler->beginFunclet(MBB);
39934013
}
@@ -4059,17 +4079,23 @@ void AsmPrinter::emitBasicBlockStart(const MachineBasicBlock &MBB) {
40594079
// With BB sections, each basic block must handle CFI information on its own
40604080
// if it begins a section (Entry block call is handled separately, next to
40614081
// beginFunction).
4062-
if (MBB.isBeginSection() && !MBB.isEntryBlock())
4063-
for (const HandlerInfo &HI : Handlers)
4082+
if (MBB.isBeginSection() && !MBB.isEntryBlock()) {
4083+
for (const auto &HI : DebugHandlers)
4084+
HI.Handler->beginBasicBlockSection(MBB);
4085+
for (const auto &HI : Handlers)
40644086
HI.Handler->beginBasicBlockSection(MBB);
4087+
}
40654088
}
40664089

40674090
void AsmPrinter::emitBasicBlockEnd(const MachineBasicBlock &MBB) {
40684091
// Check if CFI information needs to be updated for this MBB with basic block
40694092
// sections.
4070-
if (MBB.isEndSection())
4071-
for (const HandlerInfo &HI : Handlers)
4093+
if (MBB.isEndSection()) {
4094+
for (const auto &HI : DebugHandlers)
4095+
HI.Handler->endBasicBlockSection(MBB);
4096+
for (const auto &HI : Handlers)
40724097
HI.Handler->endBasicBlockSection(MBB);
4098+
}
40734099
}
40744100

40754101
void AsmPrinter::emitVisibility(MCSymbol *Sym, unsigned Visibility,

llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -517,8 +517,6 @@ class LLVM_LIBRARY_VISIBILITY CodeViewDebug : public DebugHandlerBase {
517517

518518
void beginModule(Module *M) override;
519519

520-
void setSymbolSize(const MCSymbol *, uint64_t) override {}
521-
522520
/// Emit the COFF section that holds the line table information.
523521
void endModule() override;
524522

llvm/lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,8 @@ DbgVariableLocation::extractFromMachineInstruction(
9999

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

102+
DebugHandlerBase::~DebugHandlerBase() = default;
103+
102104
void DebugHandlerBase::beginModule(Module *M) {
103105
if (M->debug_compile_units().empty())
104106
Asm = nullptr;

llvm/lib/CodeGen/AsmPrinter/EHStreamer.h

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -150,11 +150,6 @@ class LLVM_LIBRARY_VISIBILITY EHStreamer : public AsmPrinterHandler {
150150
EHStreamer(AsmPrinter *A);
151151
~EHStreamer() override;
152152

153-
// Unused.
154-
void setSymbolSize(const MCSymbol *Sym, uint64_t Size) override {}
155-
void beginInstruction(const MachineInstr *MI) override {}
156-
void endInstruction() override {}
157-
158153
/// Return `true' if this is a call to a function marked `nounwind'. Return
159154
/// `false' otherwise.
160155
static bool callToNoUnwindFunction(const MachineInstr *MI);

llvm/lib/CodeGen/AsmPrinter/PseudoProbePrinter.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,6 @@
2020

2121
using namespace llvm;
2222

23-
PseudoProbeHandler::~PseudoProbeHandler() = default;
24-
2523
void PseudoProbeHandler::emitPseudoProbe(uint64_t Guid, uint64_t Index,
2624
uint64_t Type, uint64_t Attr,
2725
const DILocation *DebugLoc) {

0 commit comments

Comments
 (0)