[LLVM] Change `ModulePass::skipModule` to take a const reference #146168

jurahul · 2025-06-27T22:03:13Z

Change ModulePass::skipModule to take const Module reference. Additionally, make OptPassGate::shouldRunPass const as well as for most implementations it's a const query. For OptBisect, make LastBisectNum mutable so it could be updated in shouldRunPass.

Additional minor cleanup: Change all StringRef arguments to simple StringRef (no const or reference), change OptBisect::Disabled to constexpr.

- Change `ModulePass::skipModule` to take const Module reference. - Additionally, make `OptPassGate::shouldRunPass` const as well as for most implementations its a const query. For `OptBisect`, make `LastBisectNum` mutable so it could be updated in `shouldRunPass`. - Minor code cleanup.

llvmbot · 2025-06-28T00:49:36Z

@llvm/pr-subscribers-llvm-ir

Author: Rahul Joshi (jurahul)

Changes

Change ModulePass::skipModule to take const Module reference. Additionally, make OptPassGate::shouldRunPass const as well as for most implementations it's a const query. For OptBisect, make LastBisectNum mutable so it could be updated in shouldRunPass.

Additional minor cleanup: Change all StringRef arguments to simple StringRef (no const or reference), change OptBisect::Disabled to constexpr.

Full diff: https://github.com/llvm/llvm-project/pull/146168.diff

8 Files Affected:

(modified) llvm/include/llvm/IR/OptBisect.h (+6-6)
(modified) llvm/include/llvm/Pass.h (+1-1)
(modified) llvm/lib/Analysis/LoopPass.cpp (+1-1)
(modified) llvm/lib/Analysis/RegionPass.cpp (+1-1)
(modified) llvm/lib/IR/OptBisect.cpp (+6-8)
(modified) llvm/lib/IR/Pass.cpp (+2-2)
(modified) llvm/lib/Passes/StandardInstrumentations.cpp (+1-1)
(modified) llvm/unittests/IR/LegacyPassManagerTest.cpp (+2-1)

diff --git a/llvm/include/llvm/IR/OptBisect.h b/llvm/include/llvm/IR/OptBisect.h
index be6aef3298b23..ea3c1defeb100 100644
--- a/llvm/include/llvm/IR/OptBisect.h
+++ b/llvm/include/llvm/IR/OptBisect.h
@@ -28,8 +28,8 @@ class OptPassGate {
 
   /// IRDescription is a textual description of the IR unit the pass is running
   /// over.
-  virtual bool shouldRunPass(const StringRef PassName,
-                             StringRef IRDescription) {
+  virtual bool shouldRunPass(StringRef PassName,
+                             StringRef IRDescription) const {
     return true;
   }
 
@@ -62,8 +62,8 @@ class LLVM_ABI OptBisect : public OptPassGate {
   /// 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;
+  bool shouldRunPass(StringRef PassName,
+                     StringRef IRDescription) const override;
 
   /// isEnabled() should return true before calling shouldRunPass().
   bool isEnabled() const override { return BisectLimit != Disabled; }
@@ -75,11 +75,11 @@ class LLVM_ABI OptBisect : public OptPassGate {
     LastBisectNum = 0;
   }
 
-  static const int Disabled = std::numeric_limits<int>::max();
+  static constexpr int Disabled = std::numeric_limits<int>::max();
 
 private:
   int BisectLimit = Disabled;
-  int LastBisectNum = 0;
+  mutable int LastBisectNum = 0;
 };
 
 /// Singleton instance of the OptBisect class, so multiple pass managers don't
diff --git a/llvm/include/llvm/Pass.h b/llvm/include/llvm/Pass.h
index 34f4f7f804a38..2ecd47dd10bde 100644
--- a/llvm/include/llvm/Pass.h
+++ b/llvm/include/llvm/Pass.h
@@ -271,7 +271,7 @@ class LLVM_ABI ModulePass : public Pass {
 protected:
   /// Optional passes call this function to check whether the pass should be
   /// skipped. This is the case when optimization bisect is over the limit.
-  bool skipModule(Module &M) const;
+  bool skipModule(const Module &M) const;
 };
 
 //===----------------------------------------------------------------------===//
diff --git a/llvm/lib/Analysis/LoopPass.cpp b/llvm/lib/Analysis/LoopPass.cpp
index 5d0f9abbb5ce7..d8680aac74b22 100644
--- a/llvm/lib/Analysis/LoopPass.cpp
+++ b/llvm/lib/Analysis/LoopPass.cpp
@@ -373,7 +373,7 @@ bool LoopPass::skipLoop(const Loop *L) const {
   if (!F)
     return false;
   // Check the opt bisect limit.
-  OptPassGate &Gate = F->getContext().getOptPassGate();
+  const OptPassGate &Gate = F->getContext().getOptPassGate();
   if (Gate.isEnabled() &&
       !Gate.shouldRunPass(this->getPassName(), getDescription(*L)))
     return true;
diff --git a/llvm/lib/Analysis/RegionPass.cpp b/llvm/lib/Analysis/RegionPass.cpp
index 9ea7d711918f5..ae1d84659de86 100644
--- a/llvm/lib/Analysis/RegionPass.cpp
+++ b/llvm/lib/Analysis/RegionPass.cpp
@@ -282,7 +282,7 @@ static std::string getDescription(const Region &R) {
 
 bool RegionPass::skipRegion(Region &R) const {
   Function &F = *R.getEntry()->getParent();
-  OptPassGate &Gate = F.getContext().getOptPassGate();
+  const OptPassGate &Gate = F.getContext().getOptPassGate();
   if (Gate.isEnabled() &&
       !Gate.shouldRunPass(this->getPassName(), getDescription(R)))
     return true;
diff --git a/llvm/lib/IR/OptBisect.cpp b/llvm/lib/IR/OptBisect.cpp
index 559b199445366..427e8b78fd03f 100644
--- a/llvm/lib/IR/OptBisect.cpp
+++ b/llvm/lib/IR/OptBisect.cpp
@@ -37,15 +37,15 @@ 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 printPassMessage(StringRef Name, int PassNum, StringRef TargetDesc,
+                             bool Running) {
   StringRef Status = Running ? "" : "NOT ";
-  errs() << "BISECT: " << Status << "running pass "
-         << "(" << PassNum << ") " << Name << " on " << TargetDesc << "\n";
+  errs() << "BISECT: " << Status << "running pass (" << PassNum << ") " << Name
+         << " on " << TargetDesc << '\n';
 }
 
-bool OptBisect::shouldRunPass(const StringRef PassName,
-                              StringRef IRDescription) {
+bool OptBisect::shouldRunPass(StringRef PassName,
+                              StringRef IRDescription) const {
   assert(isEnabled());
 
   int CurBisectNum = ++LastBisectNum;
@@ -55,6 +55,4 @@ bool OptBisect::shouldRunPass(const StringRef PassName,
   return ShouldRun;
 }
 
-const int OptBisect::Disabled;
-
 OptPassGate &llvm::getGlobalPassGate() { return getOptBisector(); }
diff --git a/llvm/lib/IR/Pass.cpp b/llvm/lib/IR/Pass.cpp
index f3ae2e6314429..2c5ef7193b463 100644
--- a/llvm/lib/IR/Pass.cpp
+++ b/llvm/lib/IR/Pass.cpp
@@ -60,8 +60,8 @@ static std::string getDescription(const Module &M) {
   return "module (" + M.getName().str() + ")";
 }
 
-bool ModulePass::skipModule(Module &M) const {
-  OptPassGate &Gate = M.getContext().getOptPassGate();
+bool ModulePass::skipModule(const Module &M) const {
+  const OptPassGate &Gate = M.getContext().getOptPassGate();
   return Gate.isEnabled() &&
          !Gate.shouldRunPass(this->getPassName(), getDescription(M));
 }
diff --git a/llvm/lib/Passes/StandardInstrumentations.cpp b/llvm/lib/Passes/StandardInstrumentations.cpp
index 4e8abc43572d7..0623e66772047 100644
--- a/llvm/lib/Passes/StandardInstrumentations.cpp
+++ b/llvm/lib/Passes/StandardInstrumentations.cpp
@@ -1074,7 +1074,7 @@ bool OptPassGateInstrumentation::shouldRun(StringRef PassName, Any IR) {
 
 void OptPassGateInstrumentation::registerCallbacks(
     PassInstrumentationCallbacks &PIC) {
-  OptPassGate &PassGate = Context.getOptPassGate();
+  const OptPassGate &PassGate = Context.getOptPassGate();
   if (!PassGate.isEnabled())
     return;
 
diff --git a/llvm/unittests/IR/LegacyPassManagerTest.cpp b/llvm/unittests/IR/LegacyPassManagerTest.cpp
index 71241e44831e0..00ceebaff4160 100644
--- a/llvm/unittests/IR/LegacyPassManagerTest.cpp
+++ b/llvm/unittests/IR/LegacyPassManagerTest.cpp
@@ -359,7 +359,8 @@ namespace llvm {
     struct CustomOptPassGate : public OptPassGate {
       bool Skip;
       CustomOptPassGate(bool Skip) : Skip(Skip) { }
-      bool shouldRunPass(const StringRef PassName, StringRef IRDescription) override {
+      bool shouldRunPass(StringRef PassName,
+                         StringRef IRDescription) const override {
         return !Skip;
       }
       bool isEnabled() const override { return true; }

llvmbot · 2025-06-28T00:49:37Z

@llvm/pr-subscribers-llvm-analysis

Author: Rahul Joshi (jurahul)

Changes

Change ModulePass::skipModule to take const Module reference. Additionally, make OptPassGate::shouldRunPass const as well as for most implementations it's a const query. For OptBisect, make LastBisectNum mutable so it could be updated in shouldRunPass.

Additional minor cleanup: Change all StringRef arguments to simple StringRef (no const or reference), change OptBisect::Disabled to constexpr.

Full diff: https://github.com/llvm/llvm-project/pull/146168.diff

8 Files Affected:

(modified) llvm/include/llvm/IR/OptBisect.h (+6-6)
(modified) llvm/include/llvm/Pass.h (+1-1)
(modified) llvm/lib/Analysis/LoopPass.cpp (+1-1)
(modified) llvm/lib/Analysis/RegionPass.cpp (+1-1)
(modified) llvm/lib/IR/OptBisect.cpp (+6-8)
(modified) llvm/lib/IR/Pass.cpp (+2-2)
(modified) llvm/lib/Passes/StandardInstrumentations.cpp (+1-1)
(modified) llvm/unittests/IR/LegacyPassManagerTest.cpp (+2-1)

diff --git a/llvm/include/llvm/IR/OptBisect.h b/llvm/include/llvm/IR/OptBisect.h
index be6aef3298b23..ea3c1defeb100 100644
--- a/llvm/include/llvm/IR/OptBisect.h
+++ b/llvm/include/llvm/IR/OptBisect.h
@@ -28,8 +28,8 @@ class OptPassGate {
 
   /// IRDescription is a textual description of the IR unit the pass is running
   /// over.
-  virtual bool shouldRunPass(const StringRef PassName,
-                             StringRef IRDescription) {
+  virtual bool shouldRunPass(StringRef PassName,
+                             StringRef IRDescription) const {
     return true;
   }
 
@@ -62,8 +62,8 @@ class LLVM_ABI OptBisect : public OptPassGate {
   /// 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;
+  bool shouldRunPass(StringRef PassName,
+                     StringRef IRDescription) const override;
 
   /// isEnabled() should return true before calling shouldRunPass().
   bool isEnabled() const override { return BisectLimit != Disabled; }
@@ -75,11 +75,11 @@ class LLVM_ABI OptBisect : public OptPassGate {
     LastBisectNum = 0;
   }
 
-  static const int Disabled = std::numeric_limits<int>::max();
+  static constexpr int Disabled = std::numeric_limits<int>::max();
 
 private:
   int BisectLimit = Disabled;
-  int LastBisectNum = 0;
+  mutable int LastBisectNum = 0;
 };
 
 /// Singleton instance of the OptBisect class, so multiple pass managers don't
diff --git a/llvm/include/llvm/Pass.h b/llvm/include/llvm/Pass.h
index 34f4f7f804a38..2ecd47dd10bde 100644
--- a/llvm/include/llvm/Pass.h
+++ b/llvm/include/llvm/Pass.h
@@ -271,7 +271,7 @@ class LLVM_ABI ModulePass : public Pass {
 protected:
   /// Optional passes call this function to check whether the pass should be
   /// skipped. This is the case when optimization bisect is over the limit.
-  bool skipModule(Module &M) const;
+  bool skipModule(const Module &M) const;
 };
 
 //===----------------------------------------------------------------------===//
diff --git a/llvm/lib/Analysis/LoopPass.cpp b/llvm/lib/Analysis/LoopPass.cpp
index 5d0f9abbb5ce7..d8680aac74b22 100644
--- a/llvm/lib/Analysis/LoopPass.cpp
+++ b/llvm/lib/Analysis/LoopPass.cpp
@@ -373,7 +373,7 @@ bool LoopPass::skipLoop(const Loop *L) const {
   if (!F)
     return false;
   // Check the opt bisect limit.
-  OptPassGate &Gate = F->getContext().getOptPassGate();
+  const OptPassGate &Gate = F->getContext().getOptPassGate();
   if (Gate.isEnabled() &&
       !Gate.shouldRunPass(this->getPassName(), getDescription(*L)))
     return true;
diff --git a/llvm/lib/Analysis/RegionPass.cpp b/llvm/lib/Analysis/RegionPass.cpp
index 9ea7d711918f5..ae1d84659de86 100644
--- a/llvm/lib/Analysis/RegionPass.cpp
+++ b/llvm/lib/Analysis/RegionPass.cpp
@@ -282,7 +282,7 @@ static std::string getDescription(const Region &R) {
 
 bool RegionPass::skipRegion(Region &R) const {
   Function &F = *R.getEntry()->getParent();
-  OptPassGate &Gate = F.getContext().getOptPassGate();
+  const OptPassGate &Gate = F.getContext().getOptPassGate();
   if (Gate.isEnabled() &&
       !Gate.shouldRunPass(this->getPassName(), getDescription(R)))
     return true;
diff --git a/llvm/lib/IR/OptBisect.cpp b/llvm/lib/IR/OptBisect.cpp
index 559b199445366..427e8b78fd03f 100644
--- a/llvm/lib/IR/OptBisect.cpp
+++ b/llvm/lib/IR/OptBisect.cpp
@@ -37,15 +37,15 @@ 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 printPassMessage(StringRef Name, int PassNum, StringRef TargetDesc,
+                             bool Running) {
   StringRef Status = Running ? "" : "NOT ";
-  errs() << "BISECT: " << Status << "running pass "
-         << "(" << PassNum << ") " << Name << " on " << TargetDesc << "\n";
+  errs() << "BISECT: " << Status << "running pass (" << PassNum << ") " << Name
+         << " on " << TargetDesc << '\n';
 }
 
-bool OptBisect::shouldRunPass(const StringRef PassName,
-                              StringRef IRDescription) {
+bool OptBisect::shouldRunPass(StringRef PassName,
+                              StringRef IRDescription) const {
   assert(isEnabled());
 
   int CurBisectNum = ++LastBisectNum;
@@ -55,6 +55,4 @@ bool OptBisect::shouldRunPass(const StringRef PassName,
   return ShouldRun;
 }
 
-const int OptBisect::Disabled;
-
 OptPassGate &llvm::getGlobalPassGate() { return getOptBisector(); }
diff --git a/llvm/lib/IR/Pass.cpp b/llvm/lib/IR/Pass.cpp
index f3ae2e6314429..2c5ef7193b463 100644
--- a/llvm/lib/IR/Pass.cpp
+++ b/llvm/lib/IR/Pass.cpp
@@ -60,8 +60,8 @@ static std::string getDescription(const Module &M) {
   return "module (" + M.getName().str() + ")";
 }
 
-bool ModulePass::skipModule(Module &M) const {
-  OptPassGate &Gate = M.getContext().getOptPassGate();
+bool ModulePass::skipModule(const Module &M) const {
+  const OptPassGate &Gate = M.getContext().getOptPassGate();
   return Gate.isEnabled() &&
          !Gate.shouldRunPass(this->getPassName(), getDescription(M));
 }
diff --git a/llvm/lib/Passes/StandardInstrumentations.cpp b/llvm/lib/Passes/StandardInstrumentations.cpp
index 4e8abc43572d7..0623e66772047 100644
--- a/llvm/lib/Passes/StandardInstrumentations.cpp
+++ b/llvm/lib/Passes/StandardInstrumentations.cpp
@@ -1074,7 +1074,7 @@ bool OptPassGateInstrumentation::shouldRun(StringRef PassName, Any IR) {
 
 void OptPassGateInstrumentation::registerCallbacks(
     PassInstrumentationCallbacks &PIC) {
-  OptPassGate &PassGate = Context.getOptPassGate();
+  const OptPassGate &PassGate = Context.getOptPassGate();
   if (!PassGate.isEnabled())
     return;
 
diff --git a/llvm/unittests/IR/LegacyPassManagerTest.cpp b/llvm/unittests/IR/LegacyPassManagerTest.cpp
index 71241e44831e0..00ceebaff4160 100644
--- a/llvm/unittests/IR/LegacyPassManagerTest.cpp
+++ b/llvm/unittests/IR/LegacyPassManagerTest.cpp
@@ -359,7 +359,8 @@ namespace llvm {
     struct CustomOptPassGate : public OptPassGate {
       bool Skip;
       CustomOptPassGate(bool Skip) : Skip(Skip) { }
-      bool shouldRunPass(const StringRef PassName, StringRef IRDescription) override {
+      bool shouldRunPass(StringRef PassName,
+                         StringRef IRDescription) const override {
         return !Skip;
       }
       bool isEnabled() const override { return true; }

jurahul marked this pull request as ready for review June 28, 2025 00:49

jurahul requested a review from arsenm June 28, 2025 00:49

llvmbot added llvm:ir llvm:analysis Includes value tracking, cost tables and constant folding labels Jun 28, 2025

jurahul requested a review from kazutakahirata June 28, 2025 00:49

arsenm approved these changes Jun 28, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[LLVM] Change `ModulePass::skipModule` to take a const reference #146168

[LLVM] Change `ModulePass::skipModule` to take a const reference #146168

jurahul commented Jun 27, 2025 •

edited

Loading

Uh oh!

llvmbot commented Jun 28, 2025

Uh oh!

llvmbot commented Jun 28, 2025

Uh oh!

Uh oh!

[LLVM] Change ModulePass::skipModule to take a const reference #146168

Are you sure you want to change the base?

[LLVM] Change ModulePass::skipModule to take a const reference #146168

Conversation

jurahul commented Jun 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Jun 28, 2025

Uh oh!

llvmbot commented Jun 28, 2025

Uh oh!

Uh oh!

[LLVM] Change `ModulePass::skipModule` to take a const reference #146168

[LLVM] Change `ModulePass::skipModule` to take a const reference #146168

jurahul commented Jun 27, 2025 •

edited

Loading