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

[Hongtao Yu via Phabricator via llvm-commits]

hoy added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:2289
+        if (std::optional<PseudoProbe> Probe = extractProbe(I))
+          IRLocations.emplace(LineLocation(Probe->Id, 0), StringRef());
+      }
----------------
I guess `IRLocations` should also be populated for non-CS. Maybe add a TODO?


================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:2304
+        // Force to overwrite the callee name in case any non-call location was
+        // wrote before.
+        IRLocations[IRCallsite] = CalleeName;
----------------
typo: wrote -> written

BTW, have you seen such collision? With pseudo probes this shouldn't happen. It should not happen with dwarf discriminators either.


================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:2334
+
+  if (IsFuncHashMismatch && SalvageStaleProfile) {
+    LLVM_DEBUG(dbgs() << "Run stale profile matching for " << F.getName()
----------------
BTW, wondering if you've ever seen mismatched callsites when function hash matches. The hash counts number of callsites but not their orders.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147545