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

Mon May 11 11:50:50 PDT 2020

mtrofin added inline comments.

================
Comment at: llvm/lib/Passes/PassBuilder.cpp:628
     IP.HintThreshold = 325;
-
-    CGSCCPassManager CGPipeline(DebugLogging);
-
-    CGPipeline.addPass(InlinerPass(IP));
+    ModuleInlinerWrapperPass MIWP(IP, DebugLogging);
+    CGSCCPassManager &CGPipeline = MIWP.getPM();
----------------
davidxl wrote:
> This is for InstrPGO early inlining. I don't see the need to hook up advisor here -- even there is need in the future, it may need a different model. recommend leave this alone and add some comments.
It uses the default advisor. I can see it either way, at the moment it's behaviorally equivalent, the reason I'd use the wrapper here is for uniformity in the pass builder.

================
Comment at: llvm/lib/Transforms/IPO/Inliner.cpp:1074
+  // module pipeline by walking the SCCs in postorder (or bottom-up).
+  if (MaxDevirtIterations > 0)
+    MPM.addPass(createModuleToPostOrderCGSCCPassAdaptor(
----------------
davidxl wrote:
> Is this guard needed?
Well... only to avoid creating a wrapper if one isn't needed?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79042