[PATCH] D96932: [SampleFDO] Support enabling -funique-internal-linkage-name

Wed Feb 24 16:34:32 PST 2021

hoy added a comment.

In D96932#2585567 <https://reviews.llvm.org/D96932#2585567>, @wmi wrote:

> In D96932#2585530 <https://reviews.llvm.org/D96932#2585530>, @hoy wrote:
>
>> In D96932#2585513 <https://reviews.llvm.org/D96932#2585513>, @wmi wrote:
>>
>>> In D96932#2584061 <https://reviews.llvm.org/D96932#2584061>, @hoy wrote:
>>>
>>>> BTW, in case of duplicated functions due to multi-versioning, the profile generator may not be able to merge them as expected without properly canonicalizing function names, the compiler might be able to achieve that though. As of now, it seems that the last profile for a given canonical name always supersedes all others in the compiler reader. This may need a change for a "merge" instead.
>>>
>>> Right. Do context string need to be canonicalized? If it is needed, the current canonicalization needs some enhancement to support that.
>>
>> A couple places in samplecontexttracker will need call getCanonicalFnName. What enhancement is needed in addition to that?
>
> For "foo1.llvm.xxx:3 @ foo2.llvm.xxx:2 @ foo3.__uniq.xxx", do we need getCanonicalFnName to canonicalize the whole context string? Or it is divided in samplecontexttracker and getCanonicalFnName will be called for each level?

Good point. Yes, it's a concerned that the suffixes have to be removed by compiler where the underlying profile context strings are not reusable and new strings have to be made. @wenlei will have more comments about this.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96932