Index: include/llvm/IR/GlobalValue.h =================================================================== --- include/llvm/IR/GlobalValue.h +++ include/llvm/IR/GlobalValue.h @@ -346,6 +346,10 @@ return !(isDeclarationForLinker() || isWeakForLinker()); } + // Returns true if the alignment of the value can be unilateraly + // increased. + bool canIncreaseAlignment() const; + /// This method unlinks 'this' from the containing module, but does not delete /// it. virtual void removeFromParent() = 0; Index: lib/CodeGen/CodeGenPrepare.cpp =================================================================== --- lib/CodeGen/CodeGenPrepare.cpp +++ lib/CodeGen/CodeGenPrepare.cpp @@ -1742,8 +1742,8 @@ // over-aligning global variables that have an explicit section is // forbidden. GlobalVariable *GV; - if ((GV = dyn_cast(Val)) && GV->hasUniqueInitializer() && - !GV->hasSection() && GV->getAlignment() < PrefAlign && + if ((GV = dyn_cast(Val)) && GV->canIncreaseAlignment() && + GV->getAlignment() < PrefAlign && DL->getTypeAllocSize(GV->getType()->getElementType()) >= MinSize + Offset2) GV->setAlignment(PrefAlign); Index: lib/IR/Globals.cpp =================================================================== --- lib/IR/Globals.cpp +++ lib/IR/Globals.cpp @@ -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,52 @@ 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, + // taking precendence over the symbol found in the library. At build + // time, the observed alignment of the variable is copied into the + // executable. (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. + // + // TODO: is it possible to determine if the relocation mode is PIC + // at this point in the code? I think it'd be correct to skip this + // check in non-PIC mode. We'd need something like: + // TargetMachine.getRelocationModel() == Reloc::PIC_; + + // 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 //===----------------------------------------------------------------------===// Index: lib/Transforms/Utils/Local.cpp =================================================================== --- lib/Transforms/Utils/Local.cpp +++ lib/Transforms/Utils/Local.cpp @@ -944,37 +944,31 @@ static unsigned enforceKnownAlignment(Value *V, unsigned Align, unsigned PrefAlign, const DataLayout &DL) { + assert(PrefAlign > Align); + V = V->stripPointerCasts(); if (AllocaInst *AI = dyn_cast(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(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; Index: test/CodeGen/ARM/memfunc.ll =================================================================== --- test/CodeGen/ARM/memfunc.ll +++ test/CodeGen/ARM/memfunc.ll @@ -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 @@ ; 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