[PATCH] D96456: [ThinLTO, NewPM] Add Config::OptPassBuilderHook

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka added inline comments.


================
Comment at: clang/lib/CodeGen/BackendUtil.cpp:1549
+  Conf.OptPassBuilderHook = [&](PassBuilder &PB) {
+    addSanitizers(TargetTriple, CGOpts, LOpts, PB);
+  };
----------------
tejohnson wrote:
> I can't find where this is defined.
to avoid noise I extracted into a separate NFC patch, to avoid, line 1061


================
Comment at: llvm/include/llvm/LTO/Config.h:54
+  /// For adding passes that run by opt.
+  std::function<void(PassBuilder &)> OptPassBuilderHook;
   Optional<Reloc::Model> RelocModel = Reloc::PIC_;
----------------
tejohnson wrote:
> Is this essentially the new PM equivalent to PreCodeGenPassesHook above (since this new hook runs at the end of opt)?
> Also, might be good to name this like OptimizerLastPassBuilderHook or something like that to indicate its position within the opt pipeline.
Kind of. PreCodeGenPassesHook is codegen and it's Legacy PM only.


================
Comment at: llvm/include/llvm/LTO/Config.h:54
+  /// For adding passes that run by opt.
+  std::function<void(PassBuilder &)> OptPassBuilderHook;
   Optional<Reloc::Model> RelocModel = Reloc::PIC_;
----------------
aeubanks wrote:
> vitalybuka wrote:
> > tejohnson wrote:
> > > Is this essentially the new PM equivalent to PreCodeGenPassesHook above (since this new hook runs at the end of opt)?
> > > Also, might be good to name this like OptimizerLastPassBuilderHook or something like that to indicate its position within the opt pipeline.
> > Kind of. PreCodeGenPassesHook is codegen and it's Legacy PM only.
> I think the idea is that this callback adds PassBuilder callbacks in various parts of the the pipeline. Perhaps just `PassBuilderHook`? It's weird that we only allow one, but I guess we can make it a vector in the future if necessary.
I guess the point of OptimizerLastPassBuilderHook name is to show that this is not any PassBuilder, but the one used in ::opt()


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96456