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

[Teresa Johnson via llvm-commits]

Thanks, I didn't notice that one as it wasn't running (thanks for
identifying that last part!). Let me ask Mehdi if there was a
particular reason he wanted to only run on X86. If needed, will have
to create an X86 subdirectory there and test config.root.targets in
the lit as done elsewhere for X86-specific tests.

Teresa

On Mon, Aug 29, 2016 at 5:50 AM, Benjamin Kramer <benny.kra at gmail.com> wrote:
> 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



-- 
Teresa Johnson | Software Engineer | tejohnson at google.com | 408-460-2413