[PATCH] D147286: [FS-AFDO] Assign discriminators to pseudo probes

[Hongtao Yu via Phabricator via llvm-commits]

hoy added a comment.

In D147286#4243840 <https://reviews.llvm.org/D147286#4243840>, @wenlei wrote:

> In D147286#4243608 <https://reviews.llvm.org/D147286#4243608>, @hoy wrote:
>
>> In D147286#4242407 <https://reviews.llvm.org/D147286#4242407>, @wenlei wrote:
>>
>>>> Also call instructions are excluded since their dwarf discriminators are used for other purposes, i.e, storing probe ids.
>>>
>>> How/where do we handle the FS discriminator for call probes? Or will that be handled in a separate patch?
>>
>> They'll be handled separately. Basically they will be ignored in MIR sample loading.
>
> So you will need a separate patch to handle them, right? Without that, they are being ignored MIR sample loading. Perhaps worth clarifying in your change description.
>
> My understand is: this is the first (of the two?) change for FS-AFDO integration with CSSPGO, where we handle non-call probes, and call probes will be handled in the 2nd patch.

Yes, there will be a separate change for probe integration with MIR sample loading and call probes will be handled there.

>> This is to reduce hash conflicts.
>
> May be nitpicking but this can be confusing too. I thought it's not about conflict. When we have probe (i.e. CSSPGO is on), the anchors are probes instead of debug lines, so we just naturally assign FS discriminators on the probe (anchors) instead. Of course, since we have less probes than general instructions, hash collision is less likely to happen.

Right, we don't need to handle dwarf lines if we only want to use probe as the anchors. But handling probes and lines exclusively basically means we cannot do dual profiles for FS-AFDO. I think if hash conflict was not an issue we wouldn't go exclusive.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147286