On Mon, May 20, 2013 at 10:51:16PM -0600, Sandra Loosemore wrote:
> On 05/20/2013 04:14 PM, Jakub Jelinek wrote:
> >
> >Isn't this ABI incompatible change? See http://gcc.gnu.org/PR56564
> >for more info (yeah, different target, but looks exactly the same issue).
> >If what this new DATA_ALIGNMENT value is optimization rather than an ABI
> >requirement, then you'll hit the same issue.
>
> Hmmmm. The originally-posted version of the patch from 2011 (which
> is the one we've been using locally since then) only changed
> LOCAL_ALIGNMENT, which is presumably safe. I only extended it to
> DATA_ALIGNMENT because of the previous review comment that it ought
> to apply to all arrays and not just local ones. :-S I did wonder
> about ABI issues when I saw the extra stuff the generic vector
> increase_alignment pass does (in vect_can_force_dr_alignment_p), but
> the documentation for DATA_ALIGNMENT in tm.texi seems to suggest
> it's fine to use it for this kind of optimization purposes.
The right solution is to have separate data_alignment macro/target hooks
for the ABI mandated alignment and extra optimization on top of that.
We can align all the decls to the extra optimization level (e.g. at least
runtime alignment check will likely figure out it is properly aligned and
result in faster code), and for variables that surely bind to the locally
defined var, we can also assume that alignment (in MEM_ALIGN, DECL_ALIGN
etc.), for others we have to assume only the ABI mandated alignment.
Honza, are you working on a fix? Once this is split, there is no problem
to apply your patch to the extra optimization path, while keep the current
state (or even lower it if ABI guarantees less than that, at least on
x86_64) for the ABI mandated path.
Jakub
