[PATCH] D29512: [PGO] Directory name stripping in global identifier for static functions

[Xinliang David Li via llvm-commits]

On Fri, Feb 3, 2017 at 4:51 PM, Rong Xu <xur at google.com> wrote:

> ThinLTO is fact coupled with PGO naming scheme as of now. It needs to have
> the same naming scheme to import the right indirect-call function.
>
> We can move this options to InstrProf.cpp, but this will break the
> indirect-call-promotion in thin-lto mode.
>

It is already broken with -static-func-full-module-name.  The new option's
main purpose is to make prof-gen and use consistent, so it belongs to PGO
only.


> I'm not sure if I understand the reconstruction method. How can this
> reconstruction affect the importing in thin-lto?
>

The issue is that after static promotion (static func becomes global), the
promoted/raw name needs to be used for target lookup -- this is the reason
why the static promotion currently needs to use the exact same scheme as
PGO.  What I suggest is that PGO code should recognize the promoted symbol
and use its original name+linkage or use the PGO name metadata you
introduced.

David

>
>
>
> On Fri, Feb 3, 2017 at 4:16 PM, Xinliang David Li <davidxl at google.com>
> wrote:
>
>>
>>
>> On Fri, Feb 3, 2017 at 4:00 PM, Teresa Johnson via Phabricator <
>> reviews at reviews.llvm.org> wrote:
>>
>>> tejohnson added inline comments.
>>>
>>>
>>> ================
>>> Comment at: lib/IR/Globals.cpp:159
>>> +    else {
>>> +      if (StaticFuncStripDirNamePrefix != 0)
>>> +        FileName = stripDirPrefix(FileName,
>>> StaticFuncStripDirNamePrefix);
>>> ----------------
>>> davidxl wrote:
>>> > getGlobalIdentifier is a low level interface. Move this into
>>> getPGOFuncName
>>> Rong and I discussed this - putting it here ensures the same naming
>>> scheme is used for PGO and for the ThinLTO index. Although for ThinLTO we
>>> will likely not want this enabled. I'm not sure if there is a good way to
>>> enforce that.
>>>
>>>
>> Ideally we should not couple ThinLTO naming scheme with PGO lookup name
>> scheme -- PGO code should be able to 'reconstruct' the original static
>> function name and then use getPGOFuncName to form the lookup name -- but
>> that is for later.
>>
>> For now, it is better to isolate this internal option to the domain of
>> PGO only.
>>
>> David
>>
>>
>>
>>>
>>> https://reviews.llvm.org/D29512
>>>
>>>
>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170203/62914a42/attachment.html>