[NFC] OptPassGate extracted from OptBisect

[yrouban]

Summary

This is an NFC refactoring of the OptBisect class to split it into an optional pass gate interface used by LLVMContext and the Optional Pass Bisector (OptBisect) used for debugging of optional passes.

This refactoring is needed for D44464, which introduces setOptPassGate() method to allow implementations other than OptBisect.

[fedor.sergeev]

include/llvm/IR/OptBisect.h

19–21 ↗

(On Diff #139566)

Hmm.. can we avoid adding these headers and add forward declarations of all the necessary classes, please?
(the problem of layering has been just recently discussed on llvm-dev@)

[andrew.w.kaylor]

include/llvm/IR/OptBisect.h

19–21 ↗

(On Diff #139566)

Forward declaration doesn't really fix the layering problem. We really need to not use these classes.

The trouble is that at least in the OptBisect case we don't really want to build the description unless we're doing OptBisect, but we can't build the description for CallGraphSCC without having the layering problem.

[fedor.sergeev]

include/llvm/IR/OptBisect.h

19–21 ↗

(On Diff #139566)

Okey, layering aside, forward declarations instead of Analysis headers would still be preferable here.

[yrouban]

changed 3 includes to forward declarations

[fedor.sergeev]

LGTM. Though, please, wait for Andrew's feedback.

[andrew.w.kaylor]

I'm OK with this.

[yrouban]

In D44821#1048379, @andrew.w.kaylor wrote:

Thank you Andy. Do you mind looking at D44464 then?

In D44821#1048181, @fedor.sergeev wrote:

LGTM. Though, please, wait for Andrew's feedback.

Thank you Fedor. Can you land the patch?

llvm/trunk/include/llvm/IR/LLVMContext.h

 namespace llvm {
 
 class DiagnosticInfo;
 enum DiagnosticSeverity : char;
 class Function;
 class Instruction;
 class LLVMContextImpl;
 class Module;
-class OptBisect;
+class OptPassGate;
 template <typename T> class SmallVectorImpl;
 class SMDiagnostic;
 class StringRef;
 class Twine;
 
 namespace yaml {
 
 class Output;
public:
   /// This function returns typed data populated from command line parsing.
   template <typename ValT, typename Base, ValT(Base::*Mem)>
   ValT getOption() const {
     return OptionRegistry::instance().template get<ValT, Base, Mem>();
   }
 
   /// \brief Access the object which manages optimization bisection for failure
   /// analysis.
-  OptBisect &getOptBisect();
+  OptPassGate &getOptPassGate();
+
 private:
   // Module needs access to the add/removeModule methods.
   friend class Module;
 
   /// addModule - Register a module as being instantiated in this context.  If
   /// the context is deleted, the module will be deleted as well.
   void addModule(Module*);
 

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; }
+};
 
 /// 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 {
+class OptBisect : public OptPassGate {
 public:
   /// \brief 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.
   ///
-  /// This function will immediate return true if bisection is disabled. If the
-  /// bisect limit is set to -1, the function will print a message describing
+  /// These functions immediately return true if bisection is disabled. If the
+  /// bisect limit is set to -1, the functions print a message describing
   /// the pass and the bisect number assigned to it and return true.  Otherwise,
-  /// the function will print a message with the bisect number assigned to the
+  /// the functions print 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 exceded or false if it has.
+  /// the bisect limit has not yet been exceeded or false if it has.
   ///
-  /// Most passes should not call this routine directly.  Instead, it is called
-  /// through a helper routine provided by the pass base class.  For instance,
-  /// function passes should call FunctionPass::skipFunction().
-  template <class UnitT>
-  bool shouldRunPass(const Pass *P, const UnitT &U);
+  /// Most passes should not call these routines 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;
 
 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

 Pass *CallGraphSCCPass::createPrinterPass(raw_ostream &OS,
                                           const std::string &Banner) const {
   return new PrintCallGraphPass(Banner, OS);
 }
 
 bool CallGraphSCCPass::skipSCC(CallGraphSCC &SCC) const {
   return !SCC.getCallGraph().getModule()
               .getContext()
-              .getOptBisect()
+              .getOptPassGate()
               .shouldRunPass(this, SCC);
 }
 
 char DummyCGSCCPass::ID = 0;
 
 INITIALIZE_PASS(DummyCGSCCPass, "DummyCGSCCPass", "DummyCGSCCPass", false,
                 false)

llvm/trunk/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.
   LLVMContext &Context = F->getContext();
-  if (!Context.getOptBisect().shouldRunPass(this, *L))
+  if (!Context.getOptPassGate().shouldRunPass(this, *L))
     return true;
   // Check for the OptimizeNone attribute.
   if (F->hasFnAttribute(Attribute::OptimizeNone)) {
     // FIXME: Report this to dbgs() only once per function.
     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);
 }
 
 bool RegionPass::skipRegion(Region &R) const {
   Function &F = *R.getEntry()->getParent();
-  if (!F.getContext().getOptBisect().shouldRunPass(this, R))
+  if (!F.getContext().getOptPassGate().shouldRunPass(this, R))
     return true;
 
   if (F.hasFnAttribute(Attribute::OptimizeNone)) {
     // Report this only once per function.
     if (R.getEntry() == &F.getEntryBlock())
       DEBUG(dbgs() << "Skipping pass '" << getPassName()
             << "' on function " << F.getName() << "\n");
     return true;
   }
   return false;
 }

llvm/trunk/lib/IR/LLVMContext.cpp

 }
 
 void LLVMContext::disableDebugTypeODRUniquing() { pImpl->DITypeMap.reset(); }
 
 void LLVMContext::setDiscardValueNames(bool Discard) {
   pImpl->DiscardValueNames = Discard;
 }
 
-OptBisect &LLVMContext::getOptBisect() {
-  return pImpl->getOptBisect();
+OptPassGate &LLVMContext::getOptPassGate() {
+  return pImpl->getOptPassGate();
 }
 
 const DiagnosticHandler *LLVMContext::getDiagHandlerPtr() const {
   return pImpl->DiagHandler.get();
 }
 
 std::unique_ptr<DiagnosticHandler> LLVMContext::getDiagnosticHandler() {
   return std::move(pImpl->DiagHandler);
 }

llvm/trunk/lib/IR/LLVMContextImpl.h

#include "llvm/IR/Metadata.def"
   LLVMContextImpl(LLVMContext &C);
   ~LLVMContextImpl();
 
   /// Destroy the ConstantArrays if they are not used.
   void dropTriviallyDeadConstantArrays();
 
   /// \brief Access the object which manages optimization bisection for failure
   /// analysis.
-  OptBisect &getOptBisect();
+  OptPassGate &getOptPassGate();
 };
 
 } // end namespace llvm
 
 #endif // LLVM_LIB_IR_LLVMCONTEXTIMPL_H

llvm/trunk/lib/IR/LLVMContextImpl.cpp

void LLVMContextImpl::getSyncScopeNames(
     SmallVectorImpl<StringRef> &SSNs) const {
   SSNs.resize(SSC.size());
   for (const auto &SSE : SSC)
     SSNs[SSE.second] = SSE.first();
 }
 
 /// Singleton instance of the OptBisect class.
 ///
-/// This singleton is accessed via the LLVMContext::getOptBisect() function.  It
-/// provides a mechanism to disable passes and individual optimizations at
+/// This singleton is accessed via the LLVMContext::getOptPassGate() function.
+/// It provides 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.
 ///
 /// Even if multiple LLVMContext objects are created, they will all return the
 /// same instance of OptBisect in order to provide a single bisect count.  Any
 /// code that uses the OptBisect object should be serialized when bisection is
 /// enabled in order to enable a consistent bisect count.
 static ManagedStatic<OptBisect> OptBisector;
 
-OptBisect &LLVMContextImpl::getOptBisect() {
+OptPassGate &LLVMContextImpl::getOptPassGate() {
   return *OptBisector;
 }

llvm/trunk/lib/IR/OptBisect.cpp

 
 using namespace llvm;
 
 static cl::opt<int> OptBisectLimit("opt-bisect-limit", cl::Hidden,
                                    cl::init(std::numeric_limits<int>::max()),
                                    cl::Optional,
                                    cl::desc("Maximum optimization to perform"));
 
-OptBisect::OptBisect() {
+OptBisect::OptBisect() : OptPassGate() {
   BisectEnabled = OptBisectLimit != std::numeric_limits<int>::max();
 }
 
 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";
if (F)
       Desc += F->getName();
     else
       Desc += "<<null function>>";
   }
   Desc += ")";
   return Desc;
 }
 
-// Force instantiations.
-template bool OptBisect::shouldRunPass(const Pass *, const Module &);
-template bool OptBisect::shouldRunPass(const Pass *, const Function &);
-template bool OptBisect::shouldRunPass(const Pass *, const BasicBlock &);
-template bool OptBisect::shouldRunPass(const Pass *, const Loop &);
-template bool OptBisect::shouldRunPass(const Pass *, const CallGraphSCC &);
-template bool OptBisect::shouldRunPass(const Pass *, const Region &);
-
-template <class UnitT>
-bool OptBisect::shouldRunPass(const Pass *P, const UnitT &U) {
-  if (!BisectEnabled)
-    return true;
-  return checkPass(P->getPassName(), getDescription(U));
+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, const CallGraphSCC &U) {
+  return !BisectEnabled || checkPass(P->getPassName(), getDescription(U));
 }
 
 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,
   return createPrintModulePass(OS, Banner);
 }
 
 PassManagerType ModulePass::getPotentialPassManagerType() const {
   return PMT_ModulePassManager;
 }
 
 bool ModulePass::skipModule(Module &M) const {
-  return !M.getContext().getOptBisect().shouldRunPass(this, M);
+  return !M.getContext().getOptPassGate().shouldRunPass(this, 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,
   return createPrintFunctionPass(OS, Banner);
 }
 
 PassManagerType FunctionPass::getPotentialPassManagerType() const {
   return PMT_FunctionPassManager;
 }
 
 bool FunctionPass::skipFunction(const Function &F) const {
-  if (!F.getContext().getOptBisect().shouldRunPass(this, F))
+  if (!F.getContext().getOptPassGate().shouldRunPass(this, F))
     return true;
 
   if (F.hasFnAttribute(Attribute::OptimizeNone)) {
     DEBUG(dbgs() << "Skipping pass '" << getPassName() << "' on function "
                  << F.getName() << "\n");
     return true;
   }
   return false;
bool BasicBlockPass::doFinalization(Function &) {
   // By default, don't do anything.
   return false;
 }
 
 bool BasicBlockPass::skipBasicBlock(const BasicBlock &BB) const {
   const Function *F = BB.getParent();
   if (!F)
     return false;
-  if (!F->getContext().getOptBisect().shouldRunPass(this, BB))
+  if (!F->getContext().getOptPassGate().shouldRunPass(this, BB))
     return true;
   if (F->hasFnAttribute(Attribute::OptimizeNone)) {
     // Report this only once per function.
     if (&BB == &F->getEntryBlock())
       DEBUG(dbgs() << "Skipping pass '" << getPassName()
             << "' on function " << F->getName() << "\n");
     return true;
   }