<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div><div class="">On Jan 3, 2016, at 10:56 AM, Dimitry Andric <<a href="mailto:dimitry@andric.com" class="">dimitry@andric.com</a>> wrote:</div><blockquote type="cite" class=""><div class=""><meta http-equiv="Content-Type" content="text/html charset=us-ascii" class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">On 01 Jan 2016, at 00:07, James Y Knight <<a href="mailto:jyknight@google.com" class="">jyknight@google.com</a>> wrote:<div class=""><blockquote type="cite" class=""><div class=""><meta http-equiv="Content-Type" content="text/html charset=us-ascii" class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class=""><div class="">On Dec 31, 2015, at 5:03 AM, Dimitry Andric via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org" class="">cfe-dev@lists.llvm.org</a>> wrote:</div><blockquote type="cite" class=""><div class="">This thread unfortunately died out without any resolution, so we are now<br class="">nearing 3.8 branching, and the problem is still not fixed. E.g., the<br class="">only custom patch we have now left in FreeBSD's version of llvm (which<br class="">we would really like to get rid of!) is the reversal of r240144: it is a<br class="">pretty gross workaround, but it works for now.<br class=""><br class="">But is there any way that we can get this resolved before 3.8.0 ships,<br class="">please? :-)<br class=""></div></blockquote></div><div class=""><br class=""></div><div class="">I believe the summary of the thread is that LLVM has a fundamentally unsound transformation: automatically increasing the required alignment of an externally visible global variable.</div></div></div></blockquote><div class=""><br class=""></div><div class="">Yes, and also that it ignores (or silently drops) an attribute added by the developer, e,g.:</div><div class=""><br class=""></div><div class=""><font face="Menlo" class="">__attribute__((__aligned__(8))) char cout[sizeof(ostream)];</font></div></div></div></div></blockquote><div><blockquote type="cite" class=""><br class=""></blockquote></div></div><div><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class=""><div class="">gets transformed into:</div><div class=""><br class=""></div><div class=""><font face="Menlo" class="">@cout = global [24 x i8] zeroinitializer, align 16</font></div></div></div></div></blockquote><div><br class=""></div><div>No -- that part is perfectly fine. A variable that is aligned at 16-bytes is also aligned at 8-bytes, it's not ignoring the attribute.</div><div><br class=""></div><div>Normally, increasing the alignment of a variable beyond what is requested is okay. The reason there's a problem in this instance is due to the way that shared objects work in ELF -- and that's a problem regardless of whether the alignment was user-specified or not.</div><div><br class=""><blockquote type="cite" class=""><div class="" style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div class=""></div></div></blockquote></div><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class=""><blockquote type="cite" class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class="">In enforceKnownAlignment in lib/Transforms/Utils/Local.cpp, it checks isStrongDefinitionForLinker() purportedly to ensure that the "memory we set aside for the global" is "the memory used by the final program.". However, even if said predicate returns true, that condition is NOT guaranteed, at least on ELF platforms, because of copy relocations.</div></div></blockquote><div class=""><br class=""></div><div class="">Ah yes, I get what you mean. When isStrongDefinitionForLinker() returns false in this case, the code falls through to the default global object handling code below there, which attempts to increase the alignment the the preferred alignment. One thing I don't completely understand though, is the comment slightly below it:</div><div class=""><br class=""></div><div class=""><div class=""><font face="Menlo" class=""> // We can only increase the alignment of the global if it has no alignment</font></div><div class=""><font face="Menlo" class=""> // specified or if it is not assigned a section. If it is assigned a</font></div><div class=""><font face="Menlo" class=""> // section, the global could be densely packed with other objects in the</font></div><div class=""><font face="Menlo" class=""> // section, increasing the alignment could cause padding issues.</font></div><div class=""><font face="Menlo" class=""> if (!GO->hasSection() || GO->getAlignment() == 0) {</font></div><div class=""><br class=""></div><div class="">However, the object certainly does have a specified alignment. So how is that supposed to work, then? :)</div></div></div></div></div></blockquote><div><br class=""></div>It's checking if you have specified alignment **AND** are in a section. That part seems ok, too.</div><div><br class=""><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class=""><blockquote type="cite" class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class="">(There's also very similar-looking alignment-fixing code in CodeGenPrepare::optimizeCallInst, too, I'm not sure why it's duplicated there.)</div><div class=""><br class=""></div><div class="">Basically, we need to not modify the alignment if:</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;" class=""><div style="margin: 0px; font-size: 11px;" class=""><span style="font-variant-ligatures: no-common-ligatures; color: #34bbc7" class=""><b class="">if</b></span> (Subtarget->isTargetELF() &&</div><div style="margin: 0px; font-size: 11px;" class=""> TM.getRelocationModel() == <span style="font-variant-ligatures: no-common-ligatures; color: #d53bd3" class="">Reloc</span>::PIC_ &&</div><div style="margin: 0px; font-size: 11px;" class=""> GV->hasDefaultVisibility() && !GV->hasLocalLinkage())</div><div style="margin: 0px; font-size: 11px;" class=""><br class=""></div><div class=""><span style="font-family: Helvetica;" class="">The above code snippet appears in X86FastISel.cpp, and equivalent-but-not-identical code appears in several other places, to determine whether a PLT load for the variable is required.</span></div><div class=""><span style="font-family: Helvetica;" class=""><br class=""></span></div><div class=""><span style="font-family: Helvetica;" class="">I'm not sure if isStrongDefinitionForLinker itself ought to be taking that into account, or if there should be a different new predicate for that check. isReallyActuallyTrulyStrongDefinitionForLinker()? :)</span></div></div></div></blockquote></div><div class=""><br class=""></div><div class="">It is probably safest to use a new predicate, since isStrongDefinitionForLinker() is called all over the place, also in target-dependent code. Or does this really apply to everything with ELF? I mean, is the above set of conditions callable in the context of isStrongDefinitionForLinker()?</div></div></div></blockquote></div><div><br class=""></div><div class=""><br class=""></div></body></html>