[OptBisect][IR] Adding a new OptPassGate for disabling passes via name #145059
Conversation
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
|
@llvm/pr-subscribers-llvm-ir Author: Cristian Assaiante (cristianassaiante) ChangesI would like to propose a new pass gate that allows selective disabling of one or more passes via the clang command line using the Example: Pass names are matched using case-insensitive comparisons. However, note that special characters, including spaces, must be included exactly as they appear in the pass names. Additionally, a To validate this add-on, a test file has also been provided. It reuses the same infrastructure as the opt-bisect test, but disables three specific passes and checks the output to ensure the expected behavior. Full diff: https://github.com/llvm/llvm-project/pull/145059.diff 3 Files Affected:
diff --git a/clang/test/CodeGen/opt-disable.c b/clang/test/CodeGen/opt-disable.c
new file mode 100644
index 0000000000000..ee90fc5620d65
--- /dev/null
+++ b/clang/test/CodeGen/opt-disable.c
@@ -0,0 +1,13 @@
+// REQUIRES: x86-registered-target
+
+// Make sure opt-bisect works through both pass managers
+//
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -O1 %s -mllvm -opt-disable="inlinerpass,SROAPass,machine code sinking" -mllvm -opt-disable-verbose -emit-obj -o /dev/null 2>&1 | FileCheck %s
+
+// CHECK-NOT: DISABLE: running pass InlinerPass
+// CHECK-NOT: DISABLE: running pass SROAPass
+// CHECK-NOT: DISABLE: running pass Machine code sinking
+// Make sure that legacy pass manager is running
+// CHECK: Instruction Selection
+
+int func(int a) { return a; }
diff --git a/llvm/include/llvm/IR/OptBisect.h b/llvm/include/llvm/IR/OptBisect.h
index be6aef3298b23..51c3a8040da9b 100644
--- a/llvm/include/llvm/IR/OptBisect.h
+++ b/llvm/include/llvm/IR/OptBisect.h
@@ -17,6 +17,7 @@
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Compiler.h"
#include <limits>
+#include <set>
namespace llvm {
@@ -82,6 +83,44 @@ class LLVM_ABI OptBisect : public OptPassGate {
int LastBisectNum = 0;
};
+/// This class implements a mechanism to disable passes and individual
+/// optimizations at compile time based on a command line option
+/// (-opt-disable) in order to study how single transformations, or
+/// combinations thereof, affect the IR.
+class LLVM_ABI OptDisable : public OptPassGate {
+public:
+ /// Default constructor. Initializes the state to empty set. The disabling
+ /// will be enabled by the cl::opt call-back when the command line option
+ /// is processed.
+ /// Clients should not instantiate this class directly. All access should go
+ /// through LLVMContext.
+ OptDisable() = default;
+
+ virtual ~OptDisable() = default;
+
+ /// Checks the pass name to determine if the specified pass should run.
+ ///
+ /// The method prints the name of the pass, and whether or not the pass
+ /// will be executed. It returns true if the pass should run, i.e. if
+ /// its name is was not provided via command line.
+ ///
+ /// Most passes should not call this routine directly. Instead, it is called
+ /// through helper routines provided by the base classes of the pass. For
+ /// instance, function passes should call FunctionPass::skipFunction().
+ bool shouldRunPass(const StringRef PassName,
+ StringRef IRDescription) override;
+
+ /// Parses the command line argument to extract the names of the passes
+ /// to be disabled. Multiple pass names can be provided with comma separation.
+ void setDisabled(StringRef Passes);
+
+ /// isEnabled() should return true before calling shouldRunPass().
+ bool isEnabled() const override { return !DisabledPasses.empty(); }
+
+private:
+ std::set<std::string> DisabledPasses = {};
+};
+
/// Singleton instance of the OptBisect class, so multiple pass managers don't
/// need to coordinate their uses of OptBisect.
LLVM_ABI OptPassGate &getGlobalPassGate();
diff --git a/llvm/lib/IR/OptBisect.cpp b/llvm/lib/IR/OptBisect.cpp
index 559b199445366..aa1dbdfdbebd4 100644
--- a/llvm/lib/IR/OptBisect.cpp
+++ b/llvm/lib/IR/OptBisect.cpp
@@ -17,6 +17,7 @@
#include "llvm/Support/CommandLine.h"
#include "llvm/Support/raw_ostream.h"
#include <cassert>
+#include <sstream>
using namespace llvm;
@@ -37,8 +38,8 @@ static cl::opt<bool> OptBisectVerbose(
cl::desc("Show verbose output when opt-bisect-limit is set"), cl::Hidden,
cl::init(true), cl::Optional);
-static void printPassMessage(const StringRef &Name, int PassNum,
- StringRef TargetDesc, bool Running) {
+static void printBisectPassMessage(const StringRef &Name, int PassNum,
+ StringRef TargetDesc, bool Running) {
StringRef Status = Running ? "" : "NOT ";
errs() << "BISECT: " << Status << "running pass "
<< "(" << PassNum << ") " << Name << " on " << TargetDesc << "\n";
@@ -51,10 +52,64 @@ bool OptBisect::shouldRunPass(const StringRef PassName,
int CurBisectNum = ++LastBisectNum;
bool ShouldRun = (BisectLimit == -1 || CurBisectNum <= BisectLimit);
if (OptBisectVerbose)
- printPassMessage(PassName, CurBisectNum, IRDescription, ShouldRun);
+ printBisectPassMessage(PassName, CurBisectNum, IRDescription, ShouldRun);
return ShouldRun;
}
const int OptBisect::Disabled;
-OptPassGate &llvm::getGlobalPassGate() { return getOptBisector(); }
+static OptDisable &getOptDisabler() {
+ static OptDisable OptDisabler;
+ return OptDisabler;
+}
+
+static cl::opt<std::string> OptDisablePass(
+ "opt-disable", cl::Hidden, cl::init(""), cl::Optional,
+ cl::cb<void, std::string>([](std::string Passes) {
+ getOptDisabler().setDisabled(Passes);
+ }),
+ cl::desc("Optimization pass(es) to disable (comma separated)"));
+
+static cl::opt<bool>
+ OptDisableVerbose("opt-disable-verbose",
+ cl::desc("Show verbose output when opt-disable is set"),
+ cl::Hidden, cl::init(false), cl::Optional);
+
+static void printDisablePassMessage(const StringRef &Name, StringRef TargetDesc,
+ bool Running) {
+ StringRef Status = Running ? "" : "NOT ";
+ errs() << "DISABLE: " << Status << "running pass " << Name << " on "
+ << TargetDesc << "\n";
+}
+
+void OptDisable::setDisabled(StringRef Passes) {
+ std::stringstream StrStream(Passes.str());
+ std::string Token;
+
+ while (std::getline(StrStream, Token, ',')) {
+ if (!Token.empty()) {
+ std::transform(Token.begin(), Token.end(), Token.begin(), ::tolower);
+ DisabledPasses.insert(Token);
+ }
+ }
+}
+
+bool OptDisable::shouldRunPass(const StringRef PassName,
+ StringRef IRDescription) {
+ assert(isEnabled());
+
+ std::string LowerName = PassName.str();
+ std::transform(LowerName.begin(), LowerName.end(), LowerName.begin(),
+ ::tolower);
+
+ bool ShouldRun = DisabledPasses.find(LowerName) == DisabledPasses.end();
+ if (OptDisableVerbose)
+ printDisablePassMessage(PassName, IRDescription, ShouldRun);
+ return ShouldRun;
+}
+
+OptPassGate &llvm::getGlobalPassGate() {
+ if (getOptDisabler().isEnabled())
+ return getOptDisabler();
+ return getOptBisector();
+}
|
|
@llvm/pr-subscribers-clang Author: Cristian Assaiante (cristianassaiante) ChangesI would like to propose a new pass gate that allows selective disabling of one or more passes via the clang command line using the Example: Pass names are matched using case-insensitive comparisons. However, note that special characters, including spaces, must be included exactly as they appear in the pass names. Additionally, a To validate this add-on, a test file has also been provided. It reuses the same infrastructure as the opt-bisect test, but disables three specific passes and checks the output to ensure the expected behavior. Full diff: https://github.com/llvm/llvm-project/pull/145059.diff 3 Files Affected:
diff --git a/clang/test/CodeGen/opt-disable.c b/clang/test/CodeGen/opt-disable.c
new file mode 100644
index 0000000000000..ee90fc5620d65
--- /dev/null
+++ b/clang/test/CodeGen/opt-disable.c
@@ -0,0 +1,13 @@
+// REQUIRES: x86-registered-target
+
+// Make sure opt-bisect works through both pass managers
+//
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -O1 %s -mllvm -opt-disable="inlinerpass,SROAPass,machine code sinking" -mllvm -opt-disable-verbose -emit-obj -o /dev/null 2>&1 | FileCheck %s
+
+// CHECK-NOT: DISABLE: running pass InlinerPass
+// CHECK-NOT: DISABLE: running pass SROAPass
+// CHECK-NOT: DISABLE: running pass Machine code sinking
+// Make sure that legacy pass manager is running
+// CHECK: Instruction Selection
+
+int func(int a) { return a; }
diff --git a/llvm/include/llvm/IR/OptBisect.h b/llvm/include/llvm/IR/OptBisect.h
index be6aef3298b23..51c3a8040da9b 100644
--- a/llvm/include/llvm/IR/OptBisect.h
+++ b/llvm/include/llvm/IR/OptBisect.h
@@ -17,6 +17,7 @@
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Compiler.h"
#include <limits>
+#include <set>
namespace llvm {
@@ -82,6 +83,44 @@ class LLVM_ABI OptBisect : public OptPassGate {
int LastBisectNum = 0;
};
+/// This class implements a mechanism to disable passes and individual
+/// optimizations at compile time based on a command line option
+/// (-opt-disable) in order to study how single transformations, or
+/// combinations thereof, affect the IR.
+class LLVM_ABI OptDisable : public OptPassGate {
+public:
+ /// Default constructor. Initializes the state to empty set. The disabling
+ /// will be enabled by the cl::opt call-back when the command line option
+ /// is processed.
+ /// Clients should not instantiate this class directly. All access should go
+ /// through LLVMContext.
+ OptDisable() = default;
+
+ virtual ~OptDisable() = default;
+
+ /// Checks the pass name to determine if the specified pass should run.
+ ///
+ /// The method prints the name of the pass, and whether or not the pass
+ /// will be executed. It returns true if the pass should run, i.e. if
+ /// its name is was not provided via command line.
+ ///
+ /// Most passes should not call this routine directly. Instead, it is called
+ /// through helper routines provided by the base classes of the pass. For
+ /// instance, function passes should call FunctionPass::skipFunction().
+ bool shouldRunPass(const StringRef PassName,
+ StringRef IRDescription) override;
+
+ /// Parses the command line argument to extract the names of the passes
+ /// to be disabled. Multiple pass names can be provided with comma separation.
+ void setDisabled(StringRef Passes);
+
+ /// isEnabled() should return true before calling shouldRunPass().
+ bool isEnabled() const override { return !DisabledPasses.empty(); }
+
+private:
+ std::set<std::string> DisabledPasses = {};
+};
+
/// Singleton instance of the OptBisect class, so multiple pass managers don't
/// need to coordinate their uses of OptBisect.
LLVM_ABI OptPassGate &getGlobalPassGate();
diff --git a/llvm/lib/IR/OptBisect.cpp b/llvm/lib/IR/OptBisect.cpp
index 559b199445366..aa1dbdfdbebd4 100644
--- a/llvm/lib/IR/OptBisect.cpp
+++ b/llvm/lib/IR/OptBisect.cpp
@@ -17,6 +17,7 @@
#include "llvm/Support/CommandLine.h"
#include "llvm/Support/raw_ostream.h"
#include <cassert>
+#include <sstream>
using namespace llvm;
@@ -37,8 +38,8 @@ static cl::opt<bool> OptBisectVerbose(
cl::desc("Show verbose output when opt-bisect-limit is set"), cl::Hidden,
cl::init(true), cl::Optional);
-static void printPassMessage(const StringRef &Name, int PassNum,
- StringRef TargetDesc, bool Running) {
+static void printBisectPassMessage(const StringRef &Name, int PassNum,
+ StringRef TargetDesc, bool Running) {
StringRef Status = Running ? "" : "NOT ";
errs() << "BISECT: " << Status << "running pass "
<< "(" << PassNum << ") " << Name << " on " << TargetDesc << "\n";
@@ -51,10 +52,64 @@ bool OptBisect::shouldRunPass(const StringRef PassName,
int CurBisectNum = ++LastBisectNum;
bool ShouldRun = (BisectLimit == -1 || CurBisectNum <= BisectLimit);
if (OptBisectVerbose)
- printPassMessage(PassName, CurBisectNum, IRDescription, ShouldRun);
+ printBisectPassMessage(PassName, CurBisectNum, IRDescription, ShouldRun);
return ShouldRun;
}
const int OptBisect::Disabled;
-OptPassGate &llvm::getGlobalPassGate() { return getOptBisector(); }
+static OptDisable &getOptDisabler() {
+ static OptDisable OptDisabler;
+ return OptDisabler;
+}
+
+static cl::opt<std::string> OptDisablePass(
+ "opt-disable", cl::Hidden, cl::init(""), cl::Optional,
+ cl::cb<void, std::string>([](std::string Passes) {
+ getOptDisabler().setDisabled(Passes);
+ }),
+ cl::desc("Optimization pass(es) to disable (comma separated)"));
+
+static cl::opt<bool>
+ OptDisableVerbose("opt-disable-verbose",
+ cl::desc("Show verbose output when opt-disable is set"),
+ cl::Hidden, cl::init(false), cl::Optional);
+
+static void printDisablePassMessage(const StringRef &Name, StringRef TargetDesc,
+ bool Running) {
+ StringRef Status = Running ? "" : "NOT ";
+ errs() << "DISABLE: " << Status << "running pass " << Name << " on "
+ << TargetDesc << "\n";
+}
+
+void OptDisable::setDisabled(StringRef Passes) {
+ std::stringstream StrStream(Passes.str());
+ std::string Token;
+
+ while (std::getline(StrStream, Token, ',')) {
+ if (!Token.empty()) {
+ std::transform(Token.begin(), Token.end(), Token.begin(), ::tolower);
+ DisabledPasses.insert(Token);
+ }
+ }
+}
+
+bool OptDisable::shouldRunPass(const StringRef PassName,
+ StringRef IRDescription) {
+ assert(isEnabled());
+
+ std::string LowerName = PassName.str();
+ std::transform(LowerName.begin(), LowerName.end(), LowerName.begin(),
+ ::tolower);
+
+ bool ShouldRun = DisabledPasses.find(LowerName) == DisabledPasses.end();
+ if (OptDisableVerbose)
+ printDisablePassMessage(PassName, IRDescription, ShouldRun);
+ return ShouldRun;
+}
+
+OptPassGate &llvm::getGlobalPassGate() {
+ if (getOptDisabler().isEnabled())
+ return getOptDisabler();
+ return getOptBisector();
+}
|
Cristian has been studying the impact of optimizations on debug information and thus the impact on AutoFDO profile quality. @mtrofin and I encouraged him to put this patch up for review so that we can reproduce his findings on internal workloads and share the insights with the rest of the community. I think this functionality will also help identify passes which drop debug information and help with the work proposed in this RFC. cc: @jmorse @SLTozer |
As it happens, I have been using a downstream modification of opt-bisect as part of my triage process for bugs detected with debugify, by counting optimization passes and printing them alongside detected bugs, making it easy to create reproducers using It would be useful in general for determining passes responsible for dropping debug information, however - I've previously spent time working on finding pass configurations that produce better debug info than the existing O2/O3 pipelines without compromising too greatly on performance - this feature would have made that process easier, and I suspect will make it easier for others to do similar investigations in future, so I give a vote for adding this! |
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
Co-authored-by: Nikita Popov <[email protected]>
Sure, sorry about that. |
cristianassaiante
force-pushed
the
opt-disable
branch
from
4810d32 to
39b85f1
Compare
|
@nikic Done! You should be able to see only my changes now. |
Co-authored-by: Nikita Popov <[email protected]>
There was a problem hiding this comment.
Thanks for the review @nikic.
@cristianassaiante I'll squash and merge the changes now. Thanks for contributing this feature to LLVM.
|
@cristianassaiante The linux CI check is failing and it looks like it's related to this change. Raw logs for the CI run are here. Can you take a look? |
Should be good now, one bisect test was wrongly left unchanged. Sorry. |
|
@cristianassaiante Congratulations on having your first Pull Request (PR) merged into the LLVM Project! Your changes will be combined with recent changes from other authors, then tested by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR. Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues. How to do this, and the rest of the post-merge process, is covered in detail here. If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again. If you don't get any reports, no action is required from you. Your changes are working as expected, well done! |
I would like to propose a new pass gate that allows selective disabling of one or more passes via the clang command line using the
-opt-disableoption. Passes to be disabled should be specified as a comma-separated list of their names.The implementation resides in the same file as the bisection tool. The
getGlobalPassGate()function returns the currently enabled gate.Example:
-opt-disable="PassA,PassB"Pass names are matched using case-insensitive comparisons. However, note that special characters, including spaces, must be included exactly as they appear in the pass names.
Additionally, a
-opt-disable-verboseflag has been introduced to enable verbose output when this functionality is in use. When enabled, it prints the status of all passes (either running or NOT running), similar to the default behavior of-opt-bisect-limit. This flag is disabled by default, which is the opposite of the-opt-bisect-verboseflag (which defaults to enabled).To validate this add-on, a test file has also been provided. It reuses the same infrastructure as the opt-bisect test, but disables three specific passes and checks the output to ensure the expected behavior.