[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:41:15 PST 2016


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/fb938472/attachment.html>


More information about the llvm-commits mailing list