[PATCH] D92897: Set legacy pass manager OptBisect to same as NPM OptBisect

2020-12-08 Thread Samuel Eubanks via Phabricator via cfe-commits 
swamulism created this revision.
swamulism added a reviewer: aeubanks.
swamulism requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

Currently there is an issue where the legacy pass manager uses a different 
OptBisect counter than the new pass manager.
This means that when using OptBisect under the NPM it runs twice when it should 
only run once.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92897

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/CodeGen/new-pass-manager-O1-opt-bisect.c
  llvm/include/llvm/Passes/StandardInstrumentations.h


Index: llvm/include/llvm/Passes/StandardInstrumentations.h
===
--- llvm/include/llvm/Passes/StandardInstrumentations.h
+++ llvm/include/llvm/Passes/StandardInstrumentations.h
@@ -281,6 +281,7 @@
   void registerCallbacks(PassInstrumentationCallbacks &PIC);
 
   TimePassesHandler &getTimePasses() { return TimePasses; }
+  OptBisectInstrumentation &getOptBisect() { return OptBisect; }
 };
 
 extern template class ChangeReporter;
Index: clang/test/CodeGen/new-pass-manager-O1-opt-bisect.c
===
--- /dev/null
+++ clang/test/CodeGen/new-pass-manager-O1-opt-bisect.c
@@ -0,0 +1,8 @@
+// Test NPM with -O1/opt-bisect
+//
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -O1 
-fexperimental-new-pass-manager %s -fdebug-pass-manager -mllvm 
-opt-bisect-limit=-1 -emit-llvm -o /dev/null 2>&1 | FileCheck %s
+
+// CHECK: BISECT: running pass (1)
+// CHECK-NOT: BISECT: running pass (1)
+
+int func(int a) { return a; }
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -1149,6 +1149,9 @@
   SI.registerCallbacks(PIC);
   PassBuilder PB(CodeGenOpts.DebugPassManager, TM.get(), PTO, PGOOpt, &PIC);
 
+  // Sets legacy pass manager OptBisect to the same one as npm so passes are 
properly skipped
+  TheModule->getContext().setOptPassGate(SI.getOptBisect());
+
   // Attempt to load pass plugins and register their callbacks with PB.
   for (auto &PluginFN : CodeGenOpts.PassPlugins) {
 auto PassPlugin = PassPlugin::Load(PluginFN);


Index: llvm/include/llvm/Passes/StandardInstrumentations.h
===
--- llvm/include/llvm/Passes/StandardInstrumentations.h
+++ llvm/include/llvm/Passes/StandardInstrumentations.h
@@ -281,6 +281,7 @@
   void registerCallbacks(PassInstrumentationCallbacks &PIC);
 
   TimePassesHandler &getTimePasses() { return TimePasses; }
+  OptBisectInstrumentation &getOptBisect() { return OptBisect; }
 };
 
 extern template class ChangeReporter;
Index: clang/test/CodeGen/new-pass-manager-O1-opt-bisect.c
===
--- /dev/null
+++ clang/test/CodeGen/new-pass-manager-O1-opt-bisect.c
@@ -0,0 +1,8 @@
+// Test NPM with -O1/opt-bisect
+//
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -O1 -fexperimental-new-pass-manager %s -fdebug-pass-manager -mllvm -opt-bisect-limit=-1 -emit-llvm -o /dev/null 2>&1 | FileCheck %s
+
+// CHECK: BISECT: running pass (1)
+// CHECK-NOT: BISECT: running pass (1)
+
+int func(int a) { return a; }
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -1149,6 +1149,9 @@
   SI.registerCallbacks(PIC);
   PassBuilder PB(CodeGenOpts.DebugPassManager, TM.get(), PTO, PGOOpt, &PIC);
 
+  // Sets legacy pass manager OptBisect to the same one as npm so passes are properly skipped
+  TheModule->getContext().setOptPassGate(SI.getOptBisect());
+
   // Attempt to load pass plugins and register their callbacks with PB.
   for (auto &PluginFN : CodeGenOpts.PassPlugins) {
 auto PassPlugin = PassPlugin::Load(PluginFN);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92897: Set legacy pass manager OptBisect to same as NPM OptBisect

2020-12-11 Thread Samuel Eubanks via Phabricator via cfe-commits 
swamulism updated this revision to Diff 311357.
swamulism added a comment.
Herald added subscribers: dexonsmith, hiraditya.

Updating to set npm OptBisect to use global singleton OptBisect instead


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92897/new/

https://reviews.llvm.org/D92897

Files:
  clang/test/CodeGen/new-pass-manager-opt-bisect.c
  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

Index: llvm/lib/Passes/StandardInstrumentations.cpp
===
--- llvm/lib/Passes/StandardInstrumentations.cpp
+++ llvm/lib/Passes/StandardInstrumentations.cpp
@@ -21,9 +21,11 @@
 #include "llvm/Analysis/LoopInfo.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/Module.h"
+#include "llvm/IR/OptBisect.h"
 #include "llvm/IR/PassInstrumentation.h"
 #include "llvm/IR/PrintPasses.h"
 #include "llvm/IR/Verifier.h"
+#include "llvm/Support/Casting.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/FormatVariadic.h"
@@ -624,11 +626,11 @@
 
 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));
   });
 }
 
Index: llvm/lib/IR/OptBisect.cpp
===
--- llvm/lib/IR/OptBisect.cpp
+++ llvm/lib/IR/OptBisect.cpp
@@ -54,3 +54,5 @@
   printPassMessage(PassName, CurBisectNum, TargetDesc, ShouldRun);
   return ShouldRun;
 }
+
+ManagedStatic llvm::OptBisector;
\ No newline at end of file
Index: llvm/lib/IR/LLVMContextImpl.cpp
===
--- llvm/lib/IR/LLVMContextImpl.cpp
+++ llvm/lib/IR/LLVMContextImpl.cpp
@@ -219,19 +219,7 @@
 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 OptBisector;
-
+/// Gets the singleton instance OptBisect if not already set
 OptPassGate &LLVMContextImpl::getOptPassGate() const {
   if (!OPG)
 OPG = &(*OptBisector);
Index: llvm/include/llvm/Passes/StandardInstrumentations.h
===
--- llvm/include/llvm/Passes/StandardInstrumentations.h
+++ llvm/include/llvm/Passes/StandardInstrumentations.h
@@ -72,7 +72,7 @@
   bool shouldRun(StringRef PassID, Any IR);
 };
 
-class OptBisectInstrumentation : public OptBisect {
+class OptBisectInstrumentation {
 public:
   OptBisectInstrumentation() {}
   void registerCallbacks(PassInstrumentationCallbacks &PIC);
Index: llvm/include/llvm/IR/OptBisect.h
===
--- llvm/include/llvm/IR/OptBisect.h
+++ 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 {
 
@@ -53,6 +54,13 @@
 
   virtual ~OptBisect() = default;
 
+  /// Checks the bisect limit to determine if the specified pass should run.
+  /// This calls 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 +72,6 @@
   /// 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; }
-
-

[PATCH] D92897: Set legacy pass manager OptBisect to same as NPM OptBisect

2020-12-11 Thread Samuel Eubanks via Phabricator via cfe-commits 
swamulism added a comment.

Thank you for the comments and code review!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92897/new/

https://reviews.llvm.org/D92897

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92897: Set legacy pass manager OptBisect to same as NPM OptBisect

2020-12-11 Thread Samuel Eubanks via Phabricator via cfe-commits 
swamulism updated this revision to Diff 311358.
swamulism marked 5 inline comments as done.
swamulism added a comment.

Address comments


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92897/new/

https://reviews.llvm.org/D92897

Files:
  clang/test/CodeGen/new-pass-manager-opt-bisect.c
  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

Index: llvm/lib/Passes/StandardInstrumentations.cpp
===
--- llvm/lib/Passes/StandardInstrumentations.cpp
+++ llvm/lib/Passes/StandardInstrumentations.cpp
@@ -624,11 +624,11 @@
 
 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));
   });
 }
 
Index: llvm/lib/IR/OptBisect.cpp
===
--- llvm/lib/IR/OptBisect.cpp
+++ llvm/lib/IR/OptBisect.cpp
@@ -54,3 +54,5 @@
   printPassMessage(PassName, CurBisectNum, TargetDesc, ShouldRun);
   return ShouldRun;
 }
+
+ManagedStatic llvm::OptBisector;
Index: llvm/lib/IR/LLVMContextImpl.cpp
===
--- llvm/lib/IR/LLVMContextImpl.cpp
+++ llvm/lib/IR/LLVMContextImpl.cpp
@@ -219,19 +219,8 @@
 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 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);
Index: llvm/include/llvm/Passes/StandardInstrumentations.h
===
--- llvm/include/llvm/Passes/StandardInstrumentations.h
+++ llvm/include/llvm/Passes/StandardInstrumentations.h
@@ -72,7 +72,7 @@
   bool shouldRun(StringRef PassID, Any IR);
 };
 
-class OptBisectInstrumentation : public OptBisect {
+class OptBisectInstrumentation {
 public:
   OptBisectInstrumentation() {}
   void registerCallbacks(PassInstrumentationCallbacks &PIC);
Index: llvm/include/llvm/IR/OptBisect.h
===
--- llvm/include/llvm/IR/OptBisect.h
+++ 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 {
 
@@ -53,6 +54,14 @@
 
   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 @@
   /// 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 @@
   unsigned LastBisectNum = 0;
 };
 
+/// Singleton instance of the OptBisect class, so multiple pass managers don't
+/// need to coordinate their uses of OptBisect.
+extern ManagedStatic OptBisector;
 } // end namespace llvm
 
 #endif // LLVM_IR_OPTBISECT_H
Index: clang/test/CodeGen/new-pass-manager-

[PATCH] D92897: Set legacy pass manager OptBisect to same as NPM OptBisect

2020-12-11 Thread Samuel Eubanks via Phabricator via cfe-commits 
swamulism updated this revision to Diff 311360.
swamulism added a comment.

Update comment strings


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92897/new/

https://reviews.llvm.org/D92897

Files:
  clang/test/CodeGen/new-pass-manager-opt-bisect.c
  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

Index: llvm/lib/Passes/StandardInstrumentations.cpp
===
--- llvm/lib/Passes/StandardInstrumentations.cpp
+++ llvm/lib/Passes/StandardInstrumentations.cpp
@@ -624,11 +624,11 @@
 
 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));
   });
 }
 
Index: llvm/lib/IR/OptBisect.cpp
===
--- llvm/lib/IR/OptBisect.cpp
+++ llvm/lib/IR/OptBisect.cpp
@@ -54,3 +54,5 @@
   printPassMessage(PassName, CurBisectNum, TargetDesc, ShouldRun);
   return ShouldRun;
 }
+
+ManagedStatic llvm::OptBisector;
Index: llvm/lib/IR/LLVMContextImpl.cpp
===
--- llvm/lib/IR/LLVMContextImpl.cpp
+++ llvm/lib/IR/LLVMContextImpl.cpp
@@ -219,19 +219,8 @@
 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 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);
Index: llvm/include/llvm/Passes/StandardInstrumentations.h
===
--- llvm/include/llvm/Passes/StandardInstrumentations.h
+++ llvm/include/llvm/Passes/StandardInstrumentations.h
@@ -72,7 +72,7 @@
   bool shouldRun(StringRef PassID, Any IR);
 };
 
-class OptBisectInstrumentation : public OptBisect {
+class OptBisectInstrumentation {
 public:
   OptBisectInstrumentation() {}
   void registerCallbacks(PassInstrumentationCallbacks &PIC);
Index: llvm/include/llvm/IR/OptBisect.h
===
--- llvm/include/llvm/IR/OptBisect.h
+++ 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 @@
 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 @@
 
   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 @@
   /// 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 @@
   unsigned LastBisectNum = 0;
 };
 
+/// Singleton instance of the OptBisect class, so multiple pa