Use PassGate from LLVMContext if any otherwise global one

[ebrevnov]

Summary

While specific PassGate can already be associated with LLVMContext it is simply ignored by the callback. As a result global one is always used. So the "meat" of that change is at StandardInstrumentations.cpp:771. All other changes are consequences of this change. In particular:

1. Changed type of the first parameter of 'shouldRunPass' from "Pass *" to "StringRef". The problem is that "Pass" exists for legacy pass manager only. Thus we are unable to call it from new pass manager (via intstrumentation callbacks mechanism).
2. LLVMContext has to be passed to registerCallbacks now
3. OptBisect is just a customization of OptPassGate abstraction. Thus getGlobalPassGate has OptPassGate return type, while still returns an instance OptBisect (which is current default pass gate)

[nhaehnle]

Seems pretty reasonable to me.

[aeubanks]

ideally we wouldn't have to pass LLVMContext if possible, it makes it easier for users of this API to mess up. the optional FAM parameter to StandardInstrumentations already bothers me

I'd like to understand your motivation for this change to see if there's a nicer way of doing this (maybe there isn't and this patch is the way to go). is it to make -opt-bisect-limit work per-LLVMContext? or a custom downstream OptPassGate?

also the Pass* -> StringRef part of this patch looks like it can be split out into a separate patch

[ebrevnov]

In D137149#3903334, @aeubanks wrote:

ideally we wouldn't have to pass LLVMContext if possible, it makes it easier for users of this API to mess up. the optional FAM parameter to StandardInstrumentations already bothers me

I agree. Let's see if we can find something better...

I'd like to understand your motivation for this change to see if there's a nicer way of doing this (maybe there isn't and this patch is the way to go). is it to make -opt-bisect-limit work per-LLVMContext? or a custom downstream OptPassGate?

The motivation is to make -opt-bisect-limit work per-LLVMContext' (as it was supposed to work I guess). In our environment, multiple threads are doing compilations in parallel and we want to be able to bisect only some particular ones.

also the Pass* -> StringRef part of this patch looks like it can be split out into a separate patch

Sure, that can be done if requested

[nhaehnle]

It's sort of strange to have a pass gate reference in the LLVMContext and not use it. But you could just have a StandardInstrumentations::setPassGate method to store a reference locally.

[ebrevnov]

In D137149#3904715, @nhaehnle wrote:

It's sort of strange to have a pass gate reference in the LLVMContext and not use it. But you could just have a StandardInstrumentations::setPassGate method to store a reference locally.

IMHO, we should make StandardInstrumentations context aware... Alternative to StandardInstrumentations::setPassGate could be passing LLVMContext during StandardInstrumentations construction.

[nhaehnle]

In D137149#3904770, @ebrevnov wrote:

In D137149#3904715, @nhaehnle wrote:

It's sort of strange to have a pass gate reference in the LLVMContext and not use it. But you could just have a StandardInstrumentations::setPassGate method to store a reference locally.

IMHO, we should make StandardInstrumentations context aware... Alternative to StandardInstrumentations::setPassGate could be passing LLVMContext during StandardInstrumentations construction.

Yeah, the use cases I'm aware of are pretty much tied to an LLVMContext already anyway, so that seems fine to me too.

[aeubanks]

thanks for the context, agreed that using the LLVMContext's pass gate is what's intended

the cleanest way IMO is to get the IR's LLVMContext (e.g. via unwrapModule(IR, /*Force*/ true).getContext()) and use the pass gate from there, rather than collect the pass gate at the beginning. WDYT of something like https://reviews.llvm.org/D137358?

[ebrevnov]

Integreated "unwrapModule" approach

[ebrevnov]

In D137149#3906302, @aeubanks wrote:

thanks for the context, agreed that using the LLVMContext's pass gate is what's intended

the cleanest way IMO is to get the IR's LLVMContext (e.g. via unwrapModule(IR, /*Force*/ true).getContext()) and use the pass gate from there, rather than collect the pass gate at the beginning. WDYT of something like https://reviews.llvm.org/D137358?

Totally agree. Just didn't realize we already can access the module. Integrated suggested approach except caching part. I think caching is unnecessary complication in this case.

[aeubanks]

In D137149#3907610, @ebrevnov wrote:

In D137149#3906302, @aeubanks wrote:

thanks for the context, agreed that using the LLVMContext's pass gate is what's intended

the cleanest way IMO is to get the IR's LLVMContext (e.g. via unwrapModule(IR, /*Force*/ true).getContext()) and use the pass gate from there, rather than collect the pass gate at the beginning. WDYT of something like https://reviews.llvm.org/D137358?

Totally agree. Just didn't realize we already can access the module. Integrated suggested approach except caching part. I think caching is unnecessary complication in this case.

then you start running into compile time issues: https://llvm-compile-time-tracker.com/compare.php?from=66b830889d9bc4bfc58aa1ea47d1b3a7cbee7c5c&to=b7972184a59c0aa0b634de653be1c1363e980f4c&stat=instructions:u
previously we didn't call OptPassGateInstrumentation::registerCallbacks and run these callbacks before every pass if -opt-bisect-limit was enabled, with this patch we do. caching mitigates a lot of that. but even better is if you can somehow go back to not calling OptPassGateInstrumentation::registerCallbacks when we're not using opt-bisect. With your per-LLVMContext opt-bisect, do you still respect -opt-bisect-limit, or are you manually setting pass gates? if you're still using -opt-bisect-limit, we can go back to checking if it's set and skipping OptPassGateInstrumentation::registerCallbacks and we don't have to worry about caching.

[ebrevnov]

In D137149#3908943, @aeubanks wrote:

In D137149#3907610, @ebrevnov wrote:

In D137149#3906302, @aeubanks wrote:

thanks for the context, agreed that using the LLVMContext's pass gate is what's intended

the cleanest way IMO is to get the IR's LLVMContext (e.g. via unwrapModule(IR, /*Force*/ true).getContext()) and use the pass gate from there, rather than collect the pass gate at the beginning. WDYT of something like https://reviews.llvm.org/D137358?

Totally agree. Just didn't realize we already can access the module. Integrated suggested approach except caching part. I think caching is unnecessary complication in this case.

then you start running into compile time issues: https://llvm-compile-time-tracker.com/compare.php?from=66b830889d9bc4bfc58aa1ea47d1b3a7cbee7c5c&to=b7972184a59c0aa0b634de653be1c1363e980f4c&stat=instructions:u
previously we didn't call OptPassGateInstrumentation::registerCallbacks and run these callbacks before every pass if -opt-bisect-limit was enabled, with this patch we do.

That's kind of unexpected (at least for me). Do we known why such lightweight operation noticeably impacts compile time? Is it getting a module from a loop or something else?

caching mitigates a lot of that.

It does but it opens legality concerns. Do we have a guarantee that a module passed to the callback (through IR parameter) hasn't been changed between the invocations? If it can't change then why passing it to each callback invocation (as opposite to setting it once at creation time or later)? Same concerns apply to immutability of OptPassGate itself and it's IsEnabled field.

but even better is if you can somehow go back to not calling OptPassGateInstrumentation::registerCallbacks when we're not using opt-bisect. With your per-LLVMContext opt-bisect, do you still respect -opt-bisect-limit, or > are you manually setting pass gates?

Currently I'm setting it manually/programmatically to LLVMContext which has preference over global on if set.

if you're still using -opt-bisect-limit, we can go back to checking if it's set and skipping OptPassGateInstrumentation::registerCallbacks and we don't have to worry about caching.

In theory, we can introduce another global which will be set to 'true' only if any pass gate in the system is enabled and use that as a filter..... but that looks overcomplication I would really like to avoid.

[aeubanks]

chasing pointers can be expensive, and the current compile time regressions are unacceptable so requesting changes in phabricator

perhaps we can only register the callback if either -opt-bisect-limit is set or if some other new cl::opt (-force-pass-gate-instrumentation?), then you'd specify that option when debugging?

[ebrevnov]

In D137149#3913303, @aeubanks wrote:

chasing pointers can be expensive, and the current compile time regressions are unacceptable so requesting changes in phabricator

perhaps we can only register the callback if either -opt-bisect-limit is set or if some other new cl::opt (-force-pass-gate-instrumentation?), then you'd specify that option when debugging?

That looks a really hacky to me ... I would choose that option as a last resort.
Once realized we can't afford requesting pass gate from the module on each invocation of callback (due to CT) I'm inclined to think that passing LLVMContext (and FAM as well) during construction of StandardInstrumentations is the right way to go. This way we explicitly set expectations that StandardInstrumentations is tied to a single LLVMContext. Are you OK with such approach? If not then I will go with the caching approach (but I think it's less expressive and more complicated).

[ebrevnov]

Gentle ping.

[ebrevnov]

@aeubanks Any comments?

[aeubanks]

In D137149#3914097, @ebrevnov wrote:

In D137149#3913303, @aeubanks wrote:

chasing pointers can be expensive, and the current compile time regressions are unacceptable so requesting changes in phabricator

perhaps we can only register the callback if either -opt-bisect-limit is set or if some other new cl::opt (-force-pass-gate-instrumentation?), then you'd specify that option when debugging?

That looks a really hacky to me ... I would choose that option as a last resort.
Once realized we can't afford requesting pass gate from the module on each invocation of callback (due to CT) I'm inclined to think that passing LLVMContext (and FAM as well) during construction of StandardInstrumentations is the right way to go. This way we explicitly set expectations that StandardInstrumentations is tied to a single LLVMContext. Are you OK with such approach? If not then I will go with the caching approach (but I think it's less expressive and more complicated).

I think I'd stlil slightly prefer caching (could cache the results of PassGate.isEnabled instead), but I think I'd be ok with adding LLVMContext as a parameter. We should add it as a required parameter to the StandardInstrumentations constructor though so people don't forget about it. (we should also move the FAM parameter into the constructor, but that's a separate issue)

[ebrevnov]

Pass LLVMContext to the StandardInstrumentations constructor

[aeubanks]

lg other than one comment + formatting

llvm/include/llvm/Passes/StandardInstrumentations.h

81–82

clang-format?

llvm/lib/Passes/StandardInstrumentations.cpp

772

this should be in OptPassGateInstrumentation::registerCallbacks gating registerShouldRunOptionalPassCallback to avoid an extra callback per pass run

[ebrevnov]

llvm/include/llvm/Passes/StandardInstrumentations.h

81–82

clang-format works this way for me...what problem do you see here?

llvm/lib/Passes/StandardInstrumentations.cpp

772

PassGate can be set/remove/changed to LLVMContext at any time using LLVMContext::setOptPassGate() API. Thus it would be invalid to "cache" the state in the registerCallbacks.

[aeubanks]

llvm/include/llvm/Passes/StandardInstrumentations.h

81–82

there should be a space in Context(Context){};

I was confused why clang-format wasn't adding the space, eventually figured out it's the unnecessary semicolon haha

llvm/lib/Passes/StandardInstrumentations.cpp

772

again, this causes compile time issues: https://llvm-compile-time-tracker.com/compare.php?from=66b830889d9bc4bfc58aa1ea47d1b3a7cbee7c5c&to=c0e4d153eecc11f1bed41f242e72197c0d5c7baa&stat=instructions:u

I don't think it's reasonable to change the gate while passes are running

(also I had to update BackendUtil.cpp in clang to get it building, you can see that in the precommit bots)

[ebrevnov]

llvm/include/llvm/Passes/StandardInstrumentations.h

81–82

Ough... fixed.

llvm/lib/Passes/StandardInstrumentations.cpp

772

I thought getting module from "IR" slows down things.... looks like that's something else.
Ok I've moved isEnabled to registerCallbacks.

I don't think it's reasonable to change the gate while passes are running

I don't think either but current API allows doing that...IMHO, we better change the API to prohibit such scenario.

(also I had to update BackendUtil.cpp in clang to get it building, you can see that in the precommit bots)

I see. Fixed. Thanks.

[ebrevnov]

Updated

[aeubanks]

the precommit bot is still complaining that flang/lib/Frontend/FrontendActions.cpp needs to be updated

lgtm if the precommit bot doesn't show anything related to this patch

[ebrevnov]

Fixed build error in flang/lib/Frontend/FrontendActions.cpp

[vchuravy]

Late to the party here, but passing an LLVMContext seems really strange from a JIT perspective.
We construct a pipeline that may be run over many modules each of them maybe using a different LLVM Context.

[aeubanks]

we don't currently support reusing a pipeline so I'm surprised that you're able to share/reuse pipelines without running into any issues

[pchintalapudi]

In D137149#4653308, @aeubanks wrote:

we don't currently support reusing a pipeline so I'm surprised that you're able to share/reuse pipelines without running into any issues

The way I see it, any pipeline state that can't be reused between runs is encapsulated within either the module or the analysis managers, with the analysis managers containing derived state that's expensive to compute about the module. Under that interpretation, the optimization passes are essentially stateless, so there's no reason they can't be run on multiple IR units (and CGSCC/Function/Loop passes must deal with this fact already since there's possibly more than one present per module). I don't know how much penalty there is for requiring passes to clean up after themselves, but I can't imagine that it's a very high cost.

[vchuravy]

In D137149#4653308, @aeubanks wrote:

we don't currently support reusing a pipeline so I'm surprised that you're able to share/reuse pipelines without running into any issues

In addition to @pchintalapudi's comment. Reuse of pipelines is something that we (JuliaLang) had come to expect from old PM. This is why opt had a -run-twice option to help flush
out bugs that arose out of the idea that passes would only be used once. @loladiro might remember those discussions.

So when we ported Julia to NewPM I didn't think twice and @pchintalapudi implemented our NewPM usage such that only the AnalysisManager
would be created fresh.

[aeubanks]

In D137149#4653381, @vchuravy wrote:

In D137149#4653308, @aeubanks wrote:

we don't currently support reusing a pipeline so I'm surprised that you're able to share/reuse pipelines without running into any issues

In addition to @pchintalapudi's comment. Reuse of pipelines is something that we (JuliaLang) had come to expect from old PM. This is why opt had a -run-twice option to help flush
out bugs that arose out of the idea that passes would only be used once. @loladiro might remember those discussions.

So when we ported Julia to NewPM I didn't think twice and @pchintalapudi implemented our NewPM usage such that only the AnalysisManager
would be created fresh.

Passes can store state that might break between runs (especially module passes since they typically only run once). I'm not saying it's impossible to reuse a pipeline, just that it's not tested, e.g. -run-twice isn't hooked up to the new PM. There was a discussion about this before somewhere, can't remember where. We'd need a lot more testing before we can claim to support reusing pipelines.

[vchuravy]

We'd need a lot more testing before we can claim to support reusing pipelines.

Sure, but moving pass pipeline state into the context makes it much harder to do this kind of testing.

clang/lib/CodeGen/BackendUtil.cpp

void EmitAssemblyHelper::RunOptimizationPipeline(
   CGSCCAnalysisManager CGAM;
   ModuleAnalysisManager MAM;
 
   bool DebugPassStructure = CodeGenOpts.DebugPass == "Structure";
   PassInstrumentationCallbacks PIC;
   PrintPassOptions PrintPassOpts;
   PrintPassOpts.Indent = DebugPassStructure;
   PrintPassOpts.SkipAnalyses = DebugPassStructure;
-  StandardInstrumentations SI(CodeGenOpts.DebugPassManager ||
-                                  DebugPassStructure,
-                              /*VerifyEach*/ false, PrintPassOpts);
+  StandardInstrumentations SI(
+      TheModule->getContext(),
+      (CodeGenOpts.DebugPassManager || DebugPassStructure),
   SI.registerCallbacks(PIC, &FAM);
   PassBuilder PB(TM.get(), PTO, PGOOpt, &PIC);
 
   if (CodeGenOpts.EnableAssignmentTracking) {
     PB.registerPipelineStartEPCallback(
         [&](ModulePassManager &MPM, OptimizationLevel Level) {
           MPM.addPass(AssignmentTrackingPass());
         });

flang/lib/Frontend/FrontendActions.cpp

void CodeGenAction::runOptimizationPipeline(llvm::raw_pwrite_stream &os) {
   llvm::FunctionAnalysisManager fam;
   llvm::CGSCCAnalysisManager cgam;
   llvm::ModuleAnalysisManager mam;
 
   // Create the pass manager builder.
   llvm::PassInstrumentationCallbacks pic;
   llvm::PipelineTuningOptions pto;
   llvm::Optional<llvm::PGOOptions> pgoOpt;
-  llvm::StandardInstrumentations si(opts.DebugPassManager);
+  llvm::StandardInstrumentations si(
+      llvmModule->getContext(), opts.DebugPassManager);
   si.registerCallbacks(pic, &fam);
   llvm::PassBuilder pb(tm.get(), pto, pgoOpt, &pic);
 
   // Attempt to load pass plugins and register their callbacks with PB.
   for (auto &pluginFile : opts.LLVMPassPlugins) {
     auto passPlugin = llvm::PassPlugin::Load(pluginFile);
     if (passPlugin) {
       passPlugin->registerPassBuilderCallbacks(pb);

llvm/include/llvm/IR/OptBisect.h

 /// Extensions to this class implement mechanisms to disable passes and
 /// individual optimizations at compile time.
 class OptPassGate {
 public:
   virtual ~OptPassGate() = default;
 
   /// IRDescription is a textual description of the IR unit the pass is running
   /// over.
-  virtual bool shouldRunPass(const Pass *P, StringRef IRDescription) {
+  virtual bool shouldRunPass(const StringRef PassName,
+                             StringRef IRDescription) {
     return true;
   }
 
   /// isEnabled() should return true before calling shouldRunPass().
   virtual bool isEnabled() const { return false; }
 };
 
 /// This class implements a mechanism to disable passes and individual
public:
   /// through LLVMContext.
   OptBisect() = default;
 
   virtual ~OptBisect() = default;
 
   /// Checks the bisect limit to determine if the specified pass should run.
   ///
   /// This forwards to checkPass().
-  bool shouldRunPass(const Pass *P, StringRef IRDescription) override;
+  bool shouldRunPass(const StringRef PassName,
+                     StringRef IRDescription) override;
 
   /// isEnabled() should return true before calling shouldRunPass().
   bool isEnabled() const override { return BisectLimit != Disabled; }
 
   /// Set the new optimization limit and reset the counter. Passing
   /// OptBisect::Disabled disables the limiting.
   void setLimit(int Limit) {
     BisectLimit = Limit;

 
 private:
   int BisectLimit = Disabled;
   int LastBisectNum = 0;
 };
 
 /// Singleton instance of the OptBisect class, so multiple pass managers don't
 /// need to coordinate their uses of OptBisect.
-OptBisect &getOptBisector();
+OptPassGate &getGlobalPassGate();
 
 } // end namespace llvm
 
 #endif // LLVM_IR_OPTBISECT_H

llvm/include/llvm/Passes/StandardInstrumentations.h

public:
   OptNoneInstrumentation(bool DebugLogging) : DebugLogging(DebugLogging) {}
   void registerCallbacks(PassInstrumentationCallbacks &PIC);
 
 private:
   bool DebugLogging;
   bool shouldRun(StringRef PassID, Any IR);
 };
 
-class OptBisectInstrumentation {
+class OptPassGateInstrumentation {
+  LLVMContext &Context;
   bool HasWrittenIR = false;
-
 public:
-  OptBisectInstrumentation() = default;
+  OptPassGateInstrumentation(LLVMContext &Context) : Context(Context) {}
+  bool shouldRun(StringRef PassName, Any IR);
   void registerCallbacks(PassInstrumentationCallbacks &PIC);
 };
 
 struct PrintPassOptions {
   /// Print adaptors and pass managers.
   bool Verbose = false;
   /// Don't print information for analyses.
   bool SkipAnalyses = false;

 /// This class provides an interface to register all the standard pass
 /// instrumentations and manages their state (if any).
 class StandardInstrumentations {
   PrintIRInstrumentation PrintIR;
   PrintPassInstrumentation PrintPass;
   TimePassesHandler TimePasses;
   TimeProfilingPassesHandler TimeProfilingPasses;
   OptNoneInstrumentation OptNone;
-  OptBisectInstrumentation OptBisect;
+  OptPassGateInstrumentation OptPassGate;
   PreservedCFGCheckerInstrumentation PreservedCFGChecker;
   IRChangedPrinter PrintChangedIR;
   PseudoProbeVerifier PseudoProbeVerification;
   InLineChangePrinter PrintChangedDiff;
   DotCfgChangeReporter WebsiteChangeReporter;
   PrintCrashIRInstrumentation PrintCrashIR;
   VerifyInstrumentation Verify;
 
   bool VerifyEach;
 
 public:
-  StandardInstrumentations(bool DebugLogging, bool VerifyEach = false,
+  StandardInstrumentations(LLVMContext &Context, bool DebugLogging,
+                           bool VerifyEach = false,
                            PrintPassOptions PrintPassOpts = PrintPassOptions());
 
   // Register all the standard instrumentation callbacks. If \p FAM is nullptr
   // then PreservedCFGChecker is not enabled.
   void registerCallbacks(PassInstrumentationCallbacks &PIC,
                          FunctionAnalysisManager *FAM = nullptr);
 
   TimePassesHandler &getTimePasses() { return TimePasses; }

llvm/lib/Analysis/CallGraphSCCPass.cpp

static std::string getDescription(const CallGraphSCC &SCC) {
   }
   Desc += ")";
   return Desc;
 }
 
 bool CallGraphSCCPass::skipSCC(CallGraphSCC &SCC) const {
   OptPassGate &Gate =
       SCC.getCallGraph().getModule().getContext().getOptPassGate();
-  return Gate.isEnabled() && !Gate.shouldRunPass(this, getDescription(SCC));
+  return Gate.isEnabled() &&
+         !Gate.shouldRunPass(this->getPassName(), getDescription(SCC));
 }
 
 char DummyCGSCCPass::ID = 0;
 
 INITIALIZE_PASS(DummyCGSCCPass, "DummyCGSCCPass", "DummyCGSCCPass", false,
                 false)

llvm/lib/Analysis/LoopPass.cpp

 }
 
 bool LoopPass::skipLoop(const Loop *L) const {
   const Function *F = L->getHeader()->getParent();
   if (!F)
     return false;
   // Check the opt bisect limit.
   OptPassGate &Gate = F->getContext().getOptPassGate();
-  if (Gate.isEnabled() && !Gate.shouldRunPass(this, getDescription(*L)))
+  if (Gate.isEnabled() &&
+      !Gate.shouldRunPass(this->getPassName(), getDescription(*L)))
     return true;
   // Check for the OptimizeNone attribute.
   if (F->hasOptNone()) {
     // FIXME: Report this to dbgs() only once per function.
     LLVM_DEBUG(dbgs() << "Skipping pass '" << getPassName() << "' in function "
                       << F->getName() << "\n");
     // FIXME: Delete loop from pass manager's queue?
     return true;

llvm/lib/Analysis/RegionPass.cpp

 
 static std::string getDescription(const Region &R) {
   return "region";
 }
 
 bool RegionPass::skipRegion(Region &R) const {
   Function &F = *R.getEntry()->getParent();
   OptPassGate &Gate = F.getContext().getOptPassGate();
-  if (Gate.isEnabled() && !Gate.shouldRunPass(this, getDescription(R)))
+  if (Gate.isEnabled() &&
+      !Gate.shouldRunPass(this->getPassName(), getDescription(R)))
     return true;
 
   if (F.hasOptNone()) {
     // Report this only once per function.
     if (R.getEntry() == &F.getEntryBlock())
       LLVM_DEBUG(dbgs() << "Skipping pass '" << getPassName()
                         << "' on function " << F.getName() << "\n");
     return true;
   }
   return false;
 }

llvm/lib/IR/LLVMContextImpl.cpp

void LLVMContextImpl::getSyncScopeNames(
   for (const auto &SSE : SSC)
     SSNs[SSE.second] = SSE.first();
 }
 
 /// Gets the OptPassGate for this LLVMContextImpl, which defaults to the
 /// singleton OptBisect if not explicitly set.
 OptPassGate &LLVMContextImpl::getOptPassGate() const {
   if (!OPG)
-    OPG = &getOptBisector();
+    OPG = &getGlobalPassGate();
   return *OPG;
 }
 
 void LLVMContextImpl::setOptPassGate(OptPassGate& OPG) {
   this->OPG = &OPG;
 }
 
 bool LLVMContextImpl::hasOpaquePointersValue() {

llvm/lib/IR/OptBisect.cpp

 #include "llvm/IR/OptBisect.h"
 #include "llvm/Pass.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/raw_ostream.h"
 #include <cassert>
 
 using namespace llvm;
 
+static OptBisect &getOptBisector() {
+  static OptBisect OptBisector;
+  return OptBisector;
+}
+
 static cl::opt<int> OptBisectLimit("opt-bisect-limit", cl::Hidden,
                                    cl::init(OptBisect::Disabled), cl::Optional,
                                    cl::cb<void, int>([](int Limit) {
-                                     llvm::getOptBisector().setLimit(Limit);
+                                     getOptBisector().setLimit(Limit);
                                    }),
                                    cl::desc("Maximum optimization to perform"));
 
 static void printPassMessage(const StringRef &Name, int PassNum,
                              StringRef TargetDesc, bool Running) {
   StringRef Status = Running ? "" : "NOT ";
   errs() << "BISECT: " << Status << "running pass "
          << "(" << PassNum << ") " << Name << " on " << TargetDesc << "\n";
 }
 
-bool OptBisect::shouldRunPass(const Pass *P, StringRef IRDescription) {
-  assert(isEnabled());
-
-  return checkPass(P->getPassName(), IRDescription);
-}
-
-bool OptBisect::checkPass(const StringRef PassName,
-                          const StringRef TargetDesc) {
+bool OptBisect::shouldRunPass(const StringRef PassName,
+                              StringRef IRDescription) {
   assert(isEnabled());
 
   int CurBisectNum = ++LastBisectNum;
   bool ShouldRun = (BisectLimit == -1 || CurBisectNum <= BisectLimit);
-  printPassMessage(PassName, CurBisectNum, TargetDesc, ShouldRun);
+  printPassMessage(PassName, CurBisectNum, IRDescription, ShouldRun);
   return ShouldRun;
 }
 
 const int OptBisect::Disabled;
 
-OptBisect &llvm::getOptBisector() {
-  static OptBisect OptBisector;
-  return OptBisector;
-}
+OptPassGate &llvm::getGlobalPassGate() { return getOptBisector(); }

llvm/lib/IR/Pass.cpp

 }
 
 static std::string getDescription(const Module &M) {
   return "module (" + M.getName().str() + ")";
 }
 
 bool ModulePass::skipModule(Module &M) const {
   OptPassGate &Gate = M.getContext().getOptPassGate();
-  return Gate.isEnabled() && !Gate.shouldRunPass(this, getDescription(M));
+  return Gate.isEnabled() &&
+         !Gate.shouldRunPass(this->getPassName(), getDescription(M));
 }
 
 bool Pass::mustPreserveAnalysisID(char &AID) const {
   return Resolver->getAnalysisIfAvailable(&AID) != nullptr;
 }
 
 // dumpPassStructure - Implement the -debug-pass=Structure option
 void Pass::dumpPassStructure(unsigned Offset) {

 }
 
 static std::string getDescription(const Function &F) {
   return "function (" + F.getName().str() + ")";
 }
 
 bool FunctionPass::skipFunction(const Function &F) const {
   OptPassGate &Gate = F.getContext().getOptPassGate();
-  if (Gate.isEnabled() && !Gate.shouldRunPass(this, getDescription(F)))
+  if (Gate.isEnabled() &&
+      !Gate.shouldRunPass(this->getPassName(), getDescription(F)))
     return true;
 
   if (F.hasOptNone()) {
     LLVM_DEBUG(dbgs() << "Skipping pass '" << getPassName() << "' on function "
                       << F.getName() << "\n");
     return true;
   }
   return false;

llvm/lib/LTO/LTOBackend.cpp

static void runNewPMPasses(const Config &Conf, Module &Mod, TargetMachine *TM,
   TM->setPGOOption(PGOOpt);
 
   LoopAnalysisManager LAM;
   FunctionAnalysisManager FAM;
   CGSCCAnalysisManager CGAM;
   ModuleAnalysisManager MAM;
 
   PassInstrumentationCallbacks PIC;
-  StandardInstrumentations SI(Conf.DebugPassManager);
+  StandardInstrumentations SI(Mod.getContext(), Conf.DebugPassManager);
   SI.registerCallbacks(PIC, &FAM);
   PassBuilder PB(TM, Conf.PTO, PGOOpt, &PIC);
 
   RegisterPassPlugins(Conf.PassPlugins, PB);
 
   std::unique_ptr<TargetLibraryInfoImpl> TLII(
       new TargetLibraryInfoImpl(Triple(TM->getTargetTriple())));
   if (Conf.Freestanding)

llvm/lib/LTO/ThinLTOCodeGenerator.cpp

static void optimizeModule(Module &TheModule, TargetMachine &TM,
                            bool DebugPassManager, ModuleSummaryIndex *Index) {
   Optional<PGOOptions> PGOOpt;
   LoopAnalysisManager LAM;
   FunctionAnalysisManager FAM;
   CGSCCAnalysisManager CGAM;
   ModuleAnalysisManager MAM;
 
   PassInstrumentationCallbacks PIC;
-  StandardInstrumentations SI(DebugPassManager);
+  StandardInstrumentations SI(TheModule.getContext(), DebugPassManager);
   SI.registerCallbacks(PIC, &FAM);
   PipelineTuningOptions PTO;
   PTO.LoopVectorization = true;
   PTO.SLPVectorization = true;
   PassBuilder PB(&TM, PTO, PGOOpt, &PIC);
 
   std::unique_ptr<TargetLibraryInfoImpl> TLII(
       new TargetLibraryInfoImpl(Triple(TM.getTargetTriple())));

llvm/lib/Passes/PassBuilderBindings.cpp

LLVMErrorRef LLVMRunPasses(LLVMModuleRef M, const char *Passes,
   CGSCCAnalysisManager CGAM;
   ModuleAnalysisManager MAM;
   PB.registerLoopAnalyses(LAM);
   PB.registerFunctionAnalyses(FAM);
   PB.registerCGSCCAnalyses(CGAM);
   PB.registerModuleAnalyses(MAM);
   PB.crossRegisterProxies(LAM, FAM, CGAM, MAM);
 
-  StandardInstrumentations SI(Debug, VerifyEach);
+  StandardInstrumentations SI(Mod->getContext(), Debug, VerifyEach);
   SI.registerCallbacks(PIC, &FAM);
   ModulePassManager MPM;
   if (VerifyEach) {
     MPM.addPass(VerifierPass());
   }
   if (auto Err = PB.parsePassPipeline(MPM, Passes)) {
     return wrap(std::move(Err));
   }

llvm/lib/Passes/StandardInstrumentations.cpp

bool OptNoneInstrumentation::shouldRun(StringRef PassID, Any IR) {
   bool ShouldRun = !(F && F->hasOptNone());
   if (!ShouldRun && DebugLogging) {
     errs() << "Skipping pass " << PassID << " on " << F->getName()
            << " due to optnone attribute\n";
   }
   return ShouldRun;
 }
 
-void OptBisectInstrumentation::registerCallbacks(
-    PassInstrumentationCallbacks &PIC) {
-  if (!getOptBisector().isEnabled())
-    return;
-  PIC.registerShouldRunOptionalPassCallback([this](StringRef PassID, Any IR) {
-    if (isIgnored(PassID))
-      return true;
-    bool ShouldRun = getOptBisector().checkPass(PassID, getIRName(IR));
-    if (!ShouldRun && !this->HasWrittenIR && !OptBisectPrintIRPath.empty()) {
-      // FIXME: print IR if limit is higher than number of opt-bisect
-      // invocations
-      this->HasWrittenIR = true;
-      const Module *M = unwrapModule(IR, /*Force=*/true);
-      assert(M && "expected Module");
-      std::error_code EC;
-      raw_fd_ostream OS(OptBisectPrintIRPath, EC);
-      if (EC)
-        report_fatal_error(errorCodeToError(EC));
-      M->print(OS, nullptr);
-    }
-    return ShouldRun;
+bool OptPassGateInstrumentation::shouldRun(StringRef PassName, Any IR) {
+  if (isIgnored(PassName))
+
+  bool ShouldRun =
+      Context.getOptPassGate().shouldRunPass(PassName, getIRName(IR));
+    assert((M && &M->getContext() == &Context) && "Missing/Mismatching Module");
+}
+
+void OptPassGateInstrumentation::registerCallbacks(
+    PassInstrumentationCallbacks &PIC) {
+  OptPassGate &PassGate = Context.getOptPassGate();
+  if (!PassGate.isEnabled())
+    return;
+
+  PIC.registerShouldRunOptionalPassCallback([this](StringRef PassName, Any IR) {
+    return this->shouldRun(PassName, IR);
   });
 }
 
 raw_ostream &PrintPassInstrumentation::print() {
   if (Opts.Indent) {
     assert(Indent >= 0);
     dbgs().indent(Indent);
   }
if (initializeHTML()) {
       ChangeReporter<IRDataT<DCData>>::registerRequiredCallbacks(PIC);
       return;
     }
     dbgs() << "Unable to open output stream for -cfg-dot-changed\n";
   }
 }
 
 StandardInstrumentations::StandardInstrumentations(
-    bool DebugLogging, bool VerifyEach, PrintPassOptions PrintPassOpts)
-    : PrintPass(DebugLogging, PrintPassOpts), OptNone(DebugLogging),
+    LLVMContext &Context, bool DebugLogging, bool VerifyEach,
+    PrintPassOptions PrintPassOpts)
+    : PrintPass(DebugLogging, PrintPassOpts),
+      OptNone(DebugLogging),
+      OptPassGate(Context),
       PrintChangedIR(PrintChanged == ChangePrinter::Verbose),
       PrintChangedDiff(PrintChanged == ChangePrinter::DiffVerbose ||
                            PrintChanged == ChangePrinter::ColourDiffVerbose,
                        PrintChanged == ChangePrinter::ColourDiffVerbose ||
                            PrintChanged == ChangePrinter::ColourDiffQuiet),
       WebsiteChangeReporter(PrintChanged == ChangePrinter::DotCfgVerbose),
       Verify(DebugLogging), VerifyEach(VerifyEach) {}
 

 }
 
 void StandardInstrumentations::registerCallbacks(
     PassInstrumentationCallbacks &PIC, FunctionAnalysisManager *FAM) {
   PrintIR.registerCallbacks(PIC);
   PrintPass.registerCallbacks(PIC);
   TimePasses.registerCallbacks(PIC);
   OptNone.registerCallbacks(PIC);
-  OptBisect.registerCallbacks(PIC);
+  OptPassGate.registerCallbacks(PIC);
   if (FAM)
     PreservedCFGChecker.registerCallbacks(PIC, *FAM);
   PrintChangedIR.registerCallbacks(PIC);
   PseudoProbeVerification.registerCallbacks(PIC);
   if (VerifyEach)
     Verify.registerCallbacks(PIC);
   PrintChangedDiff.registerCallbacks(PIC);
   WebsiteChangeReporter.registerCallbacks(PIC);

llvm/tools/opt/NewPMDriver.cpp

bool llvm::runPassPipeline(StringRef Arg0, Module &M, TargetMachine *TM,
   FunctionAnalysisManager FAM;
   CGSCCAnalysisManager CGAM;
   ModuleAnalysisManager MAM;
 
   PassInstrumentationCallbacks PIC;
   PrintPassOptions PrintPassOpts;
   PrintPassOpts.Verbose = DebugPM == DebugLogging::Verbose;
   PrintPassOpts.SkipAnalyses = DebugPM == DebugLogging::Quiet;
-  StandardInstrumentations SI(DebugPM != DebugLogging::None, VerifyEachPass,
-                              PrintPassOpts);
+  StandardInstrumentations SI(M.getContext(), DebugPM != DebugLogging::None,
+                              VerifyEachPass, PrintPassOpts);
   SI.registerCallbacks(PIC, &FAM);
   DebugifyEachInstrumentation Debugify;
   DebugifyStatsMap DIStatsMap;
   DebugInfoPerPass DebugInfoBeforePass;
   if (DebugifyEach) {
     Debugify.setDIStatsMap(DIStatsMap);
     Debugify.setDebugifyMode(DebugifyMode::SyntheticDebugInfo);
     Debugify.registerCallbacks(PIC);

llvm/unittests/IR/LegacyPassManagerTest.cpp

TEST(PassManager, MemoryOnTheFly) {
       }
       delete M;
     }
 
     // Skips or runs optional passes.
     struct CustomOptPassGate : public OptPassGate {
       bool Skip;
       CustomOptPassGate(bool Skip) : Skip(Skip) { }
-      bool shouldRunPass(const Pass *P, StringRef IRDescription) override {
-        if (P->getPassKind() == PT_Module)
-          return !Skip;
-        return OptPassGate::shouldRunPass(P, IRDescription);
+      bool shouldRunPass(const StringRef PassName, StringRef IRDescription) override {
       }
       bool isEnabled() const override { return true; }
     };
 
     // Optional module pass.
     struct ModuleOpt: public ModulePass {
       char run = 0;
       static char ID;

llvm/unittests/IR/PassManagerTest.cpp

auto M = parseIR(Context, "define void @foo() {\n"
                             "exit:\n"
                             "  ret void\n"
                             "}\n");
 
   auto *F = M->getFunction("foo");
   FunctionAnalysisManager FAM;
   FunctionPassManager FPM;
   PassInstrumentationCallbacks PIC;
-  StandardInstrumentations SI(/*DebugLogging*/ true);
+  StandardInstrumentations SI(M->getContext(), /*DebugLogging*/ true);
   SI.registerCallbacks(PIC, &FAM);
   FAM.registerPass([&] { return PassInstrumentationAnalysis(&PIC); });
   FAM.registerPass([&] { return DominatorTreeAnalysis(); });
   FAM.registerPass([&] { return AssumptionAnalysis(); });
   FAM.registerPass([&] { return TargetIRAnalysis(); });
 
   FPM.addPass(SimplifyCFGPass());
   FPM.run(*F, FAM);
auto M = parseIR(Context, "define void @foo() {\n"
                             "exit:\n"
                             "  ret void\n"
                             "}\n");
 
   auto *F = M->getFunction("foo");
   FunctionAnalysisManager FAM;
   FunctionPassManager FPM;
   PassInstrumentationCallbacks PIC;
-  StandardInstrumentations SI(/*DebugLogging*/ true);
+  StandardInstrumentations SI(M->getContext(), /*DebugLogging*/ true);
   SI.registerCallbacks(PIC, &FAM);
   FAM.registerPass([&] { return PassInstrumentationAnalysis(&PIC); });
   FAM.registerPass([&] { return DominatorTreeAnalysis(); });
   FAM.registerPass([&] { return AssumptionAnalysis(); });
   FAM.registerPass([&] { return TargetIRAnalysis(); });
 
   FPM.addPass(TestSimplifyCFGInvalidatingAnalysisPass());
   FPM.run(*F, FAM);
auto M = parseIR(Context, "define void @foo() {\n"
                             "exit:\n"
                             "  ret void\n"
                             "}\n");
 
   auto *F = M->getFunction("foo");
   FunctionAnalysisManager FAM;
   FunctionPassManager FPM;
   PassInstrumentationCallbacks PIC;
-  StandardInstrumentations SI(/*DebugLogging*/ true);
+  StandardInstrumentations SI(M->getContext(), /*DebugLogging*/ true);
   SI.registerCallbacks(PIC, &FAM);
   FAM.registerPass([&] { return PassInstrumentationAnalysis(&PIC); });
   FAM.registerPass([&] { return DominatorTreeAnalysis(); });
   FAM.registerPass([&] { return AssumptionAnalysis(); });
   FAM.registerPass([&] { return TargetIRAnalysis(); });
 
   FunctionPassManager InnerFPM;
   InnerFPM.addPass(SimplifyCFGPass());
 
   FPM.addPass(TestSimplifyCFGWrapperPass(InnerFPM));
   FPM.run(*F, FAM);
 }
 }