[PATCH] D147545: [CSSPGO] Stale profile matching(part 2)

[David Li via Phabricator via llvm-commits]

davidxl added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:449
+  // For each function, the matcher generates a map, of which each entry is a
+  // mapping from the location of current build to the location in the profile.
+  StringMap<LocToLocMap> FuncToMatchingsMap;
----------------
nit: location -> source location


================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:1846
       ++NumMismatchedProfile;
-      return false;
+      if (!SalvageStaleProfile)
+        return false;
----------------
nit: probably rename the option to be 'resync-stale-profile' to make the meaning more accurate.


================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:2161
+// Populate the anchors from profile.
+void SampleProfileMatcher::populateProfileCallsites(
+    const FunctionSamples &FS,
----------------
The mapping from callee to location is 1 to many. Should that be handled with sequential id (per callee)?


================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:2167
+    const auto &CTM = I.second.getCallTargets();
+    if (CTM.size() == 1) {
+      StringRef CalleeName = CTM.begin()->first();
----------------
hoy wrote:
> Please add a comment for this. Basically we are filtering out possible indirect calls.
indirect call sites are good anchors too. Why not use sequential id based anchoring?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147545