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