Skip to content

Commit

Permalink
Reland r327041: [ThinLTO] Keep available_externally symbols live
Browse files Browse the repository at this point in the history
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.

(Reland with a suspected fix for a unit test failure I haven't been able
to reproduce locally)

Reviewers: pcc, tejohnson

Reviewed By: tejohnson

Subscribers: grimar, mehdi_amini, inglorion, eraman, llvm-commits

Differential Revision: https://reviews.llvm.org/D43690

llvm-svn: 327360
  • Loading branch information
vlad902 committed Mar 13, 2018
1 parent ade40dd commit aab6000
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 3 deletions.
23 changes: 20 additions & 3 deletions llvm/lib/Transforms/IPO/FunctionImport.cpp
Expand Up @@ -610,9 +610,26 @@ 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;
bool Interposable = false;
for (auto &S : VI.getSummaryList()) {
if (S->linkage() == GlobalValue::AvailableExternallyLinkage)
AvailableExternally = true;
else if (GlobalValue::isInterposableLinkage(S->linkage()))
Interposable = true;
}

if (!AvailableExternally)
return;

if (Interposable)
report_fatal_error("Interposable and available_externally symbol");
}

for (auto &S : VI.getSummaryList())
S->setLive(true);
Expand Down
9 changes: 9 additions & 0 deletions llvm/test/ThinLTO/X86/deadstrip.ll
Expand Up @@ -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 \
Expand All @@ -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
Expand Down Expand Up @@ -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 \
Expand Down Expand Up @@ -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
}
6 changes: 6 additions & 0 deletions llvm/test/Transforms/FunctionImport/Inputs/not-prevailing.ll
@@ -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
}
18 changes: 18 additions & 0 deletions llvm/test/Transforms/FunctionImport/not-prevailing.ll
@@ -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 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: Interposable and available_externally symbol

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
}

0 comments on commit aab6000

Please sign in to comment.