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

Rafael EspĂ­ndola via llvm-commits llvm-commits at lists.llvm.org
Fri Nov 6 13:17:34 PST 2015


Thanks!

On 6 November 2015 at 12:50, Teresa Johnson via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
> 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}
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits


More information about the llvm-commits mailing list