[PATCH] D20586: IRLinker: fix double scheduling of mapping a global value because of an alias

[Rafael Espíndola via llvm-commits]

LGTM, thanks!

On 24 May 2016 at 14:56, Mehdi AMINI <mehdi.amini at apple.com> wrote:
> joker.eph created this revision.
> joker.eph added reviewers: rafael, dexonsmith.
> joker.eph added a subscriber: llvm-commits.
>
> This test was hitting an assertion in the value mapper because
> the IRLinker was trying to map two times @A while materializing
> the initializer for @C.
>
> Fix http://llvm.org/PR27850
>
> http://reviews.llvm.org/D20586
>
> Files:
>   lib/Linker/IRMover.cpp
>   test/Linker/alias-3.ll
>
> Index: test/Linker/alias-3.ll
> ===================================================================
> --- /dev/null
> +++ test/Linker/alias-3.ll
> @@ -0,0 +1,13 @@
> +; RUN: llvm-link %s -S -o - | FileCheck %s
> +target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
> +target triple = "x86_64-apple-macosx10.9"
> +
> +; CHECK-DAG: @A = internal constant i8 1
> +; CHECK-DAG: @B = alias i8, i8* @A
> +; CHECK-DAG: @C = global [2 x i8*] [i8* @A, i8* @B]
> +
> + at A = internal constant i8 1
> + at B = alias i8, i8* @A
> + at C = global [2 x i8*] [i8* @A, i8* @B]
> +
> +
> Index: lib/Linker/IRMover.cpp
> ===================================================================
> --- lib/Linker/IRMover.cpp
> +++ lib/Linker/IRMover.cpp
> @@ -557,6 +557,17 @@
>        return;
>    }
>
> +  // When linking a global for an alias, it will always be linked. However we
> +  // need to check if it was not already scheduled to satify a reference from a
> +  // regular global value initializer. We know if it has been schedule if the
> +  // "New" GlobalValue that is mapped here for the alias is the same as the one
> +  // already mapped. If there is an entry in the ValueMap but the value is
> +  // different, it means that the value already had a definition in the
> +  // destination module (linkonce for instance), but we need a new definition
> +  // for the alias ("New" will be different.
> +  if (ForAlias && ValueMap.lookup(Old) == New)
> +    return;
> +
>    if (ForAlias || shouldLink(New, *Old))
>      linkGlobalValueBody(*New, *Old);
>  }
>
>