[PATCH] D31443: [LTO] Do not reorder global variables unnecessarily during merging

[Rafael Espíndola via llvm-commits]

We don't need a "good reason" and the order in the IR is not "source order".



On 3 April 2017 at 16:43, Tobias Edler von Koch via Phabricator
<reviews at reviews.llvm.org> wrote:
> tobiasvk updated this revision to Diff 93938.
> tobiasvk added a comment.
> Herald added a subscriber: inglorion.
>
> Rebase and update test case comment.
>
> In the meantime, r299018 moved the iteration code into a new function,
> Module::global_values().  The only users are in ModuleSymbolTable.cpp and
> ThinLTOBitcodeWriter.cpp (where an order-sensitive hash is computed, hence the
> test case change). This will not affect anything outside of LTO.
>
>
> https://reviews.llvm.org/D31443
>
> Files:
>   include/llvm/IR/Module.h
>   test/LTO/Resolution/X86/globalorder.ll
>   test/Transforms/ThinLTOBitcodeWriter/split-internal-typeid.ll
>
>
> Index: include/llvm/IR/Module.h
> ===================================================================
> --- include/llvm/IR/Module.h
> +++ include/llvm/IR/Module.h
> @@ -617,19 +617,19 @@
>      return global_objects().end();
>    }
>
> -  typedef concat_iterator<GlobalValue, iterator, global_iterator,
> +  typedef concat_iterator<GlobalValue, global_iterator, iterator,
>                            alias_iterator, ifunc_iterator>
>        global_value_iterator;
> -  typedef concat_iterator<const GlobalValue, const_iterator,
> -                          const_global_iterator, const_alias_iterator,
> +  typedef concat_iterator<const GlobalValue, const_global_iterator,
> +                          const_iterator, const_alias_iterator,
>                            const_ifunc_iterator>
>        const_global_value_iterator;
>
>    iterator_range<global_value_iterator> global_values() {
> -    return concat<GlobalValue>(functions(), globals(), aliases(), ifuncs());
> +    return concat<GlobalValue>(globals(), functions(), aliases(), ifuncs());
>    }
>    iterator_range<const_global_value_iterator> global_values() const {
> -    return concat<const GlobalValue>(functions(), globals(), aliases(),
> +    return concat<const GlobalValue>(globals(), functions(), aliases(),
>                                       ifuncs());
>    }
>
> Index: test/Transforms/ThinLTOBitcodeWriter/split-internal-typeid.ll
> ===================================================================
> --- test/Transforms/ThinLTOBitcodeWriter/split-internal-typeid.ll
> +++ test/Transforms/ThinLTOBitcodeWriter/split-internal-typeid.ll
> @@ -19,22 +19,22 @@
>  ; M0: define void @f()
>  ; M1-NOT: @f()
>  define void @f() {
> -  ; M0: llvm.type.test{{.*}}metadata !"1$f50b51a12bb012bebbeff978335e34cf"
> +  ; M0: llvm.type.test{{.*}}metadata !"1$761a10bdafb4c4efc0f254ab1655930a"
>    %p = call i1 @llvm.type.test(i8* null, metadata !0)
> -  ; M0: llvm.type.checked.load{{.*}}metadata !"2$f50b51a12bb012bebbeff978335e34cf"
> +  ; M0: llvm.type.checked.load{{.*}}metadata !"2$761a10bdafb4c4efc0f254ab1655930a"
>    %q = call {i8*, i1} @llvm.type.checked.load(i8* null, i32 0, metadata !3)
>    ret void
>  }
>
>  declare i1 @llvm.type.test(i8*, metadata)
>  declare {i8*, i1} @llvm.type.checked.load(i8*, i32, metadata)
>
>  !0 = distinct !{}
> -; M1: !0 = !{i32 0, !"1$f50b51a12bb012bebbeff978335e34cf"}
> +; M1: !0 = !{i32 0, !"1$761a10bdafb4c4efc0f254ab1655930a"}
>  !1 = !{i32 0, !0}
> -; M1: !1 = !{i32 1, !"1$f50b51a12bb012bebbeff978335e34cf"}
> +; M1: !1 = !{i32 1, !"1$761a10bdafb4c4efc0f254ab1655930a"}
>  !2 = !{i32 1, !0}
>
>  !3 = distinct !{}
> -; M1: !2 = !{i32 0, !"2$f50b51a12bb012bebbeff978335e34cf"}
> +; M1: !2 = !{i32 0, !"2$761a10bdafb4c4efc0f254ab1655930a"}
>  !4 = !{i32 0, !3}
> Index: test/LTO/Resolution/X86/globalorder.ll
> ===================================================================
> --- /dev/null
> +++ test/LTO/Resolution/X86/globalorder.ll
> @@ -0,0 +1,31 @@
> +; Check that LTO keeps global variables in source order during merging. Note
> +; however that we do not make any guarantee that a specific ordering is
> +; maintained. If there is a good reason to reorder globals, this test may be
> +; removed.
> +;
> +; RUN: llvm-as %s -o %t.bc
> +; RUN: llvm-lto2 %t.bc -O0 -save-temps -o %t.o -r %t.bc,var1,px -r %t.bc,var2,px -r %t.bc,var3,px -r %t.bc,var4,px -r %t.bc,foo,px
> +; RUN: llvm-dis -o - %t.o.0.0.preopt.bc | FileCheck %s
> +;
> +
> +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
> +target triple = "x86_64-unknown-linux-gnu"
> +
> +; CHECK: @var1 =
> + at var1 = global i32 0, align 4
> +; CHECK: @var2 =
> + at var2 = global i32 0, align 4
> +; CHECK: @var3 =
> + at var3 = global i32* @var1, align 4
> +; CHECK: @var4 =
> + at var4 = global i32* @var2, align 4
> +
> +define i32 @foo() {
> +entry:
> +  %0 = load i32*, i32** @var3, align 4
> +  %1 = load i32, i32* %0, align 4
> +  %2 = load i32*, i32** @var4, align 4
> +  %3 = load i32, i32* %2, align 4
> +  %add = add nsw i32 %3, %1
> +  ret i32 %add
> +}
>
>