Fix IR/Analysis layering issue in OptBisect

[rtrieu]

Summary

OptBisect, a class in IR, depends on classes in Analysis. Analysis depends on IR, which creates a dependency loop. This patch attempts a minimal change to fix the issue. OptPassGate will have an isEnabled function and shouldRunPass only called if true is returned. shouldRunPass is simplified down to one function instead of several overloaded ones.

[fedor.sergeev]

The need for generic way of getting textual description for IRUnit is obvious both in legacy and new pass manager.
However in this particular approach I dont like how "const void*" is used to erase specific type of IRUnit.

In new pass manager there was a similar task of erasing the type of IRUnit for pass-instrumentation purposes.
And there we decided to wrap the pointer into llvm::Any.

I'm not sure llvm::Any is the best approach here, but it sounds better than absolute anarchy of const void*.

[dblaikie]

In D58406#1404169, @fedor.sergeev wrote:

The need for generic way of getting textual description for IRUnit is obvious both in legacy and new pass manager.
However in this particular approach I dont like how "const void*" is used to erase specific type of IRUnit.

In new pass manager there was a similar task of erasing the type of IRUnit for pass-instrumentation purposes.
And there we decided to wrap the pointer into llvm::Any.

I'm not sure llvm::Any is the best approach here, but it sounds better than absolute anarchy of const void*.

Fair.

Alternatively - how would you feel about callers passing in the strings directly?

return !BisectEnabled || checkPass(P->getPassName(), P->getDescription(U));

The Pass and Unit are only used to retrieve the Pass Name and Description (passing the Unit straight back to the Pass that just passed it in) - so why not pass in those (Pass Name and Description) instead?

I guess the way it is now, those two strings are not computed if Bisect is disabled, so there's some efficiency gain - any sense whether that's significant/important?

[fedor.sergeev]

In D58406#1404179, @dblaikie wrote:

Alternatively - how would you feel about callers passing in the strings directly?

return !BisectEnabled || checkPass(P->getPassName(), P->getDescription(U));

The Pass and Unit are only used to retrieve the Pass Name and Description (passing the Unit straight back to the Pass that just passed it in) - so why not pass in those (Pass Name and Description) instead?

You mean e.g. changing

!F.getContext().getOptPassGate().shouldRunPass(this, &F))

into

!F.getContext().getOptPassGate().shouldRunPass(this->getName(), F.getDescription()))

Yep, that would resolve the layering issue w/o requiring opaque pointers.

Btw, in new-pm's pass instrumentation we do pass Pass Name instead of a pointer to pass exactly because of this layering issue.
However, note, that there we still pass IR unit itself (wrapped into llvm::Any) instead of "description"
because instrumentations framework is generic and instrumentation functionality is not predefined.
And yet, specific instrumentations like PrintIR do need IRUnit's description and right now every instrumentation does it on its own.

If we could implement a generic way of querying IRUnit description (w/o introducing a layering issue), it would
be beneficial both for legacy and new pm implementations.

I guess the way it is now, those two strings are not computed if Bisect is disabled, so there's some efficiency gain - any sense whether that's significant/important?

I dont believe once-per-pass-invocation name query is anything essential we need to worry about.

[dblaikie]

In D58406#1404198, @fedor.sergeev wrote:

In D58406#1404179, @dblaikie wrote:

Alternatively - how would you feel about callers passing in the strings directly?

return !BisectEnabled || checkPass(P->getPassName(), P->getDescription(U));

The Pass and Unit are only used to retrieve the Pass Name and Description (passing the Unit straight back to the Pass that just passed it in) - so why not pass in those (Pass Name and Description) instead?

You mean e.g. changing

!F.getContext().getOptPassGate().shouldRunPass(this, &F))

into

!F.getContext().getOptPassGate().shouldRunPass(this->getName(), F.getDescription()))

Yep, that would resolve the layering issue w/o requiring opaque pointers.

Btw, in new-pm's pass instrumentation we do pass Pass Name instead of a pointer to pass exactly because of this layering issue.
However, note, that there we still pass IR unit itself (wrapped into llvm::Any) instead of "description"
because instrumentations framework is generic and instrumentation functionality is not predefined.
And yet, specific instrumentations like PrintIR do need IRUnit's description and right now every instrumentation does it on its own.

If we could implement a generic way of querying IRUnit description (w/o introducing a layering issue), it would
be beneficial both for legacy and new pm implementations.

OK - so sounds like you have a slight preference for keeping the description callback (though that requires also passing in the pass, not just the pass name) & using llvm::Any?

Or keeping the description API (maybe removing its virtuality until that's needed in the new PM version?) but having passes call it locally themselves, rather than being called back from OptBisect?

[philip.pfaffe]

Keep in mind that for the new PM we're talking about an API that's much more powerful, because it's designed for arbitrary hooks.

OptBisect doesn't need that necessarily. I'm in favor of passing the strings instead.

[fedor.sergeev]

In D58406#1404292, @philip.pfaffe wrote:

Keep in mind that for the new PM we're talking about an API that's much more powerful, because it's designed for arbitrary hooks.
OptBisect doesn't need that necessarily. I'm in favor of passing the strings instead.

Yep, thats true.
Was just pointing out that new PM might benefit from generalization, if anybody decides to do it ;)

I'm fine with passing the strings.

[rtrieu]

Last time, this was brought up, passing strings was deemed too expensive since it would cause a string to be created every time:
http://lists.llvm.org/pipermail/llvm-dev/2018-March/122032.html

There's a difference between getting the pass name and the IR unit name. Pass names are hard-coded string literals which can be cheaply referred to by a StringRef. IR units need to create a new string with the description (except Loop and Region, which don't have interesting descriptions yet.)

[rtrieu]

Change void pointers to llvm::Any. Update some functions to fix const issues.

[dblaikie]

In D58406#1405264, @rtrieu wrote:

Last time, this was brought up, passing strings was deemed too expensive since it would cause a string to be created every time:
http://lists.llvm.org/pipermail/llvm-dev/2018-March/122032.html

There's a difference between getting the pass name and the IR unit name. Pass names are hard-coded string literals which can be cheaply referred to by a StringRef. IR units need to create a new string with the description (except Loop and Region, which don't have interesting descriptions yet.)

Fair enough - thanks for the reference/thread!

Makes sense to me (& yeah, doing this for every Function, and the complicated CGSCC description, etc - at least for Function/Module/BasicBlock passing as a Twine might suffice to delay some/enough of the computation, but CGSCC is a bit too verbose for that).

The only other option would be passing a functor (either std::function, or making "shouldRunPass" a function template) which would avoid the need for the virtuality, Any usage, etc. *shrug*

I don't feel too strongly from the available options - but hope someone with a closer connection/appreciation for these classes might have an opinion about whether adding the extra virtual function to Pass is problematic (philosophically or practically) or not.

[andrew.w.kaylor]

For the record, I'm not sure about the previous decision that passing strings is too expensive. On the one hand, bisection is almost always disabled and is disabled in 100% of cases where anyone cares about compile time (though that may not be true of all opt gate use cases). On the other hand, even for a relatively large file with many functions and loops this is going to be on the order of 200,000 calls to this interface. Sure, building 200,000 strings isn't cheap, but in a compilation that's running 200,000 optimization passes it's minuscule.

I'm OK with either solution, and the latest revision looks good to me. I just don't think we need to go to any great lengths just to avoid building a string before running a pass. If we're really concerned about the string cost, an OptPassGate::isEnabled() call would likely accomplish the same thing.

[fedor.sergeev]

In D58406#1405975, @dblaikie wrote:

I don't feel too strongly from the available options - but hope someone with a closer connection/appreciation for these classes might have an opinion about whether adding the extra virtual function to Pass is problematic (philosophically or practically) or not.

And here will I chime yet again with connection to NewPM (where, say, Passes do not have virtual functions).

... Looking at how this starts using Any wrapper I feel more and more motivated to create an IRUnit set of interfaces
that provide a generic handling for wrapping/unwrapping/description generation etc of IRUnits.
Doing it via virtual functions of Passes seems to be inelegant at best.

[rtrieu]

Removed the virtual function from Pass. Use OptPassGate::isEnabled to gate the construction of the strings on the caller side.

[fedor.sergeev]

I do like the final result!
See my comment, but its good to go even w/o my suggestion.

include/llvm/IR/OptBisect.h

29 ↗

(On Diff #188499)

So, since we went over changing the interface to use StringRef for description, can we move one more step further
in that direction and start passing StringRef for Pass?
That would make this interface fully ready for new pass manager.

I can do it later on myself, but doing it here would lessen the churn by doing one change in interface, not two of them.

(it will not add any more overhead since pass name is always available as a string literal)

[philip.pfaffe]

include/llvm/IR/OptBisect.h

29 ↗

(On Diff #188499)

I concur with @andrew.w.kaylor that the performance hit from building the IRDescription will probably be fine. Nevertheless, it should be much cheaper to pass a Twine here instead! Additionally there should be a comment here explaining IRDescription.

[dblaikie]

include/llvm/IR/OptBisect.h

29 ↗

(On Diff #188499)

I'd probably still suggest doing it as two changes - independent changes in independent commits and all that. But I don't feel especially strongly about it/this isn't code I have much ownership of/investment in.

As for Twine - not sure that would be relevant here. The most expensive ones don't benefit from Twine anyway (only those where "getDescription" could be removed, and the implementation written inline in the call to shouldRunPass could be Twine-able) and now that the condition where the string isn't needed is hoisted out before the call - once we're at the point of producing the description we know we're going to need it in a string, so there's nothing to be gained by delaying through a Twine, I think?

[rtrieu]

Sounds like this is good with a comment on the IRDescription. I'll commit this for now and leave the suggestions for follow-ups. There's two follow-ups if any is interested in perusing:

1. Change shouldRunPass to take a StringRef instead of Pass pointer.

@fedor.sergeev says this will work better with the new PM, however this will also take away the ability of OptPassGate to use information in the Pass to decide to run the pass. Notably, LegacyPassManagerTest.cpp uses it to only run on Module IR units.

2. Change IRDescription from StringRef to Twine.

This looks possible for all the getDescription functions and may reduce a bit of time from skipping string construction. All the strings returned from getDescription are created from a mix of string literals and StringRef's, which should handle. Then shouldRunPass and checkPass would also need to have their second parameters changed to Twine. Since we are already gating the string construction on isEnabled, this will provide benefit only in a few cases.

llvm/trunk/include/llvm/IR/OptBisect.h

 #ifndef LLVM_IR_OPTBISECT_H
 #define LLVM_IR_OPTBISECT_H
 
 #include "llvm/ADT/StringRef.h"
 
 namespace llvm {
 
 class Pass;
-class Module;
-class Function;
-class BasicBlock;
-class Region;
-class Loop;
-class CallGraphSCC;
 
 /// Extensions to this class implement mechanisms to disable passes and
 /// individual optimizations at compile time.
 class OptPassGate {
 public:
   virtual ~OptPassGate() = default;
 
-  virtual bool shouldRunPass(const Pass *P, const Module &U) { return true; }
-  virtual bool shouldRunPass(const Pass *P, const Function &U)  {return true; }
-  virtual bool shouldRunPass(const Pass *P, const BasicBlock &U)  { return true; }
-  virtual bool shouldRunPass(const Pass *P, const Region &U)  { return true; }
-  virtual bool shouldRunPass(const Pass *P, const Loop &U)  { return true; }
-  virtual bool shouldRunPass(const Pass *P, const CallGraphSCC &U)  { return true; }
+  /// IRDescription is a textual description of the IR unit the pass is running
+  /// over.
+  virtual bool shouldRunPass(const Pass *P, 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
 /// optimizations at compile time based on a command line option
 /// (-opt-bisect-limit) in order to perform a bisecting search for
 /// optimization-related problems.
 class OptBisect : public OptPassGate {
 public:
   /// Default constructor, initializes the OptBisect state based on the
   /// -opt-bisect-limit command line argument.
   ///
   /// By default, bisection is disabled.
   ///
   /// Clients should not instantiate this class directly.  All access should go
   /// through LLVMContext.
   OptBisect();
 
   virtual ~OptBisect() = default;
 
   /// Checks the bisect limit to determine if the specified pass should run.
   ///
-  /// These functions immediately return true if bisection is disabled. If the
-  /// bisect limit is set to -1, the functions print a message describing
+  /// If the bisect limit is set to -1, the function prints a message describing
   /// the pass and the bisect number assigned to it and return true.  Otherwise,
-  /// the functions print a message with the bisect number assigned to the
+  /// the function prints a message with the bisect number assigned to the
   /// pass and indicating whether or not the pass will be run and return true if
   /// the bisect limit has not yet been exceeded or false if it has.
   ///
-  /// Most passes should not call these routines directly. Instead, they are
+  /// Most passes should not call this routine directly. Instead, they are
   /// called through helper routines provided by the pass base classes.  For
   /// instance, function passes should call FunctionPass::skipFunction().
-  bool shouldRunPass(const Pass *P, const Module &U) override;
-  bool shouldRunPass(const Pass *P, const Function &U) override;
-  bool shouldRunPass(const Pass *P, const BasicBlock &U) override;
-  bool shouldRunPass(const Pass *P, const Region &U) override;
-  bool shouldRunPass(const Pass *P, const Loop &U) override;
-  bool shouldRunPass(const Pass *P, const CallGraphSCC &U) override;
+  bool shouldRunPass(const Pass *P, StringRef IRDescription) override;
 
+  /// isEnabled should return true before calling shouldRunPass
+  bool isEnabled() const override { return BisectEnabled; }
 private:
   bool checkPass(const StringRef PassName, const StringRef TargetDesc);
 
   bool BisectEnabled = false;
   unsigned LastBisectNum = 0;
 };
 
 } // end namespace llvm
 
 #endif // LLVM_IR_OPTBISECT_H

llvm/trunk/lib/Analysis/CallGraphSCCPass.cpp

 
 char PrintCallGraphPass::ID = 0;
 
 Pass *CallGraphSCCPass::createPrinterPass(raw_ostream &OS,
                                           const std::string &Banner) const {
   return new PrintCallGraphPass(Banner, OS);
 }
 
+static std::string getDescription(const CallGraphSCC &SCC) {
+  std::string Desc = "SCC (";
+  bool First = true;
+  for (CallGraphNode *CGN : SCC) {
+    if (First)
+      First = false;
+    else
+      Desc += ", ";
+    Function *F = CGN->getFunction();
+    if (F)
+      Desc += F->getName();
+    else
+      Desc += "<<null function>>";
+  }
+  Desc += ")";
+  return Desc;
+}
+
 bool CallGraphSCCPass::skipSCC(CallGraphSCC &SCC) const {
-  return !SCC.getCallGraph().getModule()
-              .getContext()
-              .getOptPassGate()
-              .shouldRunPass(this, SCC);
+  OptPassGate &Gate =
+      SCC.getCallGraph().getModule().getContext().getOptPassGate();
+  return Gate.isEnabled() && !Gate.shouldRunPass(this, getDescription(SCC));
 }
 
 char DummyCGSCCPass::ID = 0;
 
 INITIALIZE_PASS(DummyCGSCCPass, "DummyCGSCCPass", "DummyCGSCCPass", false,
                 false)

llvm/trunk/lib/Analysis/LoopPass.cpp

else {
 
     // [4] Push new manager into PMS
     PMS.push(LPPM);
   }
 
   LPPM->add(this);
 }
 
+static std::string getDescription(const Loop &L) {
+  return "loop";
+}
+
 bool LoopPass::skipLoop(const Loop *L) const {
   const Function *F = L->getHeader()->getParent();
   if (!F)
     return false;
   // Check the opt bisect limit.
-  LLVMContext &Context = F->getContext();
-  if (!Context.getOptPassGate().shouldRunPass(this, *L))
+  OptPassGate &Gate = F->getContext().getOptPassGate();
+  if (Gate.isEnabled() && !Gate.shouldRunPass(this, getDescription(*L)))
     return true;
   // Check for the OptimizeNone attribute.
   if (F->hasFnAttribute(Attribute::OptimizeNone)) {
     // 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;
   }
   return false;
 }
 
 char LCSSAVerificationPass::ID = 0;
 INITIALIZE_PASS(LCSSAVerificationPass, "lcssa-verification", "LCSSA Verifier",
                 false, false)

llvm/trunk/lib/Analysis/RegionPass.cpp

 }
 
 /// Get the printer pass
 Pass *RegionPass::createPrinterPass(raw_ostream &O,
                                   const std::string &Banner) const {
   return new PrintRegionPass(Banner, O);
 }
 
+static std::string getDescription(const Region &R) {
+  return "region";
+}
+
 bool RegionPass::skipRegion(Region &R) const {
   Function &F = *R.getEntry()->getParent();
-  if (!F.getContext().getOptPassGate().shouldRunPass(this, R))
+  OptPassGate &Gate = F.getContext().getOptPassGate();
+  if (Gate.isEnabled() && Gate.shouldRunPass(this, getDescription(R)))
     return true;
 
   if (F.hasFnAttribute(Attribute::OptimizeNone)) {
     // 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/trunk/lib/IR/OptBisect.cpp

 //===- llvm/IR/OptBisect/Bisect.cpp - LLVM Bisect support -----------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 /// \file
 /// This file implements support for a bisecting optimizations based on a
 /// command line option.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/IR/OptBisect.h"
 #include "llvm/ADT/StringRef.h"
-#include "llvm/Analysis/CallGraph.h"
-#include "llvm/Analysis/CallGraphSCCPass.h"
-#include "llvm/Analysis/LoopInfo.h"
-#include "llvm/Analysis/RegionInfo.h"
-#include "llvm/IR/BasicBlock.h"
-#include "llvm/IR/Function.h"
-#include "llvm/IR/Module.h"
 #include "llvm/Pass.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/raw_ostream.h"
 #include <cassert>
 #include <limits>
 #include <string>
 
 using namespace llvm;

 
 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";
 }
 
-static std::string getDescription(const Module &M) {
-  return "module (" + M.getName().str() + ")";
-}
-
-static std::string getDescription(const Function &F) {
-  return "function (" + F.getName().str() + ")";
-}
-
-static std::string getDescription(const BasicBlock &BB) {
-  return "basic block (" + BB.getName().str() + ") in function (" +
-         BB.getParent()->getName().str() + ")";
-}
-
-static std::string getDescription(const Loop &L) {
-  // FIXME: Move into LoopInfo so we can get a better description
-  // (and avoid a circular dependency between IR and Analysis).
-  return "loop";
-}
-
-static std::string getDescription(const Region &R) {
-  // FIXME: Move into RegionInfo so we can get a better description
-  // (and avoid a circular dependency between IR and Analysis).
-  return "region";
-}
-
-static std::string getDescription(const CallGraphSCC &SCC) {
-  // FIXME: Move into CallGraphSCCPass to avoid circular dependency between
-  // IR and Analysis.
-  std::string Desc = "SCC (";
-  bool First = true;
-  for (CallGraphNode *CGN : SCC) {
-    if (First)
-      First = false;
-    else
-      Desc += ", ";
-    Function *F = CGN->getFunction();
-    if (F)
-      Desc += F->getName();
-    else
-      Desc += "<<null function>>";
-  }
-  Desc += ")";
-  return Desc;
-}
-
-bool OptBisect::shouldRunPass(const Pass *P, const Module &U) {
-  return !BisectEnabled || checkPass(P->getPassName(), getDescription(U));
-}
-
-bool OptBisect::shouldRunPass(const Pass *P, const Function &U) {
-  return !BisectEnabled || checkPass(P->getPassName(), getDescription(U));
-}
-
-bool OptBisect::shouldRunPass(const Pass *P, const BasicBlock &U) {
-  return !BisectEnabled || checkPass(P->getPassName(), getDescription(U));
-}
-
-bool OptBisect::shouldRunPass(const Pass *P, const Region &U) {
-  return !BisectEnabled || checkPass(P->getPassName(), getDescription(U));
-}
-
-bool OptBisect::shouldRunPass(const Pass *P, const Loop &U) {
-  return !BisectEnabled || checkPass(P->getPassName(), getDescription(U));
-}
+bool OptBisect::shouldRunPass(const Pass *P, StringRef IRDescription) {
+  assert(BisectEnabled);
 
-bool OptBisect::shouldRunPass(const Pass *P, const CallGraphSCC &U) {
-  return !BisectEnabled || checkPass(P->getPassName(), getDescription(U));
+  return checkPass(P->getPassName(), IRDescription);
 }
 
 bool OptBisect::checkPass(const StringRef PassName,
                           const StringRef TargetDesc) {
   assert(BisectEnabled);
 
   int CurBisectNum = ++LastBisectNum;
   bool ShouldRun = (OptBisectLimit == -1 || CurBisectNum <= OptBisectLimit);
   printPassMessage(PassName, CurBisectNum, TargetDesc, ShouldRun);
   return ShouldRun;
 }

llvm/trunk/lib/IR/Pass.cpp

Pass *ModulePass::createPrinterPass(raw_ostream &OS,
                                     const std::string &Banner) const {
   return createPrintModulePass(OS, Banner);
 }
 
 PassManagerType ModulePass::getPotentialPassManagerType() const {
   return PMT_ModulePassManager;
 }
 
+static std::string getDescription(const Module &M) {
+  return "module (" + M.getName().str() + ")";
+}
+
 bool ModulePass::skipModule(Module &M) const {
-  return !M.getContext().getOptPassGate().shouldRunPass(this, M);
+  OptPassGate &Gate = M.getContext().getOptPassGate();
+  return Gate.isEnabled() && !Gate.shouldRunPass(this, getDescription(M));
 }
 
 bool Pass::mustPreserveAnalysisID(char &AID) const {
   return Resolver->getAnalysisIfAvailable(&AID, true) != nullptr;
 }
 
 // dumpPassStructure - Implement the -debug-pass=Structure option
 void Pass::dumpPassStructure(unsigned Offset) {
Pass *FunctionPass::createPrinterPass(raw_ostream &OS,
                                       const std::string &Banner) const {
   return createPrintFunctionPass(OS, Banner);
 }
 
 PassManagerType FunctionPass::getPotentialPassManagerType() const {
   return PMT_FunctionPassManager;
 }
 
+static std::string getDescription(const Function &F) {
+  return "function (" + F.getName().str() + ")";
+}
+
 bool FunctionPass::skipFunction(const Function &F) const {
-  if (!F.getContext().getOptPassGate().shouldRunPass(this, F))
+  OptPassGate &Gate = F.getContext().getOptPassGate();
+  if (Gate.isEnabled() && !Gate.shouldRunPass(this, getDescription(F)))
     return true;
 
   if (F.hasFnAttribute(Attribute::OptimizeNone)) {
     LLVM_DEBUG(dbgs() << "Skipping pass '" << getPassName() << "' on function "
                       << F.getName() << "\n");
     return true;
   }
   return false;
bool BasicBlockPass::doInitialization(Function &) {
   return false;
 }
 
 bool BasicBlockPass::doFinalization(Function &) {
   // By default, don't do anything.
   return false;
 }
 
+static std::string getDescription(const BasicBlock &BB) {
+  return "basic block (" + BB.getName().str() + ") in function (" +
+         BB.getParent()->getName().str() + ")";
+}
+
 bool BasicBlockPass::skipBasicBlock(const BasicBlock &BB) const {
   const Function *F = BB.getParent();
   if (!F)
     return false;
-  if (!F->getContext().getOptPassGate().shouldRunPass(this, BB))
+  OptPassGate &Gate = F->getContext().getOptPassGate();
+  if (Gate.isEnabled() && !Gate.shouldRunPass(this, getDescription(BB)))
     return true;
   if (F->hasFnAttribute(Attribute::OptimizeNone)) {
     // Report this only once per function.
     if (&BB == &F->getEntryBlock())
       LLVM_DEBUG(dbgs() << "Skipping pass '" << getPassName()
                         << "' on function " << F->getName() << "\n");
     return true;
   }

llvm/trunk/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, const Module &U) { return !Skip; }
+      bool shouldRunPass(const Pass *P, StringRef IRDescription) {
+        if (P->getPassKind() == PT_Module)
+          return !Skip;
+        return OptPassGate::shouldRunPass(P, IRDescription);
+      }
+      bool isEnabled() const { return true; }
     };
 
     // Optional module pass.
     struct ModuleOpt: public ModulePass {
       char run = 0;
       static char ID;
       ModuleOpt() : ModulePass(ID) { }
       bool runOnModule(Module &M) override {