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

Lei Wang via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 5 23:01:14 PDT 2023


wlei added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:2237-2243
+    if (!IsMatched) {
+      uint32_t CandidateLineOffset = Loc.LineOffset + LocationDelta;
+      LineLocation Candidate(CandidateLineOffset, Loc.Discriminator);
+      InsertMatching(Loc, Candidate);
+      LLVM_DEBUG(dbgs() << "Location is matched from " << Loc << " to "
+                        << Candidate << "\n");
+    }
----------------
wenlei wrote:
> This doesn't seem to do what you intended to achieve, or at least what the comment says
> 
> > non-anchor location match is based on the offset to the last matched anchor.
> 
> If last matched anchor doesn't exist, should we still match anything? 
> 
> Consider a case where a function is completely changed, and there's no matched callsite at call, in which case we should probably still throw away its profile instead of matching everything with LocationDelta == 0?
> 
> 
Sorry if my description is unclear.

My implementation assumes a special non-callsite anchor: `0`, i.e. the beginning of the function. Without the fuzzy matching, it's just this only one anchor matched, and all the others(non-anchor) use the  `LocationDelta = 0` to do the matching, which  means the match source is equal to the target.

>Consider a case where a function is completely changed, and there's no matched callsite at call, in which case we should probably still throw away its profile instead of matching everything with LocationDelta == 0?

Say:
```
IR locations:      [1...100 101(foo)]
Profile locations: [1...100 101(bar)]
```
So my intuition is even without any callsite anchor, matching the leading location would be better than dropping them. (remembering current pseudo-probe implementation, the bb ids all come first, it's not uncommon)





Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147545



More information about the llvm-commits mailing list