<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">This generally looks fine.</div><div class="gmail_quote"><br></div><div class="gmail_quote">On Mon, Mar 30, 2015 at 11:20 AM, Ulrich Weigand <span dir="ltr"><<a href="mailto:Ulrich.Weigand@de.ibm.com" target="_blank">Ulrich.Weigand@de.ibm.com</a>></span> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><br>
<br>
Hello,<br>
<br>
the GCC construct __attribute__((aligned)) is defined to set alignment<br>
to "the default alignment for the target architecture" according to<br>
the GCC documentation:<br>
<br>
The default alignment is sufficient for all scalar types, but may not be<br>
enough for all vector types on a target that supports vector operations.<br>
The default alignment is fixed for a particular target ABI.<br></blockquote><div><br></div><div>I don't think this means to define a general term "default alignment"; instead, I think this is just shorthand for "default alignment for __attribute__((aligned))". I'd prefer if you used a bit more of a specific name for this field. Something like DefaultAlignForAttributeAligned would be suitably clear.</div><div><div><br></div></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">clang currently hard-codes an alignment of 16 bytes for that construct,<br>
which is correct on some platforms (including X86), but wrong on others<br>
(including SystemZ). Since this value is ABI-relevant, it is important<br>
to get correct for compatibility purposes.<br>
<br>
The following patch adds a new TargetInfo member "DefaultAlign" that<br>
targets can set to the appropriate default __attribute__((aligned)) value.<br>
<br>
Note that I'm deliberately *not* using the existing "SuitableAlign"<br>
value, which is used to set the pre-defined macro __BIGGEST_ALIGNMENT__,<br>
since those two values may not be the same on all platforms. In fact,<br>
on X86, __attribute__((aligned)) always uses 16-byte alignment, while<br>
__BIGGEST_ALIGNMENT__ may be larger if AVX-2 or AVX-512 are supported.<br>
(This is actually not yet correctly implemented in clang either.)<br>
<br>
The patch provides a value for DefaultAlign only for SystemZ, and leaves<br>
the default for all other targets at 16, which means no visible change<br>
in behavior on all other targets. (The value is still wrong for some<br>
other targets, but I'd prefer to leave it to the target maintainers for<br>
those platforms to fix and test.)<br>
<br>
(See attached file: clang-align-attribute)<br>
<br>
<br>
Mit freundlichen Gruessen / Best Regards<br>
<br>
Ulrich Weigand<br>
<br>
--<br>
Dr. Ulrich Weigand | Phone: <a href="tel:%2B49-7031%2F16-3727" value="+497031163727">+49-7031/16-3727</a><br>
STSM, GNU/Linux compilers and toolchain<br>
IBM Deutschland Research & Development GmbH<br>
Vorsitzende des Aufsichtsrats: Martina Koederitz | Geschäftsführung: Dirk<br>
Wittkopp<br>
Sitz der Gesellschaft: Böblingen | Registergericht: Amtsgericht<br>
Stuttgart, HRB 243294<br>_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
<br></blockquote></div><br></div></div>