[PATCH] D83743: [InlineAdvisor] New inliner advisor to replay inlining from optimization remarks

[Wenlei He via Phabricator via llvm-commits]

wenlei marked 6 inline comments as done.
wenlei added inline comments.


================
Comment at: llvm/lib/Analysis/InlineAdvisor.cpp:162
+      CallSiteLoc << " @ ";
+    unsigned int Offset =
+        DIL->getLine() - DIL->getScope()->getSubprogram()->getLine();
----------------
mtrofin wrote:
> this assumes DIL->getLine() >= DIL->getScope()->getSubprogram()->getLine(). Perhaps an assert before, or Offset could be int, and check it's non-negative?
This is consistent with `addLocationToRemarks` where we construct the location string for remarks, so the output can be consumed without change. Negative line offset is possible, we've seen that in FDO profile, and they're encoded with unsigned int there too - I don't know why that's the case, but seems there's convention and functionality-wise it works as long as everything is consistent..


================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:329
 
-  bool doInitialization(Module &M);
+  bool doInitialization(Module &M, FunctionAnalysisManager *FAM = nullptr);
   bool runOnModule(Module &M, ModuleAnalysisManager *AM,
----------------
mtrofin wrote:
> I'm curious, is there a case when FAM would be not passed? (i.e. why not just require it?)
There's another call site from old pass manager pipeline, where we don't have FAM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83743