[llvm] r254624 - [ThinLTO] Appending linkage fixes

Teresa Johnson via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 3 10:20:05 PST 2015


Author: tejohnson
Date: Thu Dec  3 12:20:05 2015
New Revision: 254624

URL: http://llvm.org/viewvc/llvm-project?rev=254624&view=rev
Log:
[ThinLTO] Appending linkage fixes

Summary:
Fix import from module with appending var, which cannot be imported. The
first fix is to remove an overly-aggressive error check.

The second fix is to deal with restructuring introduced to the module
linker yesterday in r254418 (actually, this fix was included already
in r254559, just added some additional cleanup).

Test by Mehdi Amini.

Reviewers: joker.eph, rafael

Subscribers: joker.eph, llvm-commits

Differential Revision: http://reviews.llvm.org/D15156

Added:
    llvm/trunk/test/Linker/Inputs/funcimport_appending_global.ll
    llvm/trunk/test/Linker/funcimport_appending_global.ll
Modified:
    llvm/trunk/lib/Linker/LinkModules.cpp

Modified: llvm/trunk/lib/Linker/LinkModules.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Linker/LinkModules.cpp?rev=254624&r1=254623&r2=254624&view=diff
==============================================================================
--- llvm/trunk/lib/Linker/LinkModules.cpp (original)
+++ llvm/trunk/lib/Linker/LinkModules.cpp Thu Dec  3 12:20:05 2015
@@ -726,8 +726,10 @@ GlobalValue::LinkageTypes ModuleLinker::
     // It would be incorrect to import an appending linkage variable,
     // since it would cause global constructors/destructors to be
     // executed multiple times. This should have already been handled
-    // by linkGlobalValueProto.
-    llvm_unreachable("Cannot import appending linkage variable");
+    // by linkIfNeeded, and we will assert in shouldLinkFromSource
+    // if we try to import, so we simply return AppendingLinkage here
+    // as this helper is called more widely in getLinkedToGlobal.
+    return GlobalValue::AppendingLinkage;
 
   case GlobalValue::InternalLinkage:
   case GlobalValue::PrivateLinkage:
@@ -1015,8 +1017,7 @@ bool ModuleLinker::shouldLinkFromSource(
 
   // We always have to add Src if it has appending linkage.
   if (Src.hasAppendingLinkage()) {
-    // Caller should have already determined that we can't link from source
-    // when importing (see comments in linkGlobalValueProto).
+    // Should have prevented importing for appending linkage in linkIfNeeded.
     assert(!isPerformingImport());
     LinkFromSrc = true;
     return false;
@@ -1387,9 +1388,12 @@ Constant *ModuleLinker::linkGlobalValueP
 
   // Handle the ultra special appending linkage case first.
   assert(!DGV || SGV->hasAppendingLinkage() == DGV->hasAppendingLinkage());
-  if (SGV->hasAppendingLinkage())
+  if (SGV->hasAppendingLinkage()) {
+    // Should have prevented importing for appending linkage in linkIfNeeded.
+    assert(!isPerformingImport());
     return linkAppendingVarProto(cast_or_null<GlobalVariable>(DGV),
                                  cast<GlobalVariable>(SGV));
+  }
 
   bool LinkFromSrc = true;
   Comdat *C = nullptr;

Added: llvm/trunk/test/Linker/Inputs/funcimport_appending_global.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Linker/Inputs/funcimport_appending_global.ll?rev=254624&view=auto
==============================================================================
--- llvm/trunk/test/Linker/Inputs/funcimport_appending_global.ll (added)
+++ llvm/trunk/test/Linker/Inputs/funcimport_appending_global.ll Thu Dec  3 12:20:05 2015
@@ -0,0 +1,6 @@
+ at v = weak global i8 1
+ at llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @foo, i8* @v}]
+
+define void @foo() {
+  ret void
+}

Added: llvm/trunk/test/Linker/funcimport_appending_global.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Linker/funcimport_appending_global.ll?rev=254624&view=auto
==============================================================================
--- llvm/trunk/test/Linker/funcimport_appending_global.ll (added)
+++ llvm/trunk/test/Linker/funcimport_appending_global.ll Thu Dec  3 12:20:05 2015
@@ -0,0 +1,20 @@
+; RUN: llvm-as -function-summary %s -o %t.bc
+; RUN: llvm-as -function-summary %p/Inputs/funcimport_appending_global.ll -o %t2.bc
+; RUN: llvm-lto -thinlto -o %t3 %t.bc %t2.bc
+
+; Do the import now
+; RUN: llvm-link %t.bc -functionindex=%t3.thinlto.bc -import=foo:%t2.bc -S | FileCheck %s
+
+; Ensure that global constructor (appending linkage) is not imported
+; CHECK-NOT: @llvm.global_ctors = {{.*}}@foo
+
+declare void @f()
+ at llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @f, i8* null}]
+
+define i32 @main() {
+entry:
+  call void @foo()
+  ret i32 0
+}
+
+declare void @foo()




More information about the llvm-commits mailing list