[llvm] r257719 - Stop increasing alignment of externally-visible globals on ELF
James Molloy via llvm-commits
llvm-commits at lists.llvm.org
Thu Jan 14 07:42:27 PST 2016
Reproducer attached. This reproduces for me using a clang built on Darwin,
so I expect it'll reproduce using any stage1 clang.
On Thu, 14 Jan 2016 at 15:41 James Molloy <james at jamesmolloy.co.uk> wrote:
> Hi James,
>
> This commit is causing stage2 build problems on ARM and AArch64:
> http://lab.llvm.org:8011/builders/clang-cmake-thumbv7-a15-full-sh/builds/3055/steps/build%20stage%202/logs/stdio
>
> Could you please take a look?
>
> Cheers,
>
> James
>
> On Thu, 14 Jan 2016 at 00:03 James Y Knight via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> Author: jyknight
>> Date: Wed Jan 13 17:59:19 2016
>> New Revision: 257719
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=257719&view=rev
>> Log:
>> Stop increasing alignment of externally-visible globals on ELF
>> platforms.
>>
>> With ELF, the alignment of a global variable in a shared library will
>> get copied into an executables linked against it, if the executable even
>> accesss the variable. So, it's not possible to implicitly increase
>> alignment based on access patterns, or you'll break existing binaries.
>>
>> This happened to affect libc++'s std::cout symbol, for example. See
>> thread: http://thread.gmane.org/gmane.comp.compilers.clang.devel/45311
>>
>> Modified:
>> llvm/trunk/include/llvm/IR/GlobalValue.h
>> llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp
>> llvm/trunk/lib/IR/Globals.cpp
>> llvm/trunk/lib/Transforms/Utils/Local.cpp
>> llvm/trunk/test/CodeGen/ARM/memfunc.ll
>>
>> Modified: llvm/trunk/include/llvm/IR/GlobalValue.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/GlobalValue.h?rev=257719&r1=257718&r2=257719&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/include/llvm/IR/GlobalValue.h (original)
>> +++ llvm/trunk/include/llvm/IR/GlobalValue.h Wed Jan 13 17:59:19 2016
>> @@ -346,6 +346,10 @@ public:
>> return !(isDeclarationForLinker() || isWeakForLinker());
>> }
>>
>> + // Returns true if the alignment of the value can be unilaterally
>> + // increased.
>> + bool canIncreaseAlignment() const;
>> +
>> /// This method unlinks 'this' from the containing module, but does
>> not delete
>> /// it.
>> virtual void removeFromParent() = 0;
>>
>> Modified: llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp?rev=257719&r1=257718&r2=257719&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp Wed Jan 13 17:59:19 2016
>> @@ -1742,8 +1742,8 @@ bool CodeGenPrepare::optimizeCallInst(Ca
>> // over-aligning global variables that have an explicit section is
>> // forbidden.
>> GlobalVariable *GV;
>> - if ((GV = dyn_cast<GlobalVariable>(Val)) &&
>> GV->hasUniqueInitializer() &&
>> - !GV->hasSection() && GV->getAlignment() < PrefAlign &&
>> + if ((GV = dyn_cast<GlobalVariable>(Val)) &&
>> GV->canIncreaseAlignment() &&
>> + GV->getAlignment() < PrefAlign &&
>> DL->getTypeAllocSize(GV->getType()->getElementType()) >=
>> MinSize + Offset2)
>> GV->setAlignment(PrefAlign);
>>
>> Modified: llvm/trunk/lib/IR/Globals.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Globals.cpp?rev=257719&r1=257718&r2=257719&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/IR/Globals.cpp (original)
>> +++ llvm/trunk/lib/IR/Globals.cpp Wed Jan 13 17:59:19 2016
>> @@ -12,11 +12,12 @@
>> //
>>
>> //===----------------------------------------------------------------------===//
>>
>> -#include "llvm/IR/GlobalValue.h"
>> #include "llvm/ADT/SmallPtrSet.h"
>> +#include "llvm/ADT/Triple.h"
>> #include "llvm/IR/Constants.h"
>> #include "llvm/IR/DerivedTypes.h"
>> #include "llvm/IR/GlobalAlias.h"
>> +#include "llvm/IR/GlobalValue.h"
>> #include "llvm/IR/GlobalVariable.h"
>> #include "llvm/IR/Module.h"
>> #include "llvm/IR/Operator.h"
>> @@ -134,6 +135,47 @@ bool GlobalValue::isDeclaration() const
>> return false;
>> }
>>
>> +bool GlobalValue::canIncreaseAlignment() const {
>> + // Firstly, can only increase the alignment of a global if it
>> + // is a strong definition.
>> + if (!isStrongDefinitionForLinker())
>> + return false;
>> +
>> + // It also has to either not have a section defined, or, not have
>> + // alignment specified. (If it is assigned a section, the global
>> + // could be densely packed with other objects in the section, and
>> + // increasing the alignment could cause padding issues.)
>> + if (hasSection() && getAlignment() > 0)
>> + return false;
>> +
>> + // On ELF platforms, we're further restricted in that we can't
>> + // increase the alignment of any variable which might be emitted
>> + // into a shared library, and which is exported. If the main
>> + // executable accesses a variable found in a shared-lib, the main
>> + // exe actually allocates memory for and exports the symbol ITSELF,
>> + // overriding the symbol found in the library. That is, at link
>> + // time, the observed alignment of the variable is copied into the
>> + // executable binary. (A COPY relocation is also generated, to copy
>> + // the initial data from the shadowed variable in the shared-lib
>> + // into the location in the main binary, before running code.)
>> + //
>> + // And thus, even though you might think you are defining the
>> + // global, and allocating the memory for the global in your object
>> + // file, and thus should be able to set the alignment arbitrarily,
>> + // that's not actually true. Doing so can cause an ABI breakage; an
>> + // executable might have already been built with the previous
>> + // alignment of the variable, and then assuming an increased
>> + // alignment will be incorrect.
>> +
>> + // Conservatively assume ELF if there's no parent pointer.
>> + bool isELF =
>> + (!Parent || Triple(Parent->getTargetTriple()).isOSBinFormatELF());
>> + if (isELF && hasDefaultVisibility() && !hasLocalLinkage())
>> + return false;
>> +
>> + return true;
>> +}
>> +
>>
>> //===----------------------------------------------------------------------===//
>> // GlobalVariable Implementation
>>
>> //===----------------------------------------------------------------------===//
>>
>> Modified: llvm/trunk/lib/Transforms/Utils/Local.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/Local.cpp?rev=257719&r1=257718&r2=257719&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/Transforms/Utils/Local.cpp (original)
>> +++ llvm/trunk/lib/Transforms/Utils/Local.cpp Wed Jan 13 17:59:19 2016
>> @@ -944,37 +944,31 @@ bool llvm::EliminateDuplicatePHINodes(Ba
>> static unsigned enforceKnownAlignment(Value *V, unsigned Align,
>> unsigned PrefAlign,
>> const DataLayout &DL) {
>> + assert(PrefAlign > Align);
>> +
>> V = V->stripPointerCasts();
>>
>> if (AllocaInst *AI = dyn_cast<AllocaInst>(V)) {
>> + assert(AI->getAlignment() <= Align);
>> // If the preferred alignment is greater than the natural stack
>> alignment
>> // then don't round up. This avoids dynamic stack realignment.
>> if (DL.exceedsNaturalStackAlignment(PrefAlign))
>> return Align;
>> - // If there is a requested alignment and if this is an alloca, round
>> up.
>> - if (AI->getAlignment() >= PrefAlign)
>> - return AI->getAlignment();
>> AI->setAlignment(PrefAlign);
>> return PrefAlign;
>> }
>>
>> if (auto *GO = dyn_cast<GlobalObject>(V)) {
>> + assert(GO->getAlignment() <= Align);
>> // If there is a large requested alignment and we can, bump up the
>> alignment
>> // of the global. If the memory we set aside for the global may not
>> be the
>> // memory used by the final program then it is impossible for us to
>> reliably
>> // enforce the preferred alignment.
>> - if (!GO->isStrongDefinitionForLinker())
>> + if (!GO->canIncreaseAlignment())
>> return Align;
>>
>> - if (GO->getAlignment() >= PrefAlign)
>> - return GO->getAlignment();
>> - // We can only increase the alignment of the global if it has no
>> alignment
>> - // specified or if it is not assigned a section. If it is assigned a
>> - // section, the global could be densely packed with other objects in
>> the
>> - // section, increasing the alignment could cause padding issues.
>> - if (!GO->hasSection() || GO->getAlignment() == 0)
>> - GO->setAlignment(PrefAlign);
>> - return GO->getAlignment();
>> + GO->setAlignment(PrefAlign);
>> + return PrefAlign;
>> }
>>
>> return Align;
>>
>> Modified: llvm/trunk/test/CodeGen/ARM/memfunc.ll
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM/memfunc.ll?rev=257719&r1=257718&r2=257719&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/test/CodeGen/ARM/memfunc.ll (original)
>> +++ llvm/trunk/test/CodeGen/ARM/memfunc.ll Wed Jan 13 17:59:19 2016
>> @@ -1,10 +1,10 @@
>> -; RUN: llc < %s -mtriple=armv7-apple-ios -disable-post-ra -o - |
>> FileCheck %s --check-prefix=CHECK-IOS
>> -; RUN: llc < %s -mtriple=thumbv7m-none-macho -disable-post-ra -o - |
>> FileCheck %s --check-prefix=CHECK-DARWIN
>> -; RUN: llc < %s -mtriple=arm-none-eabi -disable-post-ra -o - | FileCheck
>> %s --check-prefix=CHECK-EABI
>> -; RUN: llc < %s -mtriple=arm-none-eabihf -disable-post-ra -o - |
>> FileCheck %s --check-prefix=CHECK-EABI
>> -; RUN: llc < %s -mtriple=arm-none-androideabi -disable-post-ra -o - |
>> FileCheck %s --check-prefix=CHECK-EABI
>> -; RUN: llc < %s -mtriple=arm-none-gnueabi -disable-post-ra -o - |
>> FileCheck %s --check-prefix=CHECK-GNUEABI
>> -; RUN: llc < %s -mtriple=arm-none-gnueabihf -disable-post-ra -o - |
>> FileCheck %s --check-prefix=CHECK-GNUEABI
>> +; RUN: llc < %s -mtriple=armv7-apple-ios -disable-post-ra -o - |
>> FileCheck %s --check-prefix=CHECK-IOS --check-prefix=CHECK
>> +; RUN: llc < %s -mtriple=thumbv7m-none-macho -disable-post-ra -o - |
>> FileCheck %s --check-prefix=CHECK-DARWIN --check-prefix=CHECK
>> +; RUN: llc < %s -mtriple=arm-none-eabi -disable-post-ra -o - | FileCheck
>> %s --check-prefix=CHECK-EABI --check-prefix=CHECK
>> +; RUN: llc < %s -mtriple=arm-none-eabihf -disable-post-ra -o - |
>> FileCheck %s --check-prefix=CHECK-EABI --check-prefix=CHECK
>> +; RUN: llc < %s -mtriple=arm-none-androideabi -disable-post-ra -o - |
>> FileCheck %s --check-prefix=CHECK-EABI --check-prefix=CHECK
>> +; RUN: llc < %s -mtriple=arm-none-gnueabi -disable-post-ra -o - |
>> FileCheck %s --check-prefix=CHECK-GNUEABI --check-prefix=CHECK
>> +; RUN: llc < %s -mtriple=arm-none-gnueabihf -disable-post-ra -o - |
>> FileCheck %s --check-prefix=CHECK-GNUEABI --check-prefix=CHECK
>>
>> define void @f1(i8* %dest, i8* %src) {
>> entry:
>> @@ -402,8 +402,8 @@ entry:
>> ; CHECK: arr1:
>> ; CHECK-IOS: .align 3
>> ; CHECK-DARWIN: .align 2
>> -; CHECK-EABI: .align 2
>> -; CHECK-GNUEABI: .align 2
>> +; CHECK-EABI-NOT: .align
>> +; CHECK-GNUEABI-NOT: .align
>> ; CHECK: arr2:
>> ; CHECK: {{\.section.+foo,bar}}
>> ; CHECK-NOT: .align
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160114/1c35aee9/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: InstCombineAndOrXor-4af03f.zip
Type: application/zip
Size: 1367442 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160114/1c35aee9/attachment-0001.zip>
More information about the llvm-commits
mailing list