[PATCH] D88107: [NewPM] Add callbacks to PassBuilder to run before/after parsing a pass

[Arthur Eubanks via Phabricator via llvm-commits]

aeubanks added inline comments.


================
Comment at: llvm/include/llvm/Passes/PassBuilder.h:824
+  // Before/after pass parsing callbacks
+  SmallVector<std::function<void(ModulePassManager &)>, 2>
+      ModulePassWillParseCallbacks;
----------------
ychen wrote:
> How about adding a "StringRef Element" as the first callback parameter? I could not think of a concrete use case for it now. But it is more general and canonical. The current interface is restricted to callbacks that do something for every pass.
> 
> For the name, why not just use before/after instead of willParse/parsed?
We can add `StringRef Element` in a later patch if a need arises, I would prefer not to add too much now.

I was trying to think of a nice name but couldn't think of that for some reason :). Renamed.


================
Comment at: llvm/unittests/IR/PassBuilderCallbacksTest.cpp:1189
+    int C = Counter++;
+    if (I == 0)
+      I = C;
----------------
ychen wrote:
> assert (I==0); ?
The CounterPasses added in the callbacks will run multiple times since these also run on implicitly created PassManagers, and I only want to catch very first time one runs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88107