[AsmPrinter] Remove timers #97046
Merged
[AsmPrinter] Remove timers #97046
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-debuginfo Author: Alexis Engelke (aengelke) ChangesTimers are an out-of-line function call and a global variable access, here twice per emitted instruction. At this granularity, not only the time results become skewed, but the timers also add a performance overhead when profiling is disabled. Also outside of the innermost loop, timers add a measurable overhead. As this is quite expensive for a mostly unused profiling facility, remove the timers. Follow-up of #96785 (this PR currently includes the commit from that -- I don't know how to do stacked diffs on Github -- will rebase once that is merged). There are two actual commits: the first (01abb8b) removes timers just from the innermost loop (introduced in f8dba24 without a reason given in the commit message), the second (039a158) removes them altogether. I propose to remove the timers entirely, given that profilers nowadays can show pretty well how much time is spent in which handler (these are never inlined).
Patch is 24.78 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/97046.diff 13 Files Affected:
diff --git a/llvm/include/llvm/CodeGen/AsmPrinter.h b/llvm/include/llvm/CodeGen/AsmPrinter.h
index 011f8c6534b6a..d876a142cf338 100644
--- a/llvm/include/llvm/CodeGen/AsmPrinter.h
+++ b/llvm/include/llvm/CodeGen/AsmPrinter.h
@@ -20,6 +20,7 @@
#include "llvm/ADT/SmallVector.h"
#include "llvm/BinaryFormat/Dwarf.h"
#include "llvm/CodeGen/AsmPrinterHandler.h"
+#include "llvm/CodeGen/DebugHandlerBase.h"
#include "llvm/CodeGen/DwarfStringPoolEntry.h"
#include "llvm/CodeGen/MachineFunctionPass.h"
#include "llvm/CodeGen/StackMaps.h"
@@ -143,23 +144,6 @@ class AsmPrinter : public MachineFunctionPass {
using GOTEquivUsePair = std::pair<const GlobalVariable *, unsigned>;
MapVector<const MCSymbol *, GOTEquivUsePair> GlobalGOTEquivs;
- /// struct HandlerInfo and Handlers permit users or target extended
- /// AsmPrinter to add their own handlers.
- struct HandlerInfo {
- std::unique_ptr<AsmPrinterHandler> Handler;
- StringRef TimerName;
- StringRef TimerDescription;
- StringRef TimerGroupName;
- StringRef TimerGroupDescription;
-
- HandlerInfo(std::unique_ptr<AsmPrinterHandler> Handler, StringRef TimerName,
- StringRef TimerDescription, StringRef TimerGroupName,
- StringRef TimerGroupDescription)
- : Handler(std::move(Handler)), TimerName(TimerName),
- TimerDescription(TimerDescription), TimerGroupName(TimerGroupName),
- TimerGroupDescription(TimerGroupDescription) {}
- };
-
// Flags representing which CFI section is required for a function/module.
enum class CFISection : unsigned {
None = 0, ///< Do not emit either .eh_frame or .debug_frame
@@ -205,9 +189,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<std::unique_ptr<AsmPrinterHandler>, 2> Handlers;
size_t NumUserHandlers = 0;
+ /// Debuginfo handler. Protected so that targets can add their own.
+ SmallVector<std::unique_ptr<DebugHandlerBase>, 1> DebugHandlers;
+ size_t NumUserDebugHandlers = 0;
+
StackMaps SM;
private:
@@ -222,7 +210,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;
@@ -531,11 +519,16 @@ class AsmPrinter : public MachineFunctionPass {
// Overridable Hooks
//===------------------------------------------------------------------===//
- void addAsmPrinterHandler(HandlerInfo Handler) {
+ void addAsmPrinterHandler(std::unique_ptr<AsmPrinterHandler> Handler) {
Handlers.insert(Handlers.begin(), std::move(Handler));
NumUserHandlers++;
}
+ void addDebugHandler(std::unique_ptr<DebugHandlerBase> Handler) {
+ DebugHandlers.insert(DebugHandlers.begin(), std::move(Handler));
+ NumUserDebugHandlers++;
+ }
+
// 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 5c06645f767eb..ed73e618431de 100644
--- a/llvm/include/llvm/CodeGen/AsmPrinterHandler.h
+++ b/llvm/include/llvm/CodeGen/AsmPrinterHandler.h
@@ -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.
@@ -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
diff --git a/llvm/include/llvm/CodeGen/DebugHandlerBase.h b/llvm/include/llvm/CodeGen/DebugHandlerBase.h
index af25f2544da71..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,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);
diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
index 40f4dc2689cdf..8669236ea4a1e 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -113,7 +113,6 @@
#include "llvm/Support/Format.h"
#include "llvm/Support/MathExtras.h"
#include "llvm/Support/Path.h"
-#include "llvm/Support/Timer.h"
#include "llvm/Support/VCSRevision.h"
#include "llvm/Support/raw_ostream.h"
#include "llvm/Target/TargetLoweringObjectFile.h"
@@ -156,21 +155,6 @@ static cl::bits<PGOMapFeaturesEnum> PgoAnalysisMapFeatures(
"Enable extended information within the SHT_LLVM_BB_ADDR_MAP that is "
"extracted from PGO related analysis."));
-const char DWARFGroupName[] = "dwarf";
-const char DWARFGroupDescription[] = "DWARF Emission";
-const char DbgTimerName[] = "emit";
-const char DbgTimerDescription[] = "Debug Info Emission";
-const char EHTimerName[] = "write_exception";
-const char EHTimerDescription[] = "DWARF Exception Writer";
-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");
char AsmPrinter::ID = 0;
@@ -552,28 +536,19 @@ 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);
- }
+ if (EmitCodeView && TM.getTargetTriple().isOSWindows())
+ DebugHandlers.push_back(std::make_unique<CodeViewDebug>(this));
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.push_back(std::unique_ptr<DwarfDebug>(DD));
}
}
}
- 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:
@@ -630,21 +605,16 @@ bool AsmPrinter::doInitialization(Module &M) {
break;
}
if (ES)
- Handlers.emplace_back(std::unique_ptr<EHStreamer>(ES), EHTimerName,
- EHTimerDescription, DWARFGroupName,
- DWARFGroupDescription);
+ Handlers.push_back(std::unique_ptr<EHStreamer>(ES));
// Emit tables for any value of cfguard flag (i.e. cfguard=1 or cfguard=2).
if (mdconst::extract_or_null<ConstantInt>(M.getModuleFlag("cfguard")))
- Handlers.emplace_back(std::make_unique<WinCFGuard>(this), CFGuardName,
- CFGuardDescription, DWARFGroupName,
- DWARFGroupDescription);
-
- for (const HandlerInfo &HI : Handlers) {
- NamedRegionTimer T(HI.TimerName, HI.TimerDescription, HI.TimerGroupName,
- HI.TimerGroupDescription, TimePassesIsEnabled);
- HI.Handler->beginModule(&M);
- }
+ Handlers.push_back(std::make_unique<WinCFGuard>(this));
+
+ for (auto &Handler : DebugHandlers)
+ Handler->beginModule(&M);
+ for (auto &Handler : Handlers)
+ Handler->beginModule(&M);
return false;
}
@@ -791,12 +761,8 @@ 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);
- HI.Handler->setSymbolSize(GVSym, Size);
- }
+ for (auto &Handler : DebugHandlers)
+ Handler->setSymbolSize(GVSym, Size);
// Handle common symbols
if (GVKind.isCommon()) {
@@ -1067,16 +1033,14 @@ void AsmPrinter::emitFunctionHeader() {
}
// Emit pre-function debug and/or EH information.
- for (const HandlerInfo &HI : Handlers) {
- NamedRegionTimer T(HI.TimerName, HI.TimerDescription, HI.TimerGroupName,
- HI.TimerGroupDescription, TimePassesIsEnabled);
- HI.Handler->beginFunction(MF);
- }
- for (const HandlerInfo &HI : Handlers) {
- NamedRegionTimer T(HI.TimerName, HI.TimerDescription, HI.TimerGroupName,
- HI.TimerGroupDescription, TimePassesIsEnabled);
- HI.Handler->beginBasicBlockSection(MF->front());
+ for (auto &Handler : DebugHandlers) {
+ Handler->beginFunction(MF);
+ Handler->beginBasicBlockSection(MF->front());
}
+ for (auto &Handler : Handlers)
+ Handler->beginFunction(MF);
+ for (auto &Handler : Handlers)
+ Handler->beginBasicBlockSection(MF->front());
// Emit the prologue data.
if (F.hasPrologueData())
@@ -1770,11 +1734,8 @@ void AsmPrinter::emitFunctionBody() {
if (MDNode *MD = MI.getPCSections())
emitPCSectionsLabel(*MF, *MD);
- for (const HandlerInfo &HI : Handlers) {
- NamedRegionTimer T(HI.TimerName, HI.TimerDescription, HI.TimerGroupName,
- HI.TimerGroupDescription, TimePassesIsEnabled);
- HI.Handler->beginInstruction(&MI);
- }
+ for (auto &Handler : DebugHandlers)
+ Handler->beginInstruction(&MI);
if (isVerbose())
emitComments(MI, OutStreamer->getCommentOS());
@@ -1868,11 +1829,8 @@ void AsmPrinter::emitFunctionBody() {
if (MCSymbol *S = MI.getPostInstrSymbol())
OutStreamer->emitLabel(S);
- for (const HandlerInfo &HI : Handlers) {
- NamedRegionTimer T(HI.TimerName, HI.TimerDescription, HI.TimerGroupName,
- HI.TimerGroupDescription, TimePassesIsEnabled);
- HI.Handler->endInstruction();
- }
+ for (auto &Handler : DebugHandlers)
+ Handler->endInstruction();
}
// We must emit temporary symbol for the end of this basic block, if either
@@ -2003,17 +1961,13 @@ 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) {
- NamedRegionTimer T(HI.TimerName, HI.TimerDescription, HI.TimerGroupName,
- HI.TimerGroupDescription, TimePassesIsEnabled);
- HI.Handler->endBasicBlockSection(MF->back());
- }
- }
- for (const HandlerInfo &HI : Handlers) {
- NamedRegionTimer T(HI.TimerName, HI.TimerDescription, HI.TimerGroupName,
- HI.TimerGroupDescription, TimePassesIsEnabled);
- HI.Handler->markFunctionEnd();
+ for (auto &Handler : DebugHandlers)
+ Handler->endBasicBlockSection(MF->back());
+ for (auto &Handler : Handlers)
+ Handler->endBasicBlockSection(MF->back());
}
+ for (auto &Handler : Handlers)
+ Handler->markFunctionEnd();
MBBSectionRanges[MF->front().getSectionIDNum()] =
MBBSectionRange{CurrentFnBegin, CurrentFnEnd};
@@ -2022,11 +1976,10 @@ void AsmPrinter::emitFunctionBody() {
emitJumpTableInfo();
// Emit post-function debug and/or EH information.
- for (const HandlerInfo &HI : Handlers) {
- NamedRegionTimer T(HI.TimerName, HI.TimerDescription, HI.TimerGroupName,
- HI.TimerGroupDescription, TimePassesIsEnabled);
- HI.Handler->endFunction(MF);
- }
+ for (auto &Handler : DebugHandlers)
+ Handler->endFunction(MF);
+ for (auto &Handler : Handlers)
+ Handler->endFunction(MF);
// Emit section containing BB address offsets and their metadata, when
// BB labels are requested for this function. Skip empty functions.
@@ -2463,16 +2416,17 @@ bool AsmPrinter::doFinalization(Module &M) {
emitGlobalIFunc(M, IFunc);
// Finalize debug and EH information.
- for (const HandlerInfo &HI : Handlers) {
- NamedRegionTimer T(HI.TimerName, HI.TimerDescription, HI.TimerGroupName,
- HI.TimerGroupDescription, TimePassesIsEnabled);
- HI.Handler->endModule();
- }
+ for (auto &Handler : DebugHandlers)
+ Handler->endModule();
+ 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.
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.
@@ -3987,9 +3941,9 @@ 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) {
- HI.Handler->endFunclet();
- HI.Handler->beginFunclet(MBB);
+ for (auto &Handler : Handlers) {
+ Handler->endFunclet();
+ Handler->beginFunclet(MBB);
}
}
@@ -4059,17 +4013,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)
- HI.Handler->beginBasicBlockSection(MBB);
+ if (MBB.isBeginSection() && !MBB.isEntryBlock()) {
+ for (auto &Handler : DebugHandlers)
+ Handler->beginBasicBlockSection(MBB);
+ for (auto &Handler : Handlers)
+ 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)
- HI.Handler->endBasicBlockSection(MBB);
+ if (MBB.isEndSection()) {
+ for (auto &Handler : DebugHandlers)
+ Handler->endBasicBlockSection(MBB);
+ for (auto &Handler : Handlers)
+ Handler->endBasicBlockSection(MBB);
+ }
}
void AsmPrinter::emitVisibility(MCSymbol *Sym, unsigned Visibility,
diff --git a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h
index 55d149e049c94..7a138a0332b6d 100644
--- a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h
+++ b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h
@@ -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;
diff --git a/llvm/lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp b/llvm/lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp
index 24cd1b15a5736..df350b9d4814d 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp
@@ -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;
diff --git a/llvm/lib/CodeGen/AsmPrinter/EHStreamer.h b/llvm/lib/CodeGen/AsmPrinter/EHStreamer.h
index 234e62506a563..705a61fb827f3 100644
--- a/llvm/lib/CodeGen/AsmPrinter/EHStreamer.h
+++ b/llvm/lib/CodeGen/AsmPrinter/EHStreamer.h
@@ -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);
diff --git a/llvm/lib/CodeGen/AsmPrinter/PseudoProbePrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/PseudoProbePrinter.cpp
index 59c3fa15885e2..5dda38383a656 100644
--- a/llvm/lib/CodeGen/AsmPrinter/PseudoProbePrinter.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/PseudoProbePrinter.cpp
@@ -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) {
diff --git a/llvm/lib/CodeGen/AsmPrinter/PseudoProbePrinter.h b/llvm/lib/CodeGen/AsmPrinter/PseudoProbePrinter.h
index a92a89084cadb..c9aaed4800f25 100644
--- a/llvm/lib/CodeGen/AsmPrinter/PseudoProbePrinter.h
+++ b/llvm/lib/CodeGen/AsmPrinter/PseudoProbePrinter.h
@@ -21,7 +21,7 @@ namespace llvm {
class AsmPrinter;
class DILocation;
-class PseudoProbeHandler : public AsmPrinterHandler {
+class PseudoProbeHandler {
// Target of pseudo probe emission.
AsmPrinter *Asm;
// Name to GUID map, used as caching/mem...
[truncated]
|
MaskRay
approved these changes
Jun 28, 2024
There was a problem hiding this comment.
LG. Agreed with the analysis. These timers, with their own larger overhead, make their measurement inaccurate.
Eliminating per-instruction overhead yields a small yet non-negligible compile time improvement, as demonstrated by many previous patches made by you and me :) Give other reviewers some time to check.
aeubanks
approved these changes
Jun 28, 2024
|
Consider adding a link to #39650 in the description.
|
Doing per-instruction timing is inaccurate and too costly for a profiling functionality
Timers are an out-of-line function call and a global variable access, here twice per emitted instruction. At this granularity, not only the time results become skewed, but the timers also add a performance overhead when profiling is disabled. Therefore, remove the timers.
aengelke
force-pushed
the
asmprinter-notimer
branch
from
039a158 to
bb1f59e
Compare
lravenclaw
pushed a commit
to lravenclaw/llvm-project
that referenced
this pull request
Jul 3, 2024
Timers are an out-of-line function call and a global variable access, here twice per emitted instruction. At this granularity, not only the time results become skewed, but the timers also add a performance overhead when profiling is disabled. Also outside of the innermost loop, timers add a measurable overhead. As this is quite expensive for a mostly unused profiling facility, remove the timers. Fixes llvm#39650.
kbluck
pushed a commit
to kbluck/llvm-project
that referenced
this pull request
Jul 6, 2024
Timers are an out-of-line function call and a global variable access, here twice per emitted instruction. At this granularity, not only the time results become skewed, but the timers also add a performance overhead when profiling is disabled. Also outside of the innermost loop, timers add a measurable overhead. As this is quite expensive for a mostly unused profiling facility, remove the timers. Fixes llvm#39650.
Zentrik
added a commit
to Zentrik/julia
that referenced
this pull request
Aug 31, 2024
Zentrik
added a commit
to Zentrik/julia
that referenced
this pull request
Sep 6, 2024
This reverts commit 1b7d587.
Zentrik
added a commit
to Zentrik/julia
that referenced
this pull request
Sep 6, 2024
Zentrik
added a commit
to Zentrik/julia
that referenced
this pull request
Sep 27, 2024
Zentrik
added a commit
to Zentrik/julia
that referenced
this pull request
Sep 27, 2024
This reverts commit 1b7d587.
Zentrik
added a commit
to Zentrik/julia
that referenced
this pull request
Sep 27, 2024
Zentrik
added a commit
to Zentrik/julia
that referenced
this pull request
Oct 3, 2024
Zentrik
added a commit
to Zentrik/julia
that referenced
this pull request
Oct 3, 2024
This reverts commit 1b7d587.
Zentrik
added a commit
to Zentrik/julia
that referenced
this pull request
Oct 3, 2024
Zentrik
added a commit
to Zentrik/julia
that referenced
this pull request
Oct 9, 2024
Zentrik
added a commit
to Zentrik/julia
that referenced
this pull request
Oct 9, 2024
This reverts commit 1b7d587.
Zentrik
added a commit
to Zentrik/julia
that referenced
this pull request
Oct 9, 2024
maleadt
pushed a commit
to Zentrik/julia
that referenced
this pull request
Oct 11, 2024
maleadt
pushed a commit
to Zentrik/julia
that referenced
this pull request
Oct 11, 2024
This reverts commit 1b7d587.
maleadt
pushed a commit
to Zentrik/julia
that referenced
this pull request
Oct 11, 2024
Zentrik
added a commit
to Zentrik/julia
that referenced
this pull request
Oct 12, 2024
Zentrik
added a commit
to Zentrik/julia
that referenced
this pull request
Oct 12, 2024
This reverts commit 1b7d587.
Zentrik
added a commit
to Zentrik/julia
that referenced
this pull request
Oct 12, 2024
Zentrik
added a commit
to Zentrik/julia
that referenced
this pull request
Oct 13, 2024
Zentrik
added a commit
to Zentrik/julia
that referenced
this pull request
Oct 13, 2024
This reverts commit 1b7d587.
Zentrik
added a commit
to Zentrik/julia
that referenced
this pull request
Oct 13, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Timers are an out-of-line function call and a global variable access, here twice per emitted instruction. At this granularity, not only the time results become skewed, but the timers also add a performance overhead when profiling is disabled. Also outside of the innermost loop, timers add a measurable overhead. As this is quite expensive for a mostly unused profiling facility, remove the timers.
Fixes #39650.
Follow-up of #96785 (
this PR currently includes the commit from that -- I don't know how to do stacked diffs on Github -- will rebase once that is mergedrebased).There are two actual commits: the first (01abb8b) removes timers just from the innermost loop (introduced in f8dba24 without a reason given in the commit message), the second (039a158) removes them altogether. I propose to remove the timers entirely, given that profilers nowadays can show pretty well how much time is spent in which handler (these are never inlined).