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

Hongtao Yu via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 18 14:52:31 PDT 2021


hoy added a comment.

Good catch, thanks for working on this.



================
Comment at: llvm/include/llvm/Analysis/ReplayInlineAdvisor.h:36
+    return (Scope == ReplayInlineScope::Module) ||
+           CallersToReplay.contains(F.getName());
+  }
----------------
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?


================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:1095
       }
       if (Hot || ExternalInlineAdvisor) {
         CIS.insert(CIS.begin(), AllCandidates.begin(), AllCandidates.end());
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112033



More information about the llvm-commits mailing list