[llvm] r311023 - [PGO] Fix ThinLTO crash

[Teresa Johnson via llvm-commits]

On Wed, Aug 16, 2017 at 10:18 AM, Xinliang David Li via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Author: davidxl
> Date: Wed Aug 16 10:18:01 2017
> New Revision: 311023
>
> URL: http://llvm.org/viewvc/llvm-project?rev=311023&view=rev
> Log:
> [PGO] Fix ThinLTO crash
>
> Differential Revsion: http://reviews.llvm.org/D36640
>
> Added:
>     llvm/trunk/test/Transforms/FunctionImport/Inputs/funcimport_var2.ll
>     llvm/trunk/test/Transforms/FunctionImport/funcimport_var.ll
> Modified:
>     llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp
>
> Modified: llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/
> Bitcode/Writer/BitcodeWriter.cpp?rev=311023&r1=311022&r2=311023&view=diff
> ============================================================
> ==================
> --- llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp (original)
> +++ llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp Wed Aug 16 10:18:01
> 2017
> @@ -3626,6 +3626,12 @@ void IndexBitcodeWriter::writeCombinedGl
>          CallValueId = getValueId(GUID);
>          if (!CallValueId)
>            continue;
> +        // The mapping from OriginalId to GUID may return a GUID
> +        // that corresponds to a static varible. Filter it out here.
>

I think the comment should indicate why this happens - specifically because
multiple (mostly) local values will have the same OriginalId, and we are
guessing since sample PGO doesn't have better precision.
(also, typo in "variable").

+        auto *GVSum = Index.getGlobalValueSummary(GUID, false);
> +        if (GVSum &&
> +            GVSum->getSummaryKind() == GlobalValueSummary::GlobalVarKind)
> +          continue;
>        }
>        NameVals.push_back(*CallValueId);
>        if (HasProfileData)
>
> Added: llvm/trunk/test/Transforms/FunctionImport/Inputs/funcimport_var2.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/
> Transforms/FunctionImport/Inputs/funcimport_var2.ll?rev=311023&view=auto
> ============================================================
> ==================
> --- llvm/trunk/test/Transforms/FunctionImport/Inputs/funcimport_var2.ll
> (added)
> +++ llvm/trunk/test/Transforms/FunctionImport/Inputs/funcimport_var2.ll
> Wed Aug 16 10:18:01 2017
> @@ -0,0 +1,7 @@
> + at link = internal global i32 0, align 4
> +
> +; Function Attrs: norecurse nounwind readnone uwtable
> +define nonnull i32* @get_link() local_unnamed_addr {
> +  ret i32* @link
> +}
> +
>
> Added: llvm/trunk/test/Transforms/FunctionImport/funcimport_var.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/
> Transforms/FunctionImport/funcimport_var.ll?rev=311023&view=auto
> ============================================================
> ==================
> --- llvm/trunk/test/Transforms/FunctionImport/funcimport_var.ll (added)
> +++ llvm/trunk/test/Transforms/FunctionImport/funcimport_var.ll Wed Aug
> 16 10:18:01 2017
> @@ -0,0 +1,13 @@
> +; This test makes sure a static var is not selected as a callee target
> +; (which will crash compilation).
> +; RUN: opt -module-summary %s -o %t.bc
> +; RUN: opt -module-summary %p/Inputs/funcimport_var2.ll -o %t2.bc
> +; RUN: llvm-lto -thinlto -thinlto-action=thinlink -o %t3 %t.bc %t2.bc
> +; RUN: llvm-lto -thinlto -thinlto-action=import -thinlto-index=%t3 %t.bc
> %t2.bc
> +define i32 @_Z4LinkPKcS0_(i8*, i8*) local_unnamed_addr  {
> +  %3 = tail call i32 @link(i8* %0, i8* %1) #2
> +  ret i32 %3
> +}
> +
> +; Function Attrs: nounwind
> +declare i32 @link(i8*, i8*) local_unnamed_addr
>
>
> _______________________________________________
> 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 |  408-460-2413
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170816/d146510f/attachment.html>