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

Xinliang David Li via llvm-commits llvm-commits at lists.llvm.org
Fri Feb 3 21:48:55 PST 2017


On Fri, Feb 3, 2017 at 7:27 PM, Teresa Johnson <tejohnson at google.com> wrote:

>
>
> On Fri, Feb 3, 2017 at 5:12 PM, Xinliang David Li <davidxl at google.com>
> wrote:
>
>>
>>
>> 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.
>>
>
> But if the option is enabled in some fashion (like the current option is
> enabled), we will lose ThinLTO indirect call promotion to static functions
> since the name won't match.
>

This is referring to the second icall promotion pass right? There is no
reason such name mismatch should happen. If I remember correctly, the
indirect call target's IC value is MD5 sum of the PGOFuncName.  In FE
compilation of the PGO use phase, PGO annotator will annotate the static
function with the PGOFuncName meta data.  As long as the striping is
applied the same way in Gen and Use phase, the PGO name will match.



> Note we don't actually want to do any path prefix stripping in ThinLTO
> mode, since otherwise we will hit other issues (conflicts in the index). So
> ideally we won't have these options enabled at the same time as ThinLTO,
> but is there a way to detect and enforce that?
>

The stripping is off by default.

David

>
>
>>
>>> 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.
>>
>
> Which static promotion are you referring to? ThinLTO does not use the PGO
> name (aka global identifier) for static promotion. Only as the key to the
> combined index. It is this key that needs to match the PGO name for ThinLTO
> indirect call promotion to work.
>
> Teresa
>
> 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
>>>>>
>>>>>
>>>>>
>>>>>
>>>>
>>>
>>
>
>
> --
> Teresa Johnson |  Software Engineer |  tejohnson at google.com |
> 408-460-2413 <(408)%20460-2413>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170203/25826579/attachment.html>


More information about the llvm-commits mailing list