[llvm] r350671 - [PGO] Use SourceFileName rather module name in PGOFuncName

[David Blaikie via llvm-commits]

On Tue, Jan 22, 2019 at 1:24 PM Teresa Johnson <tejohnson at google.com> wrote:

> On Tue, Jan 22, 2019 at 1:06 PM David Blaikie <dblaikie at gmail.com> wrote:
> >
> > Hey Rong, Teresa,
> >
> > This seems like it might be problematic to me - Couldn't multiple
> modules have the same source file name (built with different preprocessor
> defines, etc) - at least I think that's the case for some projects at
> Google.
> >
> > Does this identifier need to be unique? What are the ramifications if
> multiple modules had the same source file name & this situation?
>
> Rong will know for sure, but I think the profiles will be merged and it is
> possible there will be a profile mismatch.
>
> However,  note that this is what already happens for PGO - this change
> retains the status quo since PGO is matched during the compile from source
> where the module name (F.getParent()->getName()) is the same as
> F.getParent()->getSourceFileName(). So any uniqueness issue already exists
> with PGO.
>
> This change is to enable context sensitive PGO which must be done in the
> *LTO backends, where we have recorded the original module name in the
> SourceFileName in the bitcode (in the *LTO backends the module name is the
> .o bitcode file name).
>

OK - thanks for the context!

>
> Teresa
>
> >
> > - Dave
>
> On Tue, Jan 8, 2019 at 2:43 PM Rong Xu via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> Author: xur
>> Date: Tue Jan  8 14:39:47 2019
>> New Revision: 350671
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=350671&view=rev
>> Log:
>> [PGO] Use SourceFileName rather module name in PGOFuncName
>>
>> In LTO or Thin-lto mode (though linker plugin), the module
>> names are of temp file names which are different for
>> different compilations. Using SourceFileName avoids the issue.
>> This should not change any functionality for current PGO as
>> all the current callers of getPGOFuncName() is before LTO.
>>
>> Differential Revision: https://reviews.llvm.org/D56327
>>
>> Modified:
>>     llvm/trunk/lib/ProfileData/InstrProf.cpp
>>
>> Modified: llvm/trunk/lib/ProfileData/InstrProf.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ProfileData/InstrProf.cpp?rev=350671&r1=350670&r2=350671&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/ProfileData/InstrProf.cpp (original)
>> +++ llvm/trunk/lib/ProfileData/InstrProf.cpp Tue Jan  8 14:39:47 2019
>> @@ -252,11 +252,12 @@ static StringRef stripDirPrefix(StringRe
>>  // data, its original linkage must be non-internal.
>>  std::string getPGOFuncName(const Function &F, bool InLTO, uint64_t
>> Version) {
>>    if (!InLTO) {
>> -    StringRef FileName = (StaticFuncFullModulePrefix
>> -                              ? F.getParent()->getName()
>> -                              :
>> sys::path::filename(F.getParent()->getName()));
>> -    if (StaticFuncFullModulePrefix && StaticFuncStripDirNamePrefix != 0)
>> -      FileName = stripDirPrefix(FileName, StaticFuncStripDirNamePrefix);
>> +    StringRef FileName(F.getParent()->getSourceFileName());
>> +    uint32_t StripLevel = StaticFuncFullModulePrefix ? 0 : (uint32_t)-1;
>> +    if (StripLevel < StaticFuncStripDirNamePrefix)
>> +      StripLevel = StaticFuncStripDirNamePrefix;
>> +    if (StripLevel)
>> +      FileName = stripDirPrefix(FileName, StripLevel);
>>      return getPGOFuncName(F.getName(), F.getLinkage(), FileName,
>> Version);
>>    }
>>
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>
>
> --
> Teresa Johnson |  Software Engineer |  tejohnson at google.com |
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190122/b51eee00/attachment-0001.html>