[llvm] r327041 - [ThinLTO] Keep available_externally symbols live
Eric Christopher via llvm-commits
llvm-commits at lists.llvm.org
Thu Mar 8 17:27:55 PST 2018
Hi Vlad,
I've gone ahead and reverted this in r327094 along with the follow up
commits since it's still failing on the bots. You can reproduce the
failures with a release+asserts build where the test still fails.
-eric
On Thu, Mar 8, 2018 at 10:50 AM Vlad Tsyrklevich via llvm-commits <
llvm-commits at lists.llvm.org> wrote:
> Author: vlad.tsyrklevich
> Date: Thu Mar 8 10:48:03 2018
> New Revision: 327041
>
> URL: http://llvm.org/viewvc/llvm-project?rev=327041&view=rev
> Log:
> [ThinLTO] Keep available_externally symbols live
>
> Summary:
> This change fixes PR36483. The bug was originally introduced by a change
> that marked non-prevailing symbols dead. This broke LowerTypeTests
> handling of available_externally functions, which are non-prevailing.
> LowerTypeTests uses liveness information to avoid emitting thunks for
> unused functions.
>
> Marking available_externally functions dead is incorrect, the functions
> are used though the function definitions are not. This change keeps them
> live, and lets the EliminateAvailableExternally/GlobalDCE passes remove
> them later instead.
>
> I've also enabled EliminateAvailableExternally for all optimization
> levels, I believe it being disabled for O1 was an oversight.
>
> Reviewers: pcc, tejohnson
>
> Reviewed By: tejohnson
>
> Subscribers: grimar, mehdi_amini, inglorion, eraman, llvm-commits
>
> Differential Revision: https://reviews.llvm.org/D43690
>
> Added:
> llvm/trunk/test/Transforms/FunctionImport/Inputs/not-prevailing.ll
> llvm/trunk/test/Transforms/FunctionImport/not-prevailing.ll
> Modified:
> llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp
> llvm/trunk/test/ThinLTO/X86/deadstrip.ll
>
> Modified: llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp?rev=327041&r1=327040&r2=327041&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp (original)
> +++ llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp Thu Mar 8 10:48:03
> 2018
> @@ -544,9 +544,25 @@ void llvm::computeDeadSymbols(
> if (S->isLive())
> return;
>
> - // We do not keep live symbols that are known to be non-prevailing.
> - if (isPrevailing(VI.getGUID()) == PrevailingType::No)
> - return;
> + // We only keep live symbols that are known to be non-prevailing if
> any are
> + // available_externally. Those symbols are discarded later in the
> + // EliminateAvailableExternally pass and setting them to not-live
> breaks
> + // downstreams users of liveness information (PR36483).
> + if (isPrevailing(VI.getGUID()) == PrevailingType::No) {
> + bool AvailableExternally = false;
> + for (auto &S : VI.getSummaryList())
> + if (S->linkage() == GlobalValue::AvailableExternallyLinkage)
> + AvailableExternally = true;
> +
> + if (!AvailableExternally)
> + return;
> +
> +#ifndef NDEBUG
> + for (auto &S : VI.getSummaryList())
> + assert(!GlobalValue::isInterposableLinkage(S->linkage()) &&
> + "Symbol with interposable and available_externally
> linkages");
> +#endif
> + }
>
> for (auto &S : VI.getSummaryList())
> S->setLive(true);
>
> Modified: llvm/trunk/test/ThinLTO/X86/deadstrip.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/ThinLTO/X86/deadstrip.ll?rev=327041&r1=327040&r2=327041&view=diff
>
> ==============================================================================
> --- llvm/trunk/test/ThinLTO/X86/deadstrip.ll (original)
> +++ llvm/trunk/test/ThinLTO/X86/deadstrip.ll Thu Mar 8 10:48:03 2018
> @@ -14,6 +14,7 @@
> ; RUN: -r %t1.bc,_dead_func,pl \
> ; RUN: -r %t1.bc,_baz,l \
> ; RUN: -r %t1.bc,_boo,l \
> +; RUN: -r %t1.bc,_live_available_externally_func,l \
> ; RUN: -r %t2.bc,_baz,pl \
> ; RUN: -r %t2.bc,_boo,pl \
> ; RUN: -r %t2.bc,_dead_func,l \
> @@ -27,6 +28,8 @@
> ; COMBINED-DAG: <COMBINED {{.*}} op2=119
> ; Live, dso_local, Internal
> ; COMBINED-DAG: <COMBINED {{.*}} op2=103
> +; Live, Local, AvailableExternally
> +; COMBINED-DAG: <COMBINED {{.*}} op2=97
> ; Live, Local, External
> ; COMBINED-DAG: <COMBINED {{.*}} op2=96
> ; COMBINED-DAG: <COMBINED {{.*}} op2=96
> @@ -79,6 +82,7 @@
> ; RUN: -r %t1.bc,_dead_func,pl \
> ; RUN: -r %t1.bc,_baz,l \
> ; RUN: -r %t1.bc,_boo,l \
> +; RUN: -r %t1.bc,_live_available_externally_func,l \
> ; RUN: -r %t3.bc,_baz,pl \
> ; RUN: -r %t3.bc,_boo,pl \
> ; RUN: -r %t3.bc,_dead_func,l \
> @@ -124,8 +128,13 @@ define void @dead_func() {
> ret void
> }
>
> +define available_externally void @live_available_externally_func() {
> + ret void
> +}
> +
> define void @main() {
> call void @bar()
> call void @bar_internal()
> + call void @live_available_externally_func()
> ret void
> }
>
> Added: llvm/trunk/test/Transforms/FunctionImport/Inputs/not-prevailing.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/FunctionImport/Inputs/not-prevailing.ll?rev=327041&view=auto
>
> ==============================================================================
> --- llvm/trunk/test/Transforms/FunctionImport/Inputs/not-prevailing.ll
> (added)
> +++ llvm/trunk/test/Transforms/FunctionImport/Inputs/not-prevailing.ll Thu
> Mar 8 10:48:03 2018
> @@ -0,0 +1,6 @@
> +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
> +target triple = "x86_64-unknown-linux-gnu"
> +
> +define weak i32 @foo() {
> + ret i32 0
> +}
>
> Added: llvm/trunk/test/Transforms/FunctionImport/not-prevailing.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/FunctionImport/not-prevailing.ll?rev=327041&view=auto
>
> ==============================================================================
> --- llvm/trunk/test/Transforms/FunctionImport/not-prevailing.ll (added)
> +++ llvm/trunk/test/Transforms/FunctionImport/not-prevailing.ll Thu Mar 8
> 10:48:03 2018
> @@ -0,0 +1,18 @@
> +; RUN: opt -module-summary %s -o %t1.bc
> +; RUN: opt -module-summary -o %t2.bc %S/Inputs/not-prevailing.ll
> +; RUN: not --crash llvm-lto2 run -o %t3.bc %t1.bc %t2.bc -r %t1.bc,bar,px
> \
> +; RUN: -r %t1.bc,foo,x -r %t2.bc,foo,x -save-temps 2>&1 | FileCheck %s
> +
> +; CHECK: Assertion {{.*}} "Symbol with interposable and
> available_externally linkages"' failed
> +
> +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
> +target triple = "x86_64-unknown-linux-gnu"
> +
> +define available_externally i32 @foo() {
> + ret i32 1
> +}
> +
> +define i32 @bar() {
> + %1 = call i32 @foo()
> + ret i32 %1
> +}
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180309/d0aef35e/attachment-0001.html>
More information about the llvm-commits
mailing list