[PATCH] D15156: [ThinLTO] Appending linkage fixes

[Teresa Johnson via llvm-commits]

tejohnson created this revision.
tejohnson added reviewers: joker.eph, rafael.
tejohnson added a subscriber: llvm-commits.
Herald added a subscriber: joker.eph.

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. The handling in linkGlobalValueProto for
not linking in appending variables during function importing is coming
too late, when we are materializing decls. Add the same check to
linkIfNeeded. (Rafael, perhaps the checking in linkGlobalValueProto and
linkIfNeeded should be commoned? Also, I'm concerned about the case
where linkGlobalValueProto decides linkFromSrc==false and there is no
existing dest GV. Previously we would skip linking for that source GV,
but now we will create a decl in the dest module, as the early return
when "!LinkFromSrc && !DGV" has been removed from linkGlobalValueProto
due to the change in callsite. Are there other lurking issues?)

Test by Mehdi Amini.

http://reviews.llvm.org/D15156

Files:
  lib/Linker/LinkModules.cpp
  test/Transforms/FunctionImport/Inputs/appending_global.ll
  test/Transforms/FunctionImport/import_appending_global.ll

Index: test/Transforms/FunctionImport/import_appending_global.ll
===================================================================
--- /dev/null
+++ test/Transforms/FunctionImport/import_appending_global.ll
@@ -0,0 +1,18 @@
+; RUN: llvm-as -function-summary %s -o %t.bc
+; RUN: llvm-as -function-summary %p/Inputs/appending_global.ll -o %t2.bc
+; RUN: llvm-lto -thinlto -o %t3 %t.bc %t2.bc
+
+; Do the import now
+; RUN: opt -function-import -summary-file %t3.thinlto.bc %s -S | FileCheck %s
+
+; Ensure that global constructors (appending linkage) is not imported
+; CHECK-NOT: @llvm.global_ctors
+
+
+define i32 @main() {
+entry:
+  call void @foo()
+  ret i32 0
+}
+
+declare void @foo()
Index: test/Transforms/FunctionImport/Inputs/appending_global.ll
===================================================================
--- /dev/null
+++ test/Transforms/FunctionImport/Inputs/appending_global.ll
@@ -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
+}
Index: lib/Linker/LinkModules.cpp
===================================================================
--- lib/Linker/LinkModules.cpp
+++ lib/Linker/LinkModules.cpp
@@ -762,8 +762,10 @@
     // 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 linkGlobalValueProto, 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:
@@ -1861,7 +1863,8 @@
 
   if (!DGV && !shouldOverrideFromSrc() &&
       (GV.hasLocalLinkage() || GV.hasLinkOnceLinkage() ||
-       GV.hasAvailableExternallyLinkage())) {
+       GV.hasAvailableExternallyLinkage() ||
+       (GV.hasAppendingLinkage() && isPerformingImport()))) {
     return false;
   }
   MapValue(&GV, ValueMap, RF_MoveDistinctMDs, &TypeMap, &ValMaterializer);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D15156.41640.patch
Type: text/x-patch
Size: 2319 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151202/e54a3ea4/attachment.bin>