[llvm] r355068 - Fix IR/Analysis layering issue with OptBisect

Richard Trieu via llvm-commits llvm-commits at lists.llvm.org
Wed Feb 27 20:00:55 PST 2019


Author: rtrieu
Date: Wed Feb 27 20:00:55 2019
New Revision: 355068

URL: http://llvm.org/viewvc/llvm-project?rev=355068&view=rev
Log:
Fix IR/Analysis layering issue with OptBisect

OptBisect is in IR due to LLVMContext using it.  However, it uses IR units from
Analysis as well.  This change moves getDescription functions from OptBisect
to their respective IR units.  Generating names for IR units will now be up
to the callers, keeping the Analysis IR units in Analysis.  To prevent
unnecessary string generation, isEnabled function is added so that callers know
when the description needs to be generated.

Differential Revision: https://reviews.llvm.org/D58406

Modified:
    llvm/trunk/include/llvm/IR/OptBisect.h
    llvm/trunk/lib/Analysis/CallGraphSCCPass.cpp
    llvm/trunk/lib/Analysis/LoopPass.cpp
    llvm/trunk/lib/Analysis/RegionPass.cpp
    llvm/trunk/lib/IR/OptBisect.cpp
    llvm/trunk/lib/IR/Pass.cpp
    llvm/trunk/unittests/IR/LegacyPassManagerTest.cpp

Modified: llvm/trunk/include/llvm/IR/OptBisect.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/OptBisect.h?rev=355068&r1=355067&r2=355068&view=diff
==============================================================================
--- llvm/trunk/include/llvm/IR/OptBisect.h (original)
+++ llvm/trunk/include/llvm/IR/OptBisect.h Wed Feb 27 20:00:55 2019
@@ -19,12 +19,6 @@
 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.
@@ -32,12 +26,14 @@ 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
@@ -59,23 +55,19 @@ public:
 
   /// 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);
 

Modified: llvm/trunk/lib/Analysis/CallGraphSCCPass.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/CallGraphSCCPass.cpp?rev=355068&r1=355067&r2=355068&view=diff
==============================================================================
--- llvm/trunk/lib/Analysis/CallGraphSCCPass.cpp (original)
+++ llvm/trunk/lib/Analysis/CallGraphSCCPass.cpp Wed Feb 27 20:00:55 2019
@@ -681,11 +681,28 @@ Pass *CallGraphSCCPass::createPrinterPas
   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;

Modified: llvm/trunk/lib/Analysis/LoopPass.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/LoopPass.cpp?rev=355068&r1=355067&r2=355068&view=diff
==============================================================================
--- llvm/trunk/lib/Analysis/LoopPass.cpp (original)
+++ llvm/trunk/lib/Analysis/LoopPass.cpp Wed Feb 27 20:00:55 2019
@@ -383,13 +383,17 @@ void LoopPass::assignPassManager(PMStack
   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)) {

Modified: llvm/trunk/lib/Analysis/RegionPass.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/RegionPass.cpp?rev=355068&r1=355067&r2=355068&view=diff
==============================================================================
--- llvm/trunk/lib/Analysis/RegionPass.cpp (original)
+++ llvm/trunk/lib/Analysis/RegionPass.cpp Wed Feb 27 20:00:55 2019
@@ -278,9 +278,14 @@ Pass *RegionPass::createPrinterPass(raw_
   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)) {

Modified: llvm/trunk/lib/IR/OptBisect.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/OptBisect.cpp?rev=355068&r1=355067&r2=355068&view=diff
==============================================================================
--- llvm/trunk/lib/IR/OptBisect.cpp (original)
+++ llvm/trunk/lib/IR/OptBisect.cpp Wed Feb 27 20:00:55 2019
@@ -14,13 +14,6 @@
 
 #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"
@@ -46,73 +39,10 @@ static void printPassMessage(const Strin
          << "(" << 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,

Modified: llvm/trunk/lib/IR/Pass.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Pass.cpp?rev=355068&r1=355067&r2=355068&view=diff
==============================================================================
--- llvm/trunk/lib/IR/Pass.cpp (original)
+++ llvm/trunk/lib/IR/Pass.cpp Wed Feb 27 20:00:55 2019
@@ -55,8 +55,13 @@ PassManagerType ModulePass::getPotential
   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 {
@@ -154,8 +159,13 @@ PassManagerType FunctionPass::getPotenti
   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)) {
@@ -185,11 +195,17 @@ bool BasicBlockPass::doFinalization(Func
   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.

Modified: llvm/trunk/unittests/IR/LegacyPassManagerTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/IR/LegacyPassManagerTest.cpp?rev=355068&r1=355067&r2=355068&view=diff
==============================================================================
--- llvm/trunk/unittests/IR/LegacyPassManagerTest.cpp (original)
+++ llvm/trunk/unittests/IR/LegacyPassManagerTest.cpp Wed Feb 27 20:00:55 2019
@@ -400,7 +400,12 @@ namespace llvm {
     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.




More information about the llvm-commits mailing list