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