[llvm] r279911 - [LTO] Don't create a new common unless merged has different size

[Benjamin Kramer via llvm-commits]

Looks like you forgot to update test/tools/llvm-lto2/common.ll. It
wasn't run due to a lit misconfiguration, I marked it as XFAIL for
now. Can you take a look?

On Sat, Aug 27, 2016 at 6:41 AM, Teresa Johnson via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
> Author: tejohnson
> Date: Fri Aug 26 23:41:22 2016
> New Revision: 279911
>
> URL: http://llvm.org/viewvc/llvm-project?rev=279911&view=rev
> Log:
> [LTO] Don't create a new common unless merged has different size
>
> Summary:
> This addresses a regression in common handling from the new LTO
> API in r278338. Only create a new common if the size is different.
> The type comparison against an array type fails when the size is
> different but not an array. GlobalMerge does not handle the
> array types as well and we lose some global merging opportunities.
>
> Reviewers: mehdi_amini
>
> Subscribers: junbuml, llvm-commits, mehdi_amini
>
> Differential Revision: https://reviews.llvm.org/D23955
>
> Modified:
>     llvm/trunk/lib/LTO/LTO.cpp
>     llvm/trunk/test/tools/gold/X86/common.ll
>     llvm/trunk/test/tools/gold/X86/start-lib-common.ll
>
> Modified: llvm/trunk/lib/LTO/LTO.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/LTO/LTO.cpp?rev=279911&r1=279910&r2=279911&view=diff
> ==============================================================================
> --- llvm/trunk/lib/LTO/LTO.cpp (original)
> +++ llvm/trunk/lib/LTO/LTO.cpp Fri Aug 26 23:41:22 2016
> @@ -417,16 +417,17 @@ Error LTO::run(AddOutputFn AddOutput) {
>  Error LTO::runRegularLTO(AddOutputFn AddOutput) {
>    // Make sure commons have the right size/alignment: we kept the largest from
>    // all the prevailing when adding the inputs, and we apply it here.
> +  const DataLayout &DL = RegularLTO.CombinedModule->getDataLayout();
>    for (auto &I : RegularLTO.Commons) {
> -    ArrayType *Ty =
> -        ArrayType::get(Type::getInt8Ty(RegularLTO.Ctx), I.second.Size);
>      GlobalVariable *OldGV = RegularLTO.CombinedModule->getNamedGlobal(I.first);
> -    if (OldGV && OldGV->getType()->getElementType() == Ty) {
> +    if (OldGV && DL.getTypeAllocSize(OldGV->getValueType()) == I.second.Size) {
>        // Don't create a new global if the type is already correct, just make
>        // sure the alignment is correct.
>        OldGV->setAlignment(I.second.Align);
>        continue;
>      }
> +    ArrayType *Ty =
> +        ArrayType::get(Type::getInt8Ty(RegularLTO.Ctx), I.second.Size);
>      auto *GV = new GlobalVariable(*RegularLTO.CombinedModule, Ty, false,
>                                    GlobalValue::CommonLinkage,
>                                    ConstantAggregateZero::get(Ty), "");
>
> Modified: llvm/trunk/test/tools/gold/X86/common.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/gold/X86/common.ll?rev=279911&r1=279910&r2=279911&view=diff
> ==============================================================================
> --- llvm/trunk/test/tools/gold/X86/common.ll (original)
> +++ llvm/trunk/test/tools/gold/X86/common.ll Fri Aug 26 23:41:22 2016
> @@ -19,7 +19,7 @@
>  ; RUN: llvm-dis %t3.o -o - | FileCheck %s --check-prefix=B
>
>  ; (i16 align 8) + (i8 align 16) = i16 align 16
> -; B: @a = common global [2 x i8] zeroinitializer, align 16
> +; B: @a = common global i16 0, align 16
>
>  ; RUN: %gold -plugin %llvmshlibdir/LLVMgold.so \
>  ; RUN:    --plugin-opt=emit-llvm \
> @@ -27,7 +27,7 @@
>  ; RUN: llvm-dis %t3.o -o - | FileCheck %s --check-prefix=C
>
>  ; (i16 align 8) + (i8 align 1) = i16 align 8.
> -; C: @a = common global [2 x i8] zeroinitializer, align 8
> +; C: @a = common global i16 0, align 8
>
>  ; RUN: %gold -plugin %llvmshlibdir/LLVMgold.so \
>  ; RUN:    --plugin-opt=emit-llvm \
> @@ -44,4 +44,4 @@
>  ; RUN: llvm-dis %t3.o -o - | FileCheck --check-prefix=MIXED %s
>
>  ; Mixed ELF and IR. We keep ours as common so the linker will finish the merge.
> -; MIXED: @a = common global [2 x i8] zeroinitializer, align 8
> +; MIXED: @a = common global i16 0, align 8
>
> Modified: llvm/trunk/test/tools/gold/X86/start-lib-common.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/gold/X86/start-lib-common.ll?rev=279911&r1=279910&r2=279911&view=diff
> ==============================================================================
> --- llvm/trunk/test/tools/gold/X86/start-lib-common.ll (original)
> +++ llvm/trunk/test/tools/gold/X86/start-lib-common.ll Fri Aug 26 23:41:22 2016
> @@ -19,4 +19,4 @@
>
>  ; Check that the common symbol is not dropped completely, which was a regression
>  ; in r262676.
> -; CHECK: @x = common global [4 x i8] zeroinitializer
> +; CHECK: @x = common global i32 0, align 8
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits