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

[David Li via Phabricator via llvm-commits]

davidxl added a comment.

In D147545#4274843 <https://reviews.llvm.org/D147545#4274843>, @wlei wrote:

> In D147545#4255958 <https://reviews.llvm.org/D147545#4255958>, @davidxl wrote:
>
>> In D147545#4255941 <https://reviews.llvm.org/D147545#4255941>, @wlei wrote:
>>
>>> In D147545#4255928 <https://reviews.llvm.org/D147545#4255928>, @wenlei wrote:
>>>
>>>>> I wonder if there is a need to do two way anchoring to handle this.
>>>>
>>>> @davidxl can you elaborate on two way anchoring? is that also consider location of non-call site as anchor?  It's trickier to use non-callsite as anchor since we would need a "signature" for block that is unique enough.
>>>>
>>>> Also since this is all in the territory of heuristic and tuning, maybe we can get the initial version in while keep improving it. We got to this version after fair amount of internal evaluation, and it showed good results already. We still plan to keep tuning it though it will take some time to experiment.
>>>
>>> I guess what @davidxl meant for two way matching is to spilt the non-anchor range. some matches are based on the start anchor, some are based on the end anchor.
>>>
>>>   foo()
>>>   inst1
>>>   inst2
>>>   inst3
>>>   inst4
>>>   bar()
>>>
>>> if foo is mismatched and bar is matched, the current version, all inst1-4 will be mismatched.
>>> and if matching in two way, say split evenly,  inst3, inst4 will be matched based on bar().
>>
>> This one way. Another way is to do backward scanning of the anchors and create another loc mapping.  The heuristic can be used to merge two location mappings (i.e., prefer the one that matches the current IR loc. If both differs, perhaps keep both - which may complicate the profile annotation).
>
> @davidxl  For the non-anchor locations, I added the support to split the range evenly and match backwards for half of it. I tested, it showed a sight win like from ~1.1 regression to ~1.0 regression, maybe also noise, but I checked several instances, some backwards matchings can help.
>
> For the anchor locations, yes, keeping both candidates will complicate the profile annotation. "prefer the one that matches the current IR loc." or search the closest loc are also good heuristic, we plan to explore more heuristic later.
>
> Moveover, I did a statistical analysis, it appears that 86% of the anchors in the profile are unique anchor for the clang-self build, that might explain the current way can cover most of the cases. So I'm also leaning to make it simple at this moment.

I am ok with keeping it simple for now -- assuming this feature is off by default.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147545