[llvm] 47dbee6 - Make NPM OptBisectInstrumentation use global singleton OptBisect

[Arthur Eubanks via llvm-commits]

Author: Samuel Eubanks
Date: 2020-12-20T13:47:56-08:00
New Revision: 47dbee6790cb813e4f4bc993585b9770c357e508

URL: https://github.com/llvm/llvm-project/commit/47dbee6790cb813e4f4bc993585b9770c357e508
DIFF: https://github.com/llvm/llvm-project/commit/47dbee6790cb813e4f4bc993585b9770c357e508.diff

LOG: Make NPM OptBisectInstrumentation use global singleton OptBisect

Currently there is an issue where the legacy pass manager uses a different OptBisect counter than the new pass manager.
This fix makes the npm OptBisectInstrumentation use the global OptBisect.

Reviewed By: aeubanks

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

Added: 
    clang/test/CodeGen/new-pass-manager-opt-bisect.c

Modified: 
    llvm/include/llvm/IR/OptBisect.h
    llvm/include/llvm/Passes/StandardInstrumentations.h
    llvm/lib/IR/LLVMContextImpl.cpp
    llvm/lib/IR/OptBisect.cpp
    llvm/lib/Passes/StandardInstrumentations.cpp

Removed: 
    


################################################################################
diff  --git a/clang/test/CodeGen/new-pass-manager-opt-bisect.c b/clang/test/CodeGen/new-pass-manager-opt-bisect.c
new file mode 100644
index 000000000000..30c4a36802a1
--- /dev/null
+++ b/clang/test/CodeGen/new-pass-manager-opt-bisect.c
@@ -0,0 +1,10 @@
+// Make sure opt-bisect works through both pass managers
+//
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -O1 -fexperimental-new-pass-manager %s -mllvm -opt-bisect-limit=-1 -emit-obj -o /dev/null 2>&1 | FileCheck %s
+
+// CHECK: BISECT: running pass (1)
+// CHECK-NOT: BISECT: running pass (1)
+// Make sure that legacy pass manager is running
+// CHECK: Instruction Selection
+
+int func(int a) { return a; }

diff  --git a/llvm/include/llvm/IR/OptBisect.h b/llvm/include/llvm/IR/OptBisect.h
index 5371d88fbb7d..6c2a1b01d897 100644
--- a/llvm/include/llvm/IR/OptBisect.h
+++ b/llvm/include/llvm/IR/OptBisect.h
@@ -15,6 +15,7 @@
 #define LLVM_IR_OPTBISECT_H
 
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/ManagedStatic.h"
 
 namespace llvm {
 
@@ -32,7 +33,7 @@ class OptPassGate {
     return true;
   }
 
-  /// isEnabled should return true before calling shouldRunPass
+  /// isEnabled() should return true before calling shouldRunPass().
   virtual bool isEnabled() const { return false; }
 };
 
@@ -53,6 +54,14 @@ class OptBisect : public OptPassGate {
 
   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;
+
+  /// isEnabled() should return true before calling shouldRunPass().
+  bool isEnabled() const override { return BisectEnabled; }
+
   /// Checks the bisect limit to determine if the specified pass should run.
   ///
   /// If the bisect limit is set to -1, the function prints a message describing
@@ -64,12 +73,6 @@ class OptBisect : public OptPassGate {
   /// 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, StringRef IRDescription) override;
-
-  /// isEnabled should return true before calling shouldRunPass
-  bool isEnabled() const override { return BisectEnabled; }
-
-protected:
   bool checkPass(const StringRef PassName, const StringRef TargetDesc);
 
 private:
@@ -77,6 +80,9 @@ class OptBisect : public OptPassGate {
   unsigned LastBisectNum = 0;
 };
 
+/// Singleton instance of the OptBisect class, so multiple pass managers don't
+/// need to coordinate their uses of OptBisect.
+extern ManagedStatic<OptBisect> OptBisector;
 } // end namespace llvm
 
 #endif // LLVM_IR_OPTBISECT_H

diff  --git a/llvm/include/llvm/Passes/StandardInstrumentations.h b/llvm/include/llvm/Passes/StandardInstrumentations.h
index c816c3fc2ab9..42751beb0685 100644
--- a/llvm/include/llvm/Passes/StandardInstrumentations.h
+++ b/llvm/include/llvm/Passes/StandardInstrumentations.h
@@ -72,7 +72,7 @@ class OptNoneInstrumentation {
   bool shouldRun(StringRef PassID, Any IR);
 };
 
-class OptBisectInstrumentation : public OptBisect {
+class OptBisectInstrumentation {
 public:
   OptBisectInstrumentation() {}
   void registerCallbacks(PassInstrumentationCallbacks &PIC);

diff  --git a/llvm/lib/IR/LLVMContextImpl.cpp b/llvm/lib/IR/LLVMContextImpl.cpp
index 875c61cda423..93a0590e236c 100644
--- a/llvm/lib/IR/LLVMContextImpl.cpp
+++ b/llvm/lib/IR/LLVMContextImpl.cpp
@@ -219,19 +219,8 @@ void LLVMContextImpl::getSyncScopeNames(
     SSNs[SSE.second] = SSE.first();
 }
 
-/// Singleton instance of the OptBisect class.
-///
-/// 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;
-
+/// Gets the OptPassGate for this LLVMContextImpl, which defaults to the
+/// singleton OptBisect if not explicitly set.
 OptPassGate &LLVMContextImpl::getOptPassGate() const {
   if (!OPG)
     OPG = &(*OptBisector);

diff  --git a/llvm/lib/IR/OptBisect.cpp b/llvm/lib/IR/OptBisect.cpp
index 3104b90f3070..dc85e1316d48 100644
--- a/llvm/lib/IR/OptBisect.cpp
+++ b/llvm/lib/IR/OptBisect.cpp
@@ -54,3 +54,5 @@ bool OptBisect::checkPass(const StringRef PassName,
   printPassMessage(PassName, CurBisectNum, TargetDesc, ShouldRun);
   return ShouldRun;
 }
+
+ManagedStatic<OptBisect> llvm::OptBisector;

diff  --git a/llvm/lib/Passes/StandardInstrumentations.cpp b/llvm/lib/Passes/StandardInstrumentations.cpp
index ba713acef411..d6351a01ef27 100644
--- a/llvm/lib/Passes/StandardInstrumentations.cpp
+++ b/llvm/lib/Passes/StandardInstrumentations.cpp
@@ -624,11 +624,11 @@ static std::string getBisectDescription(Any IR) {
 
 void OptBisectInstrumentation::registerCallbacks(
     PassInstrumentationCallbacks &PIC) {
-  if (!isEnabled())
+  if (!OptBisector->isEnabled())
     return;
-
-  PIC.registerShouldRunOptionalPassCallback([this](StringRef PassID, Any IR) {
-    return isIgnored(PassID) || checkPass(PassID, getBisectDescription(IR));
+  PIC.registerShouldRunOptionalPassCallback([](StringRef PassID, Any IR) {
+    return isIgnored(PassID) ||
+           OptBisector->checkPass(PassID, getBisectDescription(IR));
   });
 }