[PATCH] D87951: Enable opt-bisect for the new pass manager

[Arthur Eubanks via Phabricator via llvm-commits]

aeubanks added a comment.



In D87951#2290564 <https://reviews.llvm.org/D87951#2290564>, @cuviper wrote:

> Hmm, thanks for those pointers. To combine opt-bisect from the two pass managers, maybe there can be an explicit call to copy stateful instrumentation? I think it helps that they are not interleaved, so we could do `MPM.run()`, then somehow `SI.copy_to(CodeGenPasses)`, then `CodeGenPasses.run()`. I still need to read that thread too...

Yeah that makes sense. Or maybe have some global counter, since this is mostly a debugging thing, although that seems fairly hacky.

> `LTOBackend` may also need to deal with this, but I don't know if flags like opt-bisect apply there.

Not sure...
I suppose we can think about interactions with the legacy PM in a later patch, this on its own is an improvement.

Another issue is opt-bisect interacting with required passes. Currently it seems that it'll print out "NOT running pass ..." on a required pass but still run it.
`PassInstrumentation::runBeforePass()` will check all instrumentations to see if it should skip the pass, but still override that decision if the pass is required.
Before, I had wanted to let the callback know if the pass was required, but at that point just overriding the decision in `runBeforePass()` worked. The logging seems pretty important here for user understandability.
Thoughts?



================
Comment at: llvm/include/llvm/Passes/StandardInstrumentations.h:73
+public:
+  OptBisectInstrumentation() {}
+  void registerCallbacks(PassInstrumentationCallbacks &PIC);
----------------
remove?
(I deleted the default constructors above just now)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87951