[PATCH] D112033: [SampleProfile] Add all callsites to AllCandidates if InlineReplay is in effect

[Di Mo via Phabricator via llvm-commits]

modimo added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:1278-1281
+  if (!CalleeSamples &&
+      !(ExternalInlineAdvisor &&
+        ExternalInlineAdvisor->hasRemarksForFunction(
+            FunctionSamples::getCanonicalFnName(*CB->getCaller()))))
----------------
modimo wrote:
> wenlei wrote:
> > wenlei wrote:
> > > Add a comment please.
> > Maybe you missed this one, a comment will be helpful. When sample loader inliner is used for reply, we want to inline callee if replay advisor say so even when the callee doesn't have profile.
> > 
> > One thing I'm not exactly sure is how robust is the sample loader inliner for handling candidate without a profile. For testing, maybe you want to try removing the check altogether for a regular (non-replay) build for a larger component, and see if there's any trouble.
> > Maybe you missed this one, a comment will be helpful. When sample loader inliner is used for reply, we want to inline callee if replay advisor say so even when the callee doesn't have profile.
> Added
> > One thing I'm not exactly sure is how robust is the sample loader inliner for handling candidate without a profile. For testing, maybe you want to try removing the check altogether for a regular (non-replay) build for a larger component, and see if there's any trouble.
> That's a good idea to validate robustness, I'm run with this check removed on CSSPGO clang build and see what shakes loose.
> 
Building Clang with this early return commented out was successful.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112033