[PATCH] Fix PR79256

[Uros Bizjak]

On Mon, Jan 30, 2017 at 11:56 AM, Richard Biener <rguenther@suse.de> wrote:
> On Mon, 30 Jan 2017, Jakub Jelinek wrote:
>
>> On Mon, Jan 30, 2017 at 11:47:51AM +0100, Richard Biener wrote:
>> > On Mon, 30 Jan 2017, Uros Bizjak wrote:
>> >
>> > > > 2017-01-30  Richard Biener  <rguenther@suse.de>
>> > > >
>> > > > PR target/79277
>> > > > * config/i386/i386-modes.def: Align DFmode properly.
>> > >
>> > > Index: gcc/config/i386/i386-modes.def
>> > > ===================================================================
>> > > --- gcc/config/i386/i386-modes.def (revision 245021)
>> > > +++ gcc/config/i386/i386-modes.def (working copy)
>> > > @@ -33,6 +33,7 @@ ADJUST_FLOAT_FORMAT (XF, (TARGET_128BIT_
>> > >    : &ieee_extended_intel_96_format));
>> > >  ADJUST_BYTESIZE  (XF, TARGET_128BIT_LONG_DOUBLE ? 16 : 12);
>> > >  ADJUST_ALIGNMENT (XF, TARGET_128BIT_LONG_DOUBLE ? 16 : 4);
>> > > +ADJUST_ALIGNMENT (DF, !TARGET_64BIT ? 4 : 8);
>> > >
>> > > Please avoid negative logic, just swap arms of the conditional around.
>> >
>> > It was just meant as an example fix.  I don't think this is appropriate
>> > at this stage nor is it complete.  A full fix would basically make
>> > x86_field_alignment unnecessary which limits most modes alignment
>> > to 32bit (but not vector or 128bit float modes).  And the conditional
>> > needs updating to honor TARGET_ALIGN_DOUBLE.
>>
>> Yeah, at least for GCC 7 that change (quite major ABI change) is too
>> dangerous IMHO.
>
> Yes, __alignof__ (double) would change.  But I think for correctness
> at least __alignof__ (*(double *)p) needs to change.  So another
> way to fix this would be to change the FE to use a differently
> aligned double type in contexts where the default one is wrong
> (but we do have too many == double_type_node checks everywhere...).
>
> Btw, we should eventually change ADJUST_FIELD_ALIGN to take the
> type of the field instead of the FIELD_DECL (as said, only frv.c
> looks at the field, all others just look at its type).

Digging a bit more through the documentation:

`-malign-double'
`-mno-align-double'
     Control whether GCC aligns `double', `long double', and `long
     long' variables on a two word boundary or a one word boundary.
     Aligning `double' variables on a two word boundary will produce
     code that runs somewhat faster on a `Pentium' at the expense of
     more memory.

     On x86-64, `-malign-double' is enabled by default.

     *Warning:* if you use the `-malign-double' switch, structures
     containing the above types will be aligned differently than the
     published application binary interface specifications for the 386
     and will not be binary compatible with structures in code compiled
     without that switch.

The text above implies that on 32bit targets we already have alignment
to a word boundary (4-byte), unless -malign-double is used. So,
proposed i386-modes.def patch seems redundant to me.

Uros.