From aab60006842d6172ca7963e4d96540bad33ebe3d Mon Sep 17 00:00:00 2001
From: Vlad Tsyrklevich <vlad@tsyrklevich.net>
Date: Tue, 13 Mar 2018 05:08:48 +0000
Subject: [PATCH] Reland r327041: [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.

(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
---
 llvm/lib/Transforms/IPO/FunctionImport.cpp         | 23 +++++++++++++++++++---
 llvm/test/ThinLTO/X86/deadstrip.ll                 |  9 +++++++++
 .../FunctionImport/Inputs/not-prevailing.ll        |  6 ++++++
 .../Transforms/FunctionImport/not-prevailing.ll    | 18 +++++++++++++++++
 4 files changed, 53 insertions(+), 3 deletions(-)
 create mode 100644 llvm/test/Transforms/FunctionImport/Inputs/not-prevailing.ll
 create mode 100644 llvm/test/Transforms/FunctionImport/not-prevailing.ll

diff --git a/llvm/lib/Transforms/IPO/FunctionImport.cpp b/llvm/lib/Transforms/IPO/FunctionImport.cpp
index f759474..d021b4a 100644
--- a/llvm/lib/Transforms/IPO/FunctionImport.cpp
+++ b/llvm/lib/Transforms/IPO/FunctionImport.cpp
@@ -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);
diff --git a/llvm/test/ThinLTO/X86/deadstrip.ll b/llvm/test/ThinLTO/X86/deadstrip.ll
index e3f16a8..827ad22 100644
--- a/llvm/test/ThinLTO/X86/deadstrip.ll
+++ b/llvm/test/ThinLTO/X86/deadstrip.ll
@@ -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
 }
diff --git a/llvm/test/Transforms/FunctionImport/Inputs/not-prevailing.ll b/llvm/test/Transforms/FunctionImport/Inputs/not-prevailing.ll
new file mode 100644
index 0000000..ca17d7f
--- /dev/null
+++ b/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
+}
diff --git a/llvm/test/Transforms/FunctionImport/not-prevailing.ll b/llvm/test/Transforms/FunctionImport/not-prevailing.ll
new file mode 100644
index 0000000..0e8b35c
--- /dev/null
+++ b/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
+}
-- 
2.7.4