[PATCH] D80692: Run Coverage pass before other *San passes under new pass manager, round 2

[Arthur Eubanks via Phabricator via llvm-commits]

aeubanks marked 2 inline comments as done.
aeubanks added inline comments.


================
Comment at: llvm/include/llvm/Passes/PassBuilder.h:597-598
   ///
   /// This extension point allows adding optimizations at the very end of the
   /// function optimization pipeline. A key difference between this and the
   /// legacy PassManager's OptimizerLast callback is that this extension point
----------------
leonardchan wrote:
> Will need to change the wording on this if this doesn't handle function passes anymore.
This doesn't say that `registerOptimizerLastEPCallback()` handles function passes, it says it handles passes that will run right after all the default function optimization passes. I believe that's still true. The passes were previously added at the end of `OptimizePM`, now they're added right after `OptimizePM`, which is the same thing.


================
Comment at: llvm/lib/Passes/PassBuilder.cpp:1078
 
+  for (auto &C : OptimizerLastEPCallbacks)
+    C(MPM, Level);
----------------
leonardchan wrote:
> Would it be better to add another `SmallVector` and `register*Calback()` for modules in in `PassBuilder` that we could loop through here instead of changing how these extension points work?
> 
> I imagine it would still be meaningful in the future to be able to add function passes at the end of the function optimization pipeline.
You can still add function passes via `createModuleToFunctionPassAdaptor()`, which is what I've done here for the existing usages. Given that the number of calls to `registerOptimizerLastEPCallback()` is small, I don't see a huge benefit to create a both a vector of module passes and function passes that run at the same place. If in the future we end up calling `registerOptimizerLastEPCallback()` with lots of function passes we can revisit this but for now it seems unnecessary.

This change doesn't change the ordering of any passes aside from sanitizer coverage (I believe), all usages of `registerOptimizerLastEPCallback()` will still end up putting the passes in the same place in the pipeline.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80692