-
Notifications
You must be signed in to change notification settings - Fork 13.6k
Add a pass to collect dropped variable statistics #102233
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks useful; in the current form the patch needs comments and a test!
I echo Adrian's comments about tests, though I am unsure how to do it.
This will easily be optimized away / dropped |
793249c
to
4bff433
Compare
The dropped statistics for the updated patch are: AlwaysInlinerPass, 2 |
Inlining shouldn't really be dropping any variables — how do these happen / do they make sense? |
@llvm/pr-subscribers-llvm-ir @llvm/pr-subscribers-debuginfo Author: Shubham Sandeep Rastogi (rastogishubham) ChangesThis patch is inspired by @Snowy1803 excellent work in swift and the patch: https://github.com/swiftlang/swift/pull/73334/files Add an instrumentation pass to llvm to collect dropped debug information variable statistics for every Function-level and Module-level IR pass. This patch creates adds the class DroppedVariableStats which iterates over every DbgRecord in a function or module before and after an optimization pass and counts the number of variables who's debug information has been dropped due to that pass, then prints that output to stdout in a csv format. I ran this patch on optdriver.cpp with the command line:
And I see the result: Pass Name, Dropped Variables Full diff: https://github.com/llvm/llvm-project/pull/102233.diff 3 Files Affected:
diff --git a/llvm/include/llvm/Passes/StandardInstrumentations.h b/llvm/include/llvm/Passes/StandardInstrumentations.h
index fa9c744294a666..c5d68dd73a4a05 100644
--- a/llvm/include/llvm/Passes/StandardInstrumentations.h
+++ b/llvm/include/llvm/Passes/StandardInstrumentations.h
@@ -559,6 +559,37 @@ class DotCfgChangeReporter : public ChangeReporter<IRDataT<DCData>> {
std::unique_ptr<raw_fd_ostream> HTML;
};
+class DroppedVariableStats {
+public:
+ DroppedVariableStats(bool DroppedVarStatsEnabled) {
+ if (DroppedVarStatsEnabled)
+ llvm::outs()
+ << "Pass Level, Pass Name, Num of Dropped Variables, Func or "
+ "Module Name\n";
+ };
+ // We intend this to be unique per-compilation, thus no copies.
+ DroppedVariableStats(const DroppedVariableStats &) = delete;
+ void operator=(const DroppedVariableStats &) = delete;
+
+ void registerCallbacks(PassInstrumentationCallbacks &PIC);
+
+private:
+ using VarID = std::tuple<const DILocalScope *, const DILocalScope *,
+ StringRef, unsigned>;
+
+ SmallVector<llvm::DenseSet<VarID>> DebugVariablesBefore;
+ SmallVector<llvm::DenseSet<VarID>> DebugVariablesAfter;
+
+ // Implementation of pass instrumentation callbacks.
+ void runBeforePass(StringRef PassID, Any IR);
+ void runAfterPass(StringRef PassID, Any IR, const PreservedAnalyses &PA);
+
+ void runOnFunction(const Function *F, bool Before);
+ void runOnModule(const Module *M, bool Before);
+
+ void removeVarFromAllSets(VarID Var);
+};
+
// Print IR on crash.
class PrintCrashIRInstrumentation {
public:
@@ -595,6 +626,7 @@ class StandardInstrumentations {
PrintCrashIRInstrumentation PrintCrashIR;
IRChangedTester ChangeTester;
VerifyInstrumentation Verify;
+ DroppedVariableStats DroppedStats;
bool VerifyEach;
diff --git a/llvm/lib/Passes/StandardInstrumentations.cpp b/llvm/lib/Passes/StandardInstrumentations.cpp
index 8f2461f40cb004..a12b3bd8a88d99 100644
--- a/llvm/lib/Passes/StandardInstrumentations.cpp
+++ b/llvm/lib/Passes/StandardInstrumentations.cpp
@@ -25,6 +25,8 @@
#include "llvm/Demangle/Demangle.h"
#include "llvm/IR/Constants.h"
#include "llvm/IR/Function.h"
+#include "llvm/IR/InstIterator.h"
+#include "llvm/IR/IntrinsicInst.h"
#include "llvm/IR/Module.h"
#include "llvm/IR/PassInstrumentation.h"
#include "llvm/IR/PassManager.h"
@@ -139,6 +141,11 @@ static cl::opt<std::string> IRDumpDirectory(
"files in this directory rather than written to stderr"),
cl::Hidden, cl::value_desc("filename"));
+static cl::opt<bool>
+ DroppedVarStats("dropped-variable-stats", cl::Hidden,
+ cl::desc("Dump dropped debug variables stats"),
+ cl::init(false));
+
template <typename IRUnitT> static const IRUnitT *unwrapIR(Any IR) {
const IRUnitT **IRPtr = llvm::any_cast<const IRUnitT *>(&IR);
return IRPtr ? *IRPtr : nullptr;
@@ -2441,11 +2448,92 @@ void DotCfgChangeReporter::registerCallbacks(
}
}
+void DroppedVariableStats::registerCallbacks(
+ PassInstrumentationCallbacks &PIC) {
+ if (!DroppedVarStats)
+ return;
+
+ PIC.registerBeforeNonSkippedPassCallback(
+ [this](StringRef P, Any IR) { return this->runBeforePass(P, IR); });
+ PIC.registerAfterPassCallback(
+ [this](StringRef P, Any IR, const PreservedAnalyses &PA) {
+ return this->runAfterPass(P, IR, PA);
+ });
+}
+
+void DroppedVariableStats::runBeforePass(StringRef PassID, Any IR) {
+ DebugVariablesBefore.push_back(llvm::DenseSet<VarID>());
+ DebugVariablesAfter.push_back(llvm::DenseSet<VarID>());
+ if (auto *M = unwrapIR<Module>(IR))
+ return this->runOnModule(M, true);
+ if (auto *F = unwrapIR<Function>(IR))
+ return this->runOnFunction(F, true);
+ return;
+}
+
+void DroppedVariableStats::runOnFunction(const Function *F, bool Before) {
+ auto &VarIDs = (Before ? DebugVariablesBefore : DebugVariablesAfter).back();
+ for (const auto &I : instructions(F)) {
+ for (DbgRecord &DR : I.getDbgRecordRange()) {
+ if (auto *Dbg = dyn_cast<DbgVariableRecord>(&DR)) {
+ auto *DbgVar = Dbg->getVariable();
+ StringRef UniqueName = DbgVar->getName();
+ auto DbgLoc = DR.getDebugLoc();
+ unsigned Line = DbgVar->getLine();
+ VarID Key{cast<DILocalScope>(DbgVar->getScope()),
+ DbgLoc->getInlinedAtScope(), UniqueName, Line};
+ VarIDs.insert(Key);
+ }
+ }
+ }
+}
+
+void DroppedVariableStats::runOnModule(const Module *M, bool Before) {
+ for (auto &F : *M)
+ runOnFunction(&F, Before);
+}
+
+void DroppedVariableStats::removeVarFromAllSets(VarID Var) {
+ // Do not remove Var from the last element, it will be popped from the stack
+ // anyway.
+ for (auto *It = DebugVariablesBefore.begin();
+ It != DebugVariablesBefore.end() - 1; It++)
+ It->erase(Var);
+}
+
+void DroppedVariableStats::runAfterPass(StringRef PassID, Any IR,
+ const PreservedAnalyses &PA) {
+ unsigned DroppedCount = 0;
+ std::string PassLevel;
+ std::string FuncOrModName;
+ if (auto *M = unwrapIR<Module>(IR)) {
+ this->runOnModule(M, false);
+ PassLevel = "Module";
+ FuncOrModName = M->getName();
+ } else if (auto *F = unwrapIR<Function>(IR)) {
+ this->runOnFunction(F, false);
+ PassLevel = "Function";
+ FuncOrModName = F->getName();
+ } else {
+ return;
+ }
+ for (auto Var : DebugVariablesBefore.back())
+ if (!DebugVariablesAfter.back().contains(Var)) {
+ DroppedCount++;
+ removeVarFromAllSets(Var);
+ }
+ if (DroppedCount > 0)
+ llvm::outs() << PassLevel << ", " << PassID << ", " << DroppedCount << ", "
+ << FuncOrModName << "\n";
+ DebugVariablesBefore.pop_back();
+ DebugVariablesAfter.pop_back();
+ return;
+}
+
StandardInstrumentations::StandardInstrumentations(
LLVMContext &Context, bool DebugLogging, bool VerifyEach,
PrintPassOptions PrintPassOpts)
- : PrintPass(DebugLogging, PrintPassOpts),
- OptNone(DebugLogging),
+ : PrintPass(DebugLogging, PrintPassOpts), OptNone(DebugLogging),
OptPassGate(Context),
PrintChangedIR(PrintChanged == ChangePrinter::Verbose),
PrintChangedDiff(PrintChanged == ChangePrinter::DiffVerbose ||
@@ -2453,7 +2541,8 @@ StandardInstrumentations::StandardInstrumentations(
PrintChanged == ChangePrinter::ColourDiffVerbose ||
PrintChanged == ChangePrinter::ColourDiffQuiet),
WebsiteChangeReporter(PrintChanged == ChangePrinter::DotCfgVerbose),
- Verify(DebugLogging), VerifyEach(VerifyEach) {}
+ Verify(DebugLogging), DroppedStats(DroppedVarStats),
+ VerifyEach(VerifyEach) {}
PrintCrashIRInstrumentation *PrintCrashIRInstrumentation::CrashReporter =
nullptr;
@@ -2528,6 +2617,7 @@ void StandardInstrumentations::registerCallbacks(
WebsiteChangeReporter.registerCallbacks(PIC);
ChangeTester.registerCallbacks(PIC);
PrintCrashIR.registerCallbacks(PIC);
+ DroppedStats.registerCallbacks(PIC);
if (MAM)
PreservedCFGChecker.registerCallbacks(PIC, *MAM);
diff --git a/llvm/test/Other/dropped-var-stats.ll b/llvm/test/Other/dropped-var-stats.ll
new file mode 100644
index 00000000000000..93e00f2c744907
--- /dev/null
+++ b/llvm/test/Other/dropped-var-stats.ll
@@ -0,0 +1,26 @@
+; RUN: opt -dropped-variable-stats %s -passes='adce' -S | FileCheck %s
+
+; CHECK: Pass Level, Pass Name, Num of Dropped Variables, Func or Module Name
+; CHECK-NEXT: Function, ADCEPass, 1, _Z3bari
+
+; ModuleID = '/tmp/dropped.cpp'
+define noundef range(i32 -2147483646, -2147483648) i32 @_Z3bari(i32 noundef %y) local_unnamed_addr #1 !dbg !19 {
+ #dbg_value(i32 %y, !15, !DIExpression(), !23)
+ %add = add nsw i32 %y, 2,!dbg !25
+ ret i32 %add,!dbg !26
+}
+!llvm.module.flags = !{ !3, !7}
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, sysroot: "/")
+!1 = !DIFile(filename: "/tmp/dropped.cpp", directory: "/Users/shubham/Development/llvm-project")
+!3 = !{i32 2, !"Debug Info Version", i32 3}
+!7 = !{i32 7, !"frame-pointer", i32 1}
+!9 = distinct !DISubprogram( unit: !0, retainedNodes: !14)
+!13 = !DIBasicType()
+!14 = !{}
+!15 = !DILocalVariable( scope: !9, type: !13)
+!19 = distinct !DISubprogram( unit: !0, retainedNodes: !20)
+!20 = !{}
+!23 = !DILocation( scope: !9, inlinedAt: !24)
+!24 = distinct !DILocation( scope: !19)
+!25 = !DILocation( scope: !19)
+!26 = !DILocation( scope: !19)
|
Looks quite helpful, thanks for working on this! 😄 Does this report variables that have been dropped entirely, or also ones which have lost partial coverage...? I can't quite tell from the code, as you do appear to track some variable line info... |
Currently only variables that are dropped entirely, it doesn't track fragments yet. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, although I'd suggest you want a test to check that nothing is printed when nothing is dropped (such as when the verifier runs), purely as a sanity-check. Thanks for working on this!
What's motivated using the line-number to identify a variable, instead of using the DILocalVariable directly? There's the DebugVariable
class out there that exists for variable-identity purposes, it even has fragment-handling built in.
In this implementation "dropped" is meaning "all the records of its location disappear" as opposed to "it's completely optimised away and all the records have an undef/poison value". I think sometimes we use the terms interchangeably, the former seems like the most useful in this context though.
4bff433
to
3e35650
Compare
I have updated the patch to make the dropped variable statistics more accurate. We do not want to count a variable as dropped if the function that contained the dbg_val was eliminated due to dead code elimination. Do this by checking if the This still won't be perfect, the real instruction we are looking for, could have either the same scope or any child scope of that scope to be considered dropped. The new numbers for optdriver.cpp are: SCCPPass: 6 |
52efbcc
to
fbdcb19
Compare
@jmorse Thanks for the feedback! I added a testcase to make sure we don't report dropped variables in a pass like the verifier pass. I also didn't think of using the DILocalVarible, it was an oversight, I have updated the patch with your feedback! |
fbdcb19
to
fc20332
Compare
@adrian-prantl @jmorse @jryans just pinging again to see if there were any other changes or is this patch good to go in |
#dbg_value(i32 %y, !15, !DIExpression(), !23) | ||
%add = add nsw i32 %y, 2,!dbg !25 | ||
ret i32 %add,!dbg !26 | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're missing tests for the inline case and/or a lexical scope with a dbg_value that has all of its (childrens') instructions eliminated.
fc20332
to
9dd69c5
Compare
"CSK_MD5, checksum: \"719364c4b07176af8515cac6bd21008c\")\n" | ||
"!2 = !{i32 7, !\"Dwarf Version\", i32 5}\n" | ||
"!3 = !{i32 2, !\"Debug Info Version\", i32 3}\n" | ||
"!4 = !{i32 1, !\"wchar_size\", i32 4}\n" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think you need any of these
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So weirdly enough, the DWARF version is the one that is needed, but everything else is removable
9dd69c5
to
fc4a352
Compare
|
||
void DroppedVariableStats::runBeforePass(StringRef PassID, Any IR) { | ||
DebugVariablesStack.push_back( | ||
{DenseMap<const Function *, DenseSet<VarID>>(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this work too?
DebugVariablesStack.push_back({})
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No that does not work
fc4a352
to
f91a34e
Compare
I ran the dropped variable statistics on SemaExpr.cpp in -O2 using the invocation:
And I see: 'SCCPPass': 1, Seems like JumpThreading and SimplifyCFG passes drop the most debug information On -O3 (SemaExpr.cpp.o is built with -O3 by default):
'SCCPPass': 7 |
This patch adds the class DroppedVariableStats to StandardInstrumentations which gathers information on whether debug information was dropped by an optimization pass in llvm. This runs on every Function-level and Module-level IR pass.
f91a34e
to
8180056
Compare
This patch is inspired by @Snowy1803 excellent work in swift and the patch: https://github.com/swiftlang/swift/pull/73334/files Add an instrumentation pass to llvm to collect dropped debug information variable statistics for every Function-level and Module-level IR pass. This patch creates adds the class DroppedVariableStats which iterates over every DbgRecord in a function or module before and after an optimization pass and counts the number of variables who's debug information has been dropped due to that pass, then prints that output to stdout in a csv format. I ran this patch on optdriver.cpp can see: Pass Name, Dropped Variables 'InstCombinePass', 1 'SimplifyCFGPass', 6 'JumpThreadingPass', 25 (cherry picked from commit b8930cd)
Add a pass to collect dropped variable statistics (llvm#102233)
This patch is inspired by @Snowy1803 excellent work in swift and the patch: https://github.com/swiftlang/swift/pull/73334/files
Add an instrumentation pass to llvm to collect dropped debug information variable statistics for every Function-level and Module-level IR pass.
This patch creates adds the class DroppedVariableStats which iterates over every DbgRecord in a function or module before and after an optimization pass and counts the number of variables who's debug information has been dropped due to that pass, then prints that output to stdout in a csv format.
I ran this patch on optdriver.cpp with the command line:
And I see the result:
Pass Name, Dropped Variables
'SCCPPass', 6
'InstCombinePass', 78)
'GlobalOptPass', 84
'SimplifyCFGPass', 131
'JumpThreadingPass', 385
'ModuleToFunctionPassAdaptor', 4053
'SROAPass', 4128
'ADCEPass', 5435
'PassManager', 9032