[llvm] r252320 - Restore "Move metadata linking after lazy global materialization/linking."

Teresa Johnson via llvm-commits llvm-commits at lists.llvm.org
Fri Nov 6 09:50:53 PST 2015


Author: tejohnson
Date: Fri Nov  6 11:50:53 2015
New Revision: 252320

URL: http://llvm.org/viewvc/llvm-project?rev=252320&view=rev
Log:
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

Modified:
    llvm/trunk/lib/Linker/LinkModules.cpp
    llvm/trunk/test/Linker/Inputs/only-needed-named-metadata.ll
    llvm/trunk/test/Linker/distinct.ll
    llvm/trunk/test/Linker/only-needed-named-metadata.ll

Modified: llvm/trunk/lib/Linker/LinkModules.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Linker/LinkModules.cpp?rev=252320&r1=252319&r2=252320&view=diff
==============================================================================
--- llvm/trunk/lib/Linker/LinkModules.cpp (original)
+++ llvm/trunk/lib/Linker/LinkModules.cpp Fri Nov  6 11:50:53 2015
@@ -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::materializeV
   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.

Modified: llvm/trunk/test/Linker/Inputs/only-needed-named-metadata.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Linker/Inputs/only-needed-named-metadata.ll?rev=252320&r1=252319&r2=252320&view=diff
==============================================================================
--- llvm/trunk/test/Linker/Inputs/only-needed-named-metadata.ll (original)
+++ llvm/trunk/test/Linker/Inputs/only-needed-named-metadata.ll Fri Nov  6 11:50:53 2015
@@ -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
 }

Modified: llvm/trunk/test/Linker/distinct.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Linker/distinct.ll?rev=252320&r1=252319&r2=252320&view=diff
==============================================================================
--- llvm/trunk/test/Linker/distinct.ll (original)
+++ llvm/trunk/test/Linker/distinct.ll Fri Nov  6 11:50:53 2015
@@ -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.
+ at 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}

Modified: llvm/trunk/test/Linker/only-needed-named-metadata.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Linker/only-needed-named-metadata.ll?rev=252320&r1=252319&r2=252320&view=diff
==============================================================================
--- llvm/trunk/test/Linker/only-needed-named-metadata.ll (original)
+++ llvm/trunk/test/Linker/only-needed-named-metadata.ll Fri Nov  6 11:50:53 2015
@@ -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
+ at 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
+ at 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 }
+ at linkoncealias = alias void (...), bitcast (void ()* @linkoncefunc2 to void (...)*)
+
+ at weakalias = weak alias void (...), bitcast (void ()* @globalfunc1 to void (...)*)
+ at 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}




More information about the llvm-commits mailing list