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

[Teresa Johnson via llvm-commits]

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).

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/61b8abc9/attachment.html>