[rs6000] Fix alignment of non-Altivec vector struct fields

Hello,

running the ABI compatibility test suite against another compiler showed
strange effects caused by code in ADJUST_FIELD_ALIGN on rs6000:

#define ADJUST_FIELD_ALIGN(FIELD, COMPUTED) \
  ((TARGET_ALTIVEC && TREE_CODE (TREE_TYPE (FIELD)) == VECTOR_TYPE)    \
   ? 128                                                               \
   : (TARGET_64BIT                                                     \
      && TARGET_ALIGN_NATURAL == 0                                     \
      && TYPE_MODE (strip_array_types (TREE_TYPE (FIELD))) == DFmode)  \
   ? MIN ((COMPUTED), 32)						\
   : (COMPUTED))

The first test for VECTOR_TYPE if TARGET_ALTIVEC causes the trouble.
This has been present (in slight variations) ever since Altivec support
was first merged by Aldy Hernandez back in 2001:
https://gcc.gnu.org/ml/gcc-cvs/2001-11/msg00148.html

Note that at this time, both ADJUST_FIELD_ALIGN and ROUND_TYPE_ALIGN
contained that check.  The check was later removed from ROUND_TYPE_ALIGN
in the following patch intended to fix PR 17961:
https://gcc.gnu.org/ml/gcc-patches/2005-06/msg00920.html

The check has no effect in most usual cases anyway, since Altivec vectors
already carry a default alignment of 16 bytes.  However, the check *does*
affect non-Altivec vectors created via attribute((vector_size)) with a
size != 16.  When any of those is used as element of a struct, GCC on
PowerPC will ignore the normal alignment and always force 16 byte alignment.
(An explicit attribute((aligned)) however, is still respected.)  This
behavior seems to have been unintended.

In off-line discussions, we came to the conclusion that this check
should have been removed from ADJUST_FIELD_ALIGN at the same time
as it was removed from ROUND_TYPE_ALIGN.  This patch implements this.

Note that the check has been copied over time into the instances of
ADJUST_FIELD_ALIGN in sysv4.h, linux64.h, and freebsd64.h.  For
consistency, the patch removes the check in all these places.

Tested on powerpc64-linux and powerp64le-linux; built a cross-cc1
for powerpc64-freebsd6.

OK for mainline?
[ It may also be useful to backport to the currently open branches,
but the bug was already present in many releases that are no longer
maintained ... ]

Bye,
Ulrich

On Wed, Jul 9, 2014 at 12:06 PM, Ulrich Weigand <uweigand@de.ibm.com> wrote:
> Hello,
>
> running the ABI compatibility test suite against another compiler showed
> strange effects caused by code in ADJUST_FIELD_ALIGN on rs6000:
>
> #define ADJUST_FIELD_ALIGN(FIELD, COMPUTED) \
>   ((TARGET_ALTIVEC && TREE_CODE (TREE_TYPE (FIELD)) == VECTOR_TYPE)    \
>    ? 128                                                               \
>    : (TARGET_64BIT                                                     \
>       && TARGET_ALIGN_NATURAL == 0                                     \
>       && TYPE_MODE (strip_array_types (TREE_TYPE (FIELD))) == DFmode)  \
>    ? MIN ((COMPUTED), 32)                                               \
>    : (COMPUTED))
>
> The first test for VECTOR_TYPE if TARGET_ALTIVEC causes the trouble.
> This has been present (in slight variations) ever since Altivec support
> was first merged by Aldy Hernandez back in 2001:
> https://gcc.gnu.org/ml/gcc-cvs/2001-11/msg00148.html
>
> Note that at this time, both ADJUST_FIELD_ALIGN and ROUND_TYPE_ALIGN
> contained that check.  The check was later removed from ROUND_TYPE_ALIGN
> in the following patch intended to fix PR 17961:
> https://gcc.gnu.org/ml/gcc-patches/2005-06/msg00920.html
>
> The check has no effect in most usual cases anyway, since Altivec vectors
> already carry a default alignment of 16 bytes.  However, the check *does*
> affect non-Altivec vectors created via attribute((vector_size)) with a
> size != 16.  When any of those is used as element of a struct, GCC on
> PowerPC will ignore the normal alignment and always force 16 byte alignment.
> (An explicit attribute((aligned)) however, is still respected.)  This
> behavior seems to have been unintended.
>
> In off-line discussions, we came to the conclusion that this check
> should have been removed from ADJUST_FIELD_ALIGN at the same time
> as it was removed from ROUND_TYPE_ALIGN.  This patch implements this.
>
> Note that the check has been copied over time into the instances of
> ADJUST_FIELD_ALIGN in sysv4.h, linux64.h, and freebsd64.h.  For
> consistency, the patch removes the check in all these places.
>
> Tested on powerpc64-linux and powerp64le-linux; built a cross-cc1
> for powerpc64-freebsd6.
>
> OK for mainline?
> [ It may also be useful to backport to the currently open branches,
> but the bug was already present in many releases that are no longer
> maintained ... ]

This is okay with me.

Do you want to explicitly ask rth if he has any further insight?

Thanks, David