Restore "Move metadata linking after lazy global materialization/linking."

Summary:
This reverts commit r251965.

Restore "Move metadata linking after lazy global materialization/linking."

This restores commit r251926, with fixes for the LTO bootstrapping bot
failure.

The bot failure was caused by references from debug metadata to
otherwise unreferenced globals. Previously, this caused the lazy linking
to link in their defs, which is unnecessary. With this patch, because
lazy linking is complete when we encounter the metadata reference, the
materializer created a declaration. For definitions such as aliases and
comdats, it is illegal to have a declaration. Furthermore, metadata
linking should not change code generation. Therefore, when linking of
global value bodies is complete, the materializer will simply return
nullptr as the new reference for the linked metadata.

This change required fixing a different test to ensure there was a
real reference to a linkonce global that was only being reference from
metadata.

Note that the new changes to the only-needed-named-metadata.ll test
illustrate an issue with llvm-link -only-needed handling of comdat
groups, whereby it may result in an incomplete comdat group. I note this
in the test comments, but the issue is orthogonal to this patch (it can
be reproduced without any metadata at head).

Reviewers: dexonsmith, rafael, tra

Subscribers: tobiasvk, joker.eph, llvm-commits

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

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@252320 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Linker/LinkModules.cpp b/lib/Linker/LinkModules.cpp
index 74ca197ddb3cf38bad3466a7b5bedb7afbf18630..010c5611949011c26f4477569c90d1f2624efb2f 100644 (file)
--- a/lib/Linker/LinkModules.cpp
+++ b/lib/Linker/LinkModules.cpp
@@ -440,6 +440,11 @@ class ModuleLinker {
   /// as part of a different backend compilation process.
   bool HasExportedFunctions;
 
+  /// Set to true when all global value body linking is complete (including
+  /// lazy linking). Used to prevent metadata linking from creating new
+  /// references.
+  bool DoneLinkingBodies;
+
 public:
   ModuleLinker(Module *dstM, Linker::IdentifiedStructTypeSet &Set, Module *srcM,
                DiagnosticHandlerFunction DiagnosticHandler, unsigned Flags,
@@ -448,7 +453,8 @@ public:
       : DstM(dstM), SrcM(srcM), TypeMap(Set),
         ValMaterializer(TypeMap, DstM, LazilyLinkGlobalValues, this),
         DiagnosticHandler(DiagnosticHandler), Flags(Flags), ImportIndex(Index),
-        ImportFunction(FuncToImport), HasExportedFunctions(false) {
+        ImportFunction(FuncToImport), HasExportedFunctions(false),
+        DoneLinkingBodies(false) {
     assert((ImportIndex || !ImportFunction) &&
            "Expect a FunctionInfoIndex when importing");
     // If we have a FunctionInfoIndex but no function to import,
@@ -475,6 +481,9 @@ public:
   /// Check if we should promote the given local value to global scope.
   bool doPromoteLocalToGlobal(const GlobalValue *SGV);
 
+  /// Check if all global value body linking is complete.
+  bool doneLinkingBodies() { return DoneLinkingBodies; }
+
 private:
   bool shouldLinkFromSource(bool &LinkFromSrc, const GlobalValue &Dest,
                             const GlobalValue &Src);
@@ -888,6 +897,12 @@ Value *ValueMaterializerTy::materializeValueFor(Value *V) {
   if (!SGV)
     return nullptr;
 
+  // If we are done linking global value bodies (i.e. we are performing
+  // metadata linking), don't link in the global value due to this
+  // reference, simply map it to null.
+  if (ModLinker->doneLinkingBodies())
+    return nullptr;
+
   GlobalValue *DGV = ModLinker->copyGlobalValueProto(TypeMap, SGV);
 
   if (Comdat *SC = SGV->getComdat()) {
@@ -1918,6 +1933,10 @@ bool ModuleLinker::run() {
       return true;
   }
 
+  // Note that we are done linking global value bodies. This prevents
+  // metadata linking from creating new references.
+  DoneLinkingBodies = true;
+
   // Remap all of the named MDNodes in Src into the DstM module. We do this
   // after linking GlobalValues so that MDNodes that reference GlobalValues
   // are properly remapped.

diff --git a/test/Linker/Inputs/only-needed-named-metadata.ll b/test/Linker/Inputs/only-needed-named-metadata.ll
index fa7bc2e3cc874f341626b104a56865e0635699b1..95151692d3b5093420e802ebef80d32eb673a67a 100644 (file)
--- a/test/Linker/Inputs/only-needed-named-metadata.ll
+++ b/test/Linker/Inputs/only-needed-named-metadata.ll
@@ -2,8 +2,11 @@
 
 declare i32 @foo()
 
+declare void @c1_a()
+
 define void @bar() {
        load i32, i32* @X
        call i32 @foo()
+       call void @c1_a()
        ret void
 }

diff --git a/test/Linker/distinct.ll b/test/Linker/distinct.ll
index c8e5c89eb095d23aca2fc3348e33e3fc193a7f36..d88d8ae16d9adad79590807f9fbc072ea2432ede 100644 (file)
--- a/test/Linker/distinct.ll
+++ b/test/Linker/distinct.ll
@@ -6,6 +6,8 @@
 
 ; CHECK: @global = linkonce global i32 0
 @global = linkonce global i32 0
+; Add an external reference to @global so that it gets linked in.
+@alias = alias i32, i32* @global
 
 ; CHECK: !named = !{!0, !1, !2, !3, !4, !5, !6, !7, !8, !0, !1, !2, !9, !10, !11, !12, !13, !14}
 !named = !{!0, !1, !2, !3, !4, !5, !6, !7, !8}

diff --git a/test/Linker/only-needed-named-metadata.ll b/test/Linker/only-needed-named-metadata.ll
index f64637d3c7545a19f24aa87e9c1857052c9099a4..36b425906280f5c187d68ec56668f06393d84ddc 100644 (file)
--- a/test/Linker/only-needed-named-metadata.ll
+++ b/test/Linker/only-needed-named-metadata.ll
@@ -1,16 +1,75 @@
 ; RUN: llvm-as %S/only-needed-named-metadata.ll -o %t.bc
 ; RUN: llvm-as %S/Inputs/only-needed-named-metadata.ll -o %t2.bc
-; RUN: llvm-link -S -only-needed %t2.bc %t.bc | FileCheck %s
-; RUN: llvm-link -S -internalize -only-needed %t2.bc %t.bc | FileCheck %s
 
-; CHECK: @U = external global i32
-; CHECK: declare i32 @unused()
+; Without -only-needed we should lazy link linkonce globals, and the
+; metadata reference should not cause them to be linked.
+; RUN: llvm-link -S %t2.bc %t.bc | FileCheck %s
+; CHECK-NOT:@U_linkonce
+; CHECK-NOT:@unused_linkonce()
+
+; With -only-needed the metadata references should not cause any of the
+; otherwise unreferenced globals to be linked. This also ensures that the
+; metadata references don't provoke the module linker to create declarations,
+; which are illegal for aliases and globals in comdats.
+; Note that doing -only-needed with the comdat shown below leads to a only
+; part of the comdat group being linked, which is not technically correct.
+; RUN: llvm-link -S -only-needed %t2.bc %t.bc | FileCheck %s -check-prefix=ONLYNEEDED
+; RUN: llvm-link -S -internalize -only-needed %t2.bc %t.bc | FileCheck %s -check-prefix=ONLYNEEDED
+; ONLYNEEDED-NOT:@U
+; ONLYNEEDED-NOT:@U_linkonce
+; ONLYNEEDED-NOT:@unused()
+; ONLYNEEDED-NOT:@unused_linkonce()
+; ONLYNEEDED-NOT:@linkoncealias
+; ONLYNEEDED-NOT:@linkoncefunc2()
+; ONLYNEEDED-NOT:@weakalias
+; ONLYNEEDED-NOT:@globalfunc1()
+; ONLYNEEDED-NOT:@analias
+; ONLYNEEDED-NOT:@globalfunc2()
+; ONLYNEEDED-NOT:@c1_c
+; ONLYNEEDED-NOT:@c1()
+
+$c1 = comdat any
+@c1_c = global i32 0, comdat($c1)
+define void @c1() comdat {
+  ret void
+}
+define void @c1_a() comdat($c1) {
+  ret void
+}
 
 @X = global i32 5
 @U = global i32 6
+@U_linkonce = linkonce_odr hidden global i32 6
 define i32 @foo() { ret i32 7 }
 define i32 @unused() { ret i32 8 }
+define linkonce_odr hidden i32 @unused_linkonce() { ret i32 8 }
+@linkoncealias = alias void (...), bitcast (void ()* @linkoncefunc2 to void (...)*)
+
+@weakalias = weak alias void (...), bitcast (void ()* @globalfunc1 to void (...)*)
+@analias = alias void (...), bitcast (void ()* @globalfunc2 to void (...)*)
+
+define void @globalfunc1() #0 {
+entry:
+  ret void
+}
+
+define void @globalfunc2() #0 {
+entry:
+  ret void
+}
+
+$linkoncefunc2 = comdat any
+define linkonce_odr void @linkoncefunc2() #0 comdat {
+entry:
+  ret void
+}
 
-!llvm.named = !{!0, !1}
+!llvm.named = !{!0, !1, !2, !3, !4, !5, !6, !7}
 !0 = !{i32 ()* @unused}
 !1 = !{i32* @U}
+!2 = !{i32 ()* @unused_linkonce}
+!3 = !{i32* @U_linkonce}
+!4 = !{void (...)* @weakalias}
+!5 = !{void (...)* @analias}
+!6 = !{void (...)* @linkoncealias}
+!7 = !{void ()* @c1}