[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