From 6107a4195d84279bb71f58b4b7d9b76384b43af0 Mon Sep 17 00:00:00 2001
From: Teresa Johnson <tejohnson@google.com>
Date: Mon, 15 Aug 2016 21:00:04 +0000
Subject: [PATCH] [ThinLTO] Remove functions resolved to available_externally
 from comdats

Summary:
thinLTOResolveWeakForLinkerModule needs to drop any preempted weak symbols
that were converted to available_externally from comdats, otherwise we
will get a verification failure (since available_externally is a
declaration for the linker, and no declarations can be in a comdat).

Reviewers: mehdi_amini

Subscribers: llvm-commits, mehdi_amini

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

llvm-svn: 278739
---
 llvm/lib/Transforms/IPO/FunctionImport.cpp    |  9 ++++++++
 .../X86/Inputs/linkonce_resolution_comdat.ll  | 13 +++++++++++
 .../ThinLTO/X86/linkonce_resolution_comdat.ll | 23 +++++++++++++++++++
 3 files changed, 45 insertions(+)
 create mode 100644 llvm/test/ThinLTO/X86/Inputs/linkonce_resolution_comdat.ll
 create mode 100644 llvm/test/ThinLTO/X86/linkonce_resolution_comdat.ll

diff --git a/llvm/lib/Transforms/IPO/FunctionImport.cpp b/llvm/lib/Transforms/IPO/FunctionImport.cpp
index c9d008ebd3827..4b429d8821580 100644
--- a/llvm/lib/Transforms/IPO/FunctionImport.cpp
+++ b/llvm/lib/Transforms/IPO/FunctionImport.cpp
@@ -493,6 +493,15 @@ void llvm::thinLTOResolveWeakForLinkerModule(
     DEBUG(dbgs() << "ODR fixing up linkage for `" << GV.getName() << "` from "
                  << GV.getLinkage() << " to " << NewLinkage << "\n");
     GV.setLinkage(NewLinkage);
+    // Remove functions converted to available_externally from comdats,
+    // as this is a declaration for the linker, and will be dropped eventually.
+    // It is illegal for comdats to contain declarations.
+    auto *GO = dyn_cast_or_null<GlobalObject>(&GV);
+    if (GO && GO->isDeclarationForLinker() && GO->hasComdat()) {
+      assert(GO->hasAvailableExternallyLinkage() &&
+             "Expected comdat on definition (possibly available external)");
+      GO->setComdat(nullptr);
+    }
   };
 
   // Process functions and global now
diff --git a/llvm/test/ThinLTO/X86/Inputs/linkonce_resolution_comdat.ll b/llvm/test/ThinLTO/X86/Inputs/linkonce_resolution_comdat.ll
new file mode 100644
index 0000000000000..59434a819869e
--- /dev/null
+++ b/llvm/test/ThinLTO/X86/Inputs/linkonce_resolution_comdat.ll
@@ -0,0 +1,13 @@
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+$c2 = comdat any
+
+define linkonce_odr i32 @f(i8*) unnamed_addr comdat($c2) {
+    ret i32 41
+}
+
+define i32 @g() {
+    %i = call i32 @f(i8* null)
+    ret i32 %i
+}
diff --git a/llvm/test/ThinLTO/X86/linkonce_resolution_comdat.ll b/llvm/test/ThinLTO/X86/linkonce_resolution_comdat.ll
new file mode 100644
index 0000000000000..689ea68f3e4d5
--- /dev/null
+++ b/llvm/test/ThinLTO/X86/linkonce_resolution_comdat.ll
@@ -0,0 +1,23 @@
+; This test ensures that we drop the preempted copy of @f from %t2.bc from its
+; comdat after making it available_externally. If not we would get a
+; verification error.
+; RUN: opt -module-summary %s -o %t1.bc
+; RUN: opt -module-summary %p/Inputs/linkonce_resolution_comdat.ll -o %t2.bc
+; RUN: llvm-lto -thinlto-action=run %t1.bc %t2.bc -exported-symbol=f -exported-symbol=g -disable-inlining
+
+; RUN: llvm-nm -o - < %t1.bc.thinlto.o | FileCheck %s --check-prefix=NM1
+; NM1: W f
+
+; RUN: llvm-nm -o - < %t2.bc.thinlto.o | FileCheck %s --check-prefix=NM2
+; f() would have been turned into available_externally since it is preempted,
+; and inlined into g()
+; NM2-NOT: f
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+$c1 = comdat any
+
+define linkonce_odr i32 @f(i8*) unnamed_addr comdat($c1) {
+    ret i32 43
+}