[PATCH] D23015: [ThinLTO] Remove functions resolved to available_externally from comdats

[Teresa Johnson via llvm-commits]

tejohnson updated this revision to Diff 68075.
tejohnson updated the summary for this revision.
tejohnson added a comment.

Rename test to something more appropriate before committing.


https://reviews.llvm.org/D23015

Files:
  lib/Transforms/IPO/FunctionImport.cpp
  test/ThinLTO/X86/Inputs/linkonce_resolution_comdat.ll
  test/ThinLTO/X86/linkonce_resolution_comdat.ll

Index: test/ThinLTO/X86/linkonce_resolution_comdat.ll
===================================================================
--- /dev/null
+++ 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
+}
Index: test/ThinLTO/X86/Inputs/linkonce_resolution_comdat.ll
===================================================================
--- /dev/null
+++ 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
+}
Index: lib/Transforms/IPO/FunctionImport.cpp
===================================================================
--- lib/Transforms/IPO/FunctionImport.cpp
+++ lib/Transforms/IPO/FunctionImport.cpp
@@ -493,6 +493,15 @@
     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


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D23015.68075.patch
Type: text/x-patch
Size: 2537 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160815/40b34030/attachment.bin>