[NewPM] Add callback for skipped passes

[aeubanks]

Summary

Parallel to https://reviews.llvm.org/D84772.

Will use this for printing when a pass is skipped.

[aeubanks]

I'm having trouble figuring out how where "<string>" is coming from...

[ychen]

llvm/unittests/IR/PassBuilderCallbacksTest.cpp

534

Add an empty line and a short comment, the underscore should be "<string>". Please also do this for

• TEST_F(FunctionCallbacksTest, InstrumentedPasses)
• TEST_F(LoopCallbacksTest, InstrumentedPasses)
• TEST_F(CGSCCCallbacksTest, InstrumentedPasses)

553

Maybe we don't need this? This test fixture is for module passes. foo is a function name. Since the Module is always in memory without a filename, the Module::getName(() gives "<string>".

567

move up before the AnalysisHandle check. Use "<string>" for the underscore.

743

move up before the AnalysisHandle check. Use "foo" for the underscore.

745

Thanks for catching these runBeforeNonSkippedPass that I missed previously. Maybe move these to a separate patch?

898

move up before the AnalysisHandle check. Use "loop" for the underscore.

1052

move up before the AnalysisHandle check. Use "(foo)" for the underscore.

[aeubanks]

Address review comments

[aeubanks]

llvm/unittests/IR/PassBuilderCallbacksTest.cpp

553

This test runs a module pass, a function pass, and a loop pass, which is why this is necessary.

745

I'm lazy and think it's too much work for a new patch :)

[ychen]

LGTM with one nit.

llvm/unittests/IR/PassBuilderCallbacksTest.cpp

553

I see. Thanks.

737

underscore?

[aeubanks]

One last fix

llvm/include/llvm/IR/PassInstrumentation.h

public:
   // callback to use - AfterPassInvalidated.
   // We call all BeforePassFuncs to determine if a pass should run or not.
   // BeforeNonSkippedPassFuncs are called only if the pass should run.
   // TODO: currently AfterPassInvalidated does not accept IRUnit, since passing
   // already invalidated IRUnit is unsafe. There are ways to handle invalidated
   // IRUnits in a safe way, and we might pursue that as soon as there is a
   // useful instrumentation that needs it.
   using BeforePassFunc = bool(StringRef, Any);
+  using BeforeSkippedPassFunc = void(StringRef, Any);
   using BeforeNonSkippedPassFunc = void(StringRef, Any);
   using AfterPassFunc = void(StringRef, Any);
   using AfterPassInvalidatedFunc = void(StringRef);
   using BeforeAnalysisFunc = void(StringRef, Any);
   using AfterAnalysisFunc = void(StringRef, Any);
 
 public:
   PassInstrumentationCallbacks() {}
 
   /// Copying PassInstrumentationCallbacks is not intended.
   PassInstrumentationCallbacks(const PassInstrumentationCallbacks &) = delete;
   void operator=(const PassInstrumentationCallbacks &) = delete;
 
   template <typename CallableT> void registerBeforePassCallback(CallableT C) {
     BeforePassCallbacks.emplace_back(std::move(C));
   }
 
   template <typename CallableT>
+  void registerBeforeSkippedPassCallback(CallableT C) {
+    BeforeSkippedPassCallbacks.emplace_back(std::move(C));
+  }
+
+  template <typename CallableT>
   void registerBeforeNonSkippedPassCallback(CallableT C) {
     BeforeNonSkippedPassCallbacks.emplace_back(std::move(C));
   }
 
   template <typename CallableT> void registerAfterPassCallback(CallableT C) {
     AfterPassCallbacks.emplace_back(std::move(C));
   }
 
public:
   void registerAfterAnalysisCallback(CallableT C) {
     AfterAnalysisCallbacks.emplace_back(std::move(C));
   }
 
 private:
   friend class PassInstrumentation;
 
   SmallVector<llvm::unique_function<BeforePassFunc>, 4> BeforePassCallbacks;
+  SmallVector<llvm::unique_function<BeforeSkippedPassFunc>, 4>
+      BeforeSkippedPassCallbacks;
   SmallVector<llvm::unique_function<BeforeNonSkippedPassFunc>, 4>
       BeforeNonSkippedPassCallbacks;
   SmallVector<llvm::unique_function<AfterPassFunc>, 4> AfterPassCallbacks;
   SmallVector<llvm::unique_function<AfterPassInvalidatedFunc>, 4>
       AfterPassInvalidatedCallbacks;
   SmallVector<llvm::unique_function<BeforeAnalysisFunc>, 4>
       BeforeAnalysisCallbacks;
   SmallVector<llvm::unique_function<AfterAnalysisFunc>, 4>
bool runBeforePass(const PassT &Pass, const IRUnitT &IR) const {
     bool ShouldRun = true;
     for (auto &C : Callbacks->BeforePassCallbacks)
       ShouldRun &= C(Pass.name(), llvm::Any(&IR));
     ShouldRun = ShouldRun || isRequired(Pass);
 
     if (ShouldRun) {
       for (auto &C : Callbacks->BeforeNonSkippedPassCallbacks)
         C(Pass.name(), llvm::Any(&IR));
+    } else {
+      for (auto &C : Callbacks->BeforeSkippedPassCallbacks)
+        C(Pass.name(), llvm::Any(&IR));
     }
 
     return ShouldRun;
   }
 
   /// AfterPass instrumentation point - takes \p Pass instance that has
   /// just been executed and constant reference to \p IR it operates on.
   /// \p IR is guaranteed to be valid at this point.

llvm/unittests/IR/PassBuilderCallbacksTest.cpp

 
 struct MockPassInstrumentationCallbacks {
   PassInstrumentationCallbacks Callbacks;
 
   MockPassInstrumentationCallbacks() {
     ON_CALL(*this, runBeforePass(_, _)).WillByDefault(Return(true));
   }
   MOCK_METHOD2(runBeforePass, bool(StringRef PassID, llvm::Any));
+  MOCK_METHOD2(runBeforeSkippedPass, void(StringRef PassID, llvm::Any));
   MOCK_METHOD2(runBeforeNonSkippedPass, void(StringRef PassID, llvm::Any));
   MOCK_METHOD2(runAfterPass, void(StringRef PassID, llvm::Any));
   MOCK_METHOD1(runAfterPassInvalidated, void(StringRef PassID));
   MOCK_METHOD2(runBeforeAnalysis, void(StringRef PassID, llvm::Any));
   MOCK_METHOD2(runAfterAnalysis, void(StringRef PassID, llvm::Any));
 
   void registerPassInstrumentation() {
     Callbacks.registerBeforePassCallback([this](StringRef P, llvm::Any IR) {
       return this->runBeforePass(P, IR);
     });
+    Callbacks.registerBeforeSkippedPassCallback(
+        [this](StringRef P, llvm::Any IR) {
+          this->runBeforeSkippedPass(P, IR);
+        });
     Callbacks.registerBeforeNonSkippedPassCallback(
         [this](StringRef P, llvm::Any IR) {
           this->runBeforeNonSkippedPass(P, IR);
         });
     Callbacks.registerAfterPassCallback(
         [this](StringRef P, llvm::Any IR) { this->runAfterPass(P, IR); });
     Callbacks.registerAfterPassInvalidatedCallback(
         [this](StringRef P) { this->runAfterPassInvalidated(P); });
     Callbacks.registerBeforeAnalysisCallback([this](StringRef P, llvm::Any IR) {
       return this->runBeforeAnalysis(P, IR);
     });
     Callbacks.registerAfterAnalysisCallback(
         [this](StringRef P, llvm::Any IR) { this->runAfterAnalysis(P, IR); });
   }
 
   void ignoreNonMockPassInstrumentation(StringRef IRName) {
     // Generic EXPECT_CALLs are needed to match instrumentation on unimportant
     // parts of a pipeline that we do not care about (e.g. various passes added
     // by default by PassBuilder - Verifier pass etc).
     // Make sure to avoid ignoring Mock passes/analysis, we definitely want
     // to check these explicitly.
     EXPECT_CALL(*this,
                 runBeforePass(Not(HasNameRegex("Mock")), HasName(IRName)))
         .Times(AnyNumber());
+    EXPECT_CALL(
+        *this, runBeforeSkippedPass(Not(HasNameRegex("Mock")), HasName(IRName)))
+        .Times(AnyNumber());
     EXPECT_CALL(*this, runBeforeNonSkippedPass(Not(HasNameRegex("Mock")),
                                                HasName(IRName)))
         .Times(AnyNumber());
     EXPECT_CALL(*this, runAfterPass(Not(HasNameRegex("Mock")), HasName(IRName)))
         .Times(AnyNumber());
     EXPECT_CALL(*this,
                 runBeforeAnalysis(Not(HasNameRegex("Mock")), HasName(IRName)))
         .Times(AnyNumber());
EXPECT_CALL(CallbacksHandle,
       .InSequence(PISequence);
   EXPECT_CALL(
       CallbacksHandle,
       runAfterAnalysis(HasNameRegex("MockAnalysisHandle"), HasName("<string>")))
       .InSequence(PISequence);
   EXPECT_CALL(CallbacksHandle,
               runAfterPass(HasNameRegex("MockPassHandle"), HasName("<string>")))
       .InSequence(PISequence);
 
+  // No passes are skipped, so there should be no calls to
+  // runBeforeSkippedPass().
+  EXPECT_CALL(
+      CallbacksHandle,
+      runBeforeSkippedPass(HasNameRegex("MockPassHandle"), HasName("<string>")))
+      .Times(0);
+
   StringRef PipelineText = "test-transform";
   ASSERT_THAT_ERROR(PB.parsePassPipeline(PM, PipelineText, true), Succeeded())
       << "Pipeline was: " << PipelineText;
 
   PM.run(*M, AM);
 }
 
 TEST_F(ModuleCallbacksTest, InstrumentedSkippedPasses) {
   CallbacksHandle.registerPassInstrumentation();
   // Non-mock instrumentation run here can safely be ignored.
   CallbacksHandle.ignoreNonMockPassInstrumentation("<string>");
+  CallbacksHandle.ignoreNonMockPassInstrumentation("foo");
 
   // Skip all passes by returning false. Pass managers and adaptor passes are
   // also passes that observed by the callbacks.
   EXPECT_CALL(CallbacksHandle, runBeforePass(_, _))
       .WillRepeatedly(Return(false));
 
+  EXPECT_CALL(CallbacksHandle,
+              runBeforeSkippedPass(HasNameRegex("MockPassHandle"), _))
+      .Times(3);
+
   EXPECT_CALL(AnalysisHandle, run(HasName("<string>"), _)).Times(0);
   EXPECT_CALL(PassHandle, run(HasName("<string>"), _)).Times(0);
 
   // As the pass is skipped there is no nonskippedpass/afterPass,
   // beforeAnalysis/afterAnalysis as well.
   EXPECT_CALL(CallbacksHandle,
               runBeforeNonSkippedPass(HasNameRegex("MockPassHandle"), _))
       .Times(0);
   EXPECT_CALL(CallbacksHandle, runAfterPass(HasNameRegex("MockPassHandle"), _))
       .Times(0);
   EXPECT_CALL(CallbacksHandle,
               runBeforeAnalysis(HasNameRegex("MockAnalysisHandle"), _))
TEST_F(FunctionCallbacksTest, InstrumentedPasses) {
   EXPECT_CALL(
       CallbacksHandle,
       runAfterAnalysis(HasNameRegex("MockAnalysisHandle"), HasName("foo")))
       .InSequence(PISequence);
   EXPECT_CALL(CallbacksHandle,
               runAfterPass(HasNameRegex("MockPassHandle"), HasName("foo")))
       .InSequence(PISequence);
 
+  // No passes are skipped, so there should be no calls to
+  // runBeforeSkippedPass().
+  EXPECT_CALL(
+      CallbacksHandle,
+      runBeforeSkippedPass(HasNameRegex("MockPassHandle"), HasName("foo")))
+      .Times(0);
+
   // Our mock pass does not invalidate IR.
   EXPECT_CALL(CallbacksHandle,
               runAfterPassInvalidated(HasNameRegex("MockPassHandle")))
       .Times(0);
 
   StringRef PipelineText = "test-transform";
   ASSERT_THAT_ERROR(PB.parsePassPipeline(PM, PipelineText, true), Succeeded())
       << "Pipeline was: " << PipelineText;
   PM.run(*M, AM);
 }
 
 TEST_F(FunctionCallbacksTest, InstrumentedSkippedPasses) {
   CallbacksHandle.registerPassInstrumentation();
   // Non-mock instrumentation run here can safely be ignored.
   CallbacksHandle.ignoreNonMockPassInstrumentation("<string>");
   CallbacksHandle.ignoreNonMockPassInstrumentation("foo");
 
   // Skip the pass by returning false.
   EXPECT_CALL(CallbacksHandle,
               runBeforePass(HasNameRegex("MockPassHandle"), HasName("foo")))
       .WillOnce(Return(false));
 
+  EXPECT_CALL(CallbacksHandle,
+              runBeforeSkippedPass(HasNameRegex("MockPassHandle"), _))
+      .Times(1);
+
   EXPECT_CALL(AnalysisHandle, run(HasName("foo"), _)).Times(0);
   EXPECT_CALL(PassHandle, run(HasName("foo"), _)).Times(0);
 
   // As the pass is skipped there is no afterPass, beforeAnalysis/afterAnalysis
   // as well.
+  EXPECT_CALL(CallbacksHandle,
+              runBeforeNonSkippedPass(HasNameRegex("MockPassHandle"), _))
+      .Times(0);
   EXPECT_CALL(CallbacksHandle, runAfterPass(HasNameRegex("MockPassHandle"), _))
       .Times(0);
   EXPECT_CALL(CallbacksHandle,
               runAfterPassInvalidated(HasNameRegex("MockPassHandle")))
       .Times(0);
   EXPECT_CALL(CallbacksHandle, runAfterPass(HasNameRegex("MockPassHandle"), _))
       .Times(0);
   EXPECT_CALL(CallbacksHandle,
EXPECT_CALL(CallbacksHandle,
               runAfterPass(HasNameRegex("MockPassHandle"), HasName("loop")))
       .InSequence(PISequence);
 
   // Our mock pass does not invalidate IR.
   EXPECT_CALL(CallbacksHandle,
               runAfterPassInvalidated(HasNameRegex("MockPassHandle")))
       .Times(0);
 
+  // No passes are skipped, so there should be no calls to
+  // runBeforeSkippedPass().
+  EXPECT_CALL(
+      CallbacksHandle,
+      runBeforeSkippedPass(HasNameRegex("MockPassHandle"), HasName("loop")))
+      .Times(0);
+
   StringRef PipelineText = "test-transform";
   ASSERT_THAT_ERROR(PB.parsePassPipeline(PM, PipelineText, true), Succeeded())
       << "Pipeline was: " << PipelineText;
   PM.run(*M, AM);
 }
 
 TEST_F(LoopCallbacksTest, InstrumentedInvalidatingPasses) {
   CallbacksHandle.registerPassInstrumentation();
TEST_F(LoopCallbacksTest, InstrumentedSkippedPasses) {
   CallbacksHandle.ignoreNonMockPassInstrumentation("foo");
   CallbacksHandle.ignoreNonMockPassInstrumentation("loop");
 
   // Skip the pass by returning false.
   EXPECT_CALL(CallbacksHandle,
               runBeforePass(HasNameRegex("MockPassHandle"), HasName("loop")))
       .WillOnce(Return(false));
 
+  EXPECT_CALL(
+      CallbacksHandle,
+      runBeforeSkippedPass(HasNameRegex("MockPassHandle"), HasName("loop")))
+      .Times(1);
+
   EXPECT_CALL(AnalysisHandle, run(HasName("loop"), _, _)).Times(0);
   EXPECT_CALL(PassHandle, run(HasName("loop"), _, _, _)).Times(0);
 
   // As the pass is skipped there is no afterPass, beforeAnalysis/afterAnalysis
   // as well.
+  EXPECT_CALL(CallbacksHandle,
+              runBeforeNonSkippedPass(HasNameRegex("MockPassHandle"), _))
+      .Times(0);
   EXPECT_CALL(CallbacksHandle, runAfterPass(HasNameRegex("MockPassHandle"), _))
       .Times(0);
   EXPECT_CALL(CallbacksHandle,
               runAfterPassInvalidated(HasNameRegex("MockPassHandle")))
       .Times(0);
   EXPECT_CALL(CallbacksHandle,
               runBeforeAnalysis(HasNameRegex("MockAnalysisHandle"), _))
       .Times(0);
EXPECT_CALL(CallbacksHandle,
               runAfterPass(HasNameRegex("MockPassHandle"), HasName("(foo)")))
       .InSequence(PISequence);
 
   // Our mock pass does not invalidate IR.
   EXPECT_CALL(CallbacksHandle,
               runAfterPassInvalidated(HasNameRegex("MockPassHandle")))
       .Times(0);
 
+  // No passes are skipped, so there should be no calls to
+  // runBeforeSkippedPass().
+  EXPECT_CALL(
+      CallbacksHandle,
+      runBeforeSkippedPass(HasNameRegex("MockPassHandle"), HasName("(foo)")))
+      .Times(0);
+
   StringRef PipelineText = "test-transform";
   ASSERT_THAT_ERROR(PB.parsePassPipeline(PM, PipelineText, true), Succeeded())
       << "Pipeline was: " << PipelineText;
   PM.run(*M, AM);
 }
 
 TEST_F(CGSCCCallbacksTest, InstrumentedInvalidatingPasses) {
   CallbacksHandle.registerPassInstrumentation();
TEST_F(CGSCCCallbacksTest, InstrumentedSkippedPasses) {
   CallbacksHandle.ignoreNonMockPassInstrumentation("foo");
   CallbacksHandle.ignoreNonMockPassInstrumentation("(foo)");
 
   // Skip the pass by returning false.
   EXPECT_CALL(CallbacksHandle,
               runBeforePass(HasNameRegex("MockPassHandle"), HasName("(foo)")))
       .WillOnce(Return(false));
 
+  EXPECT_CALL(
+      CallbacksHandle,
+      runBeforeSkippedPass(HasNameRegex("MockPassHandle"), HasName("(foo)")))
+      .Times(1);
+
   // neither Analysis nor Pass are called.
   EXPECT_CALL(AnalysisHandle, run(HasName("(foo)"), _, _)).Times(0);
   EXPECT_CALL(PassHandle, run(HasName("(foo)"), _, _, _)).Times(0);
 
   // As the pass is skipped there is no afterPass, beforeAnalysis/afterAnalysis
   // as well.
+  EXPECT_CALL(CallbacksHandle,
+              runBeforeNonSkippedPass(HasNameRegex("MockPassHandle"), _))
+      .Times(0);
   EXPECT_CALL(CallbacksHandle, runAfterPass(HasNameRegex("MockPassHandle"), _))
       .Times(0);
   EXPECT_CALL(CallbacksHandle,
               runAfterPassInvalidated(HasNameRegex("MockPassHandle")))
       .Times(0);
   EXPECT_CALL(CallbacksHandle,
               runBeforeAnalysis(HasNameRegex("MockAnalysisHandle"), _))
       .Times(0);