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

[Di Mo via Phabricator via llvm-commits]

modimo marked an inline comment as not done.
modimo added inline comments.


================
Comment at: llvm/include/llvm/Analysis/ReplayInlineAdvisor.h:36
+    return (Scope == ReplayInlineScope::Module) ||
+           CallersToReplay.contains(F.getName());
+  }
----------------
hoy wrote:
> Wondering if `CallersToReplay` could contain function names with those deduplicating suffixes, like `.llvm.` and if we should exclude those suffixes from `F.getName` using `FunctionSamples::getCanonicalFnName`. If the replay file from dwarf dump or Rpass?
Both dwarf (from llvm-symbolizer) and Rpass will emit `.llvm.` suffixes. Added getCanonicalFnName to uses.


================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:1095
       }
       if (Hot || ExternalInlineAdvisor) {
         CIS.insert(CIS.begin(), AllCandidates.begin(), AllCandidates.end());
----------------
hoy wrote:
> Nit: This code doesn't exist on the `inlineHotFunctionsWithPriority` path which mainly serves csspgo. We should bring up the support there, could be in a separate change.
Makes sense. I added the code path but not sure how to generate a csspgo profile for function_metadata.ll to test it out. Is there a simple way to do that or should I create a test case from scratch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112033