leonardchan accepted this revision. leonardchan added a comment. This revision is now accepted and ready to land.
Would be worthwhile to write a small test that asserts Sancov runs before the other sanitizers under the new PM? Aside from this, LGTM. ================ 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 ---------------- aeubanks wrote: > 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. Ah I see. You're right. I misread. ================ Comment at: llvm/lib/Passes/PassBuilder.cpp:1078 + for (auto &C : OptimizerLastEPCallbacks) + C(MPM, Level); ---------------- aeubanks wrote: > 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. I see. Makes sense. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80692/new/ https://reviews.llvm.org/D80692 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
