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

[Mircea Trofin via Phabricator via llvm-commits]

mtrofin added inline comments.


================
Comment at: llvm/lib/Analysis/InlineAdvisor.cpp:162
+      CallSiteLoc << " @ ";
+    unsigned int Offset =
+        DIL->getLine() - DIL->getScope()->getSubprogram()->getLine();
----------------
wenlei wrote:
> 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..
I don't think that makes it right, though. I'd suggest at least having a test here, then, i.e. bailing out if the this is greater than lhs. I'll look into the other API.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83743