[PATCH] D79042: [llvm] Add interface to drive inlining decision using ML model

Tue May 12 22:37:08 PDT 2020

tejohnson added inline comments.

================
Comment at: llvm/test/Other/new-pm-lto-defaults.ll:79
+; CHECK-O2-NEXT: Starting llvm::Module pass manager run.
+; CHECK-O2-NEXT: Running pass: ModuleToPostOrderCGSCCPassAdaptor<{{.*}}LazyCallGraph{{.*}}>
+; CHECK-O2-NEXT: Starting CGSCC pass manager run.
----------------
mtrofin wrote:
> tejohnson wrote:
> > mtrofin wrote:
> > > tejohnson wrote:
> > > > What causes this one to get added?
> > > See PassBuilder.cpp:1329 - the patch uses the ModuleInlinerWrapper, so that we always inject, canonically, the InlineAdvisor outside the inline pass.
> > Right, I understand why the InlineAdvisorAnalysis gets added. But why does LazyCallGraph get added when it wasn't before?
> Oh! It's actually the devirt pass, which should be a no-op here. If we prefer, for simplicity/clarity, I can bypass using it in Inliner.cpp:1076
Oh I see. It is a noop because in the regular LTO case there is no simplification pass added to the CGSCC PM, and the inliner pass itself cannot create devirtualization opportunities? The regular LTO pipeline is going to be more sensitive to compile time, what is the overhead of adding this even when it is a noop? Might be better to avoid it.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79042