[PATCH] D85478: [NewPM] Add callback for skipped passes

[Yuanfang Chen via Phabricator via llvm-commits]

ychen added inline comments.


================
Comment at: llvm/unittests/IR/PassBuilderCallbacksTest.cpp:534
       .InSequence(PISequence);
+  EXPECT_CALL(CallbacksHandle,
+              runBeforeSkippedPass(HasNameRegex("MockPassHandle"), _))
----------------
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)


================
Comment at: llvm/unittests/IR/PassBuilderCallbacksTest.cpp:549
   CallbacksHandle.ignoreNonMockPassInstrumentation("<string>");
+  CallbacksHandle.ignoreNonMockPassInstrumentation("foo");
 
----------------
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>".


================
Comment at: llvm/unittests/IR/PassBuilderCallbacksTest.cpp:559
 
+  EXPECT_CALL(CallbacksHandle,
+              runBeforeSkippedPass(HasNameRegex("MockPassHandle"), _))
----------------
move up before the AnalysisHandle check. Use "<string>" for the underscore.


================
Comment at: llvm/unittests/IR/PassBuilderCallbacksTest.cpp:728
 
+  EXPECT_CALL(CallbacksHandle,
+              runBeforeSkippedPass(HasNameRegex("MockPassHandle"), _))
----------------
move up before the AnalysisHandle check. Use "foo" for the underscore.


================
Comment at: llvm/unittests/IR/PassBuilderCallbacksTest.cpp:734
   // as well.
+  EXPECT_CALL(CallbacksHandle,
+              runBeforeNonSkippedPass(HasNameRegex("MockPassHandle"), _))
----------------
Thanks for catching these `runBeforeNonSkippedPass` that I missed previously. Maybe move these to a separate patch?


================
Comment at: llvm/unittests/IR/PassBuilderCallbacksTest.cpp:875
 
+  EXPECT_CALL(CallbacksHandle,
+              runBeforeSkippedPass(HasNameRegex("MockPassHandle"), _))
----------------
move up before the AnalysisHandle check. Use "loop" for the underscore.


================
Comment at: llvm/unittests/IR/PassBuilderCallbacksTest.cpp:1021
 
+  EXPECT_CALL(CallbacksHandle,
+              runBeforeSkippedPass(HasNameRegex("MockPassHandle"), _))
----------------
move up before the AnalysisHandle check. Use "(foo)" for the underscore.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85478