This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH, rs6000, v2] Fix aggregate alignment ABI issue


Uli,

Bootstrap on AIX has been broken because of the generic GCC latent bug
that seems to be popping up on various architectures.  I could not
bootstrap GCC from 11 Jul to 14 Jul.  I was able to  bootstrap on 15
Jul. I want to ensure that bootstrap is functioning consistently
before approving these and the other ABI changes so that I can confirm
that the patches don't accidentally affect AIX.

Thanks, David


On Mon, Jul 14, 2014 at 2:54 PM, Ulrich Weigand <uweigand@de.ibm.com> wrote:
> Hello,
>
> this patch updates the prior version:
> https://gcc.gnu.org/ml/gcc-patches/2014-07/msg00634.html
> to add a -Wpsabi note as discussed.
>
> Note that the warning triggers whenever type is encountered whose alignment
> *requirement* changes due to this patch.  In some cases, this does not actually
> change the ABI of a given function call since the argument happened to be
> already aligned for the higher requirement as well.
>
> Except for a few cases in the ABI compat suite (as expected), this note
> triggers in three test cases:
>
> In the ELFv1 ABI only:
> gcc.c-torture/compile/20040709-1.c:7:1: note: the ABI of passing aggregates with 16-byte alignment has changed in GCC 4.10
> union foo {
>   long double ld;
> } bar;
>
> Note that this union counts as homogeneous aggregate according to
> the ELFv2 ABI, so it is treated the same before and after this patch
> (i.e. aligned to 8 bytes).
>
> In the ELFv1 ABI, however, this union does *not* count as a single-element
> float struct (because its mode is TImode, not TFmode, since common code
> does the latter only for single-element structs, not unions).  Therefore,
> this union used to be 8-byte aligned but is now 16-byte aligned.
>
> In both the ELFv1 and ELFv2 ABIs:
> gcc.dg/pr32293.c:46:1: note: the ABI of passing aggregates with 16-byte alignment has changed in GCC 4.10
> typedef
> __attribute__ ((aligned(16)))
>      struct {
>        UINT64 w[2];
>      } UINT128;
>
> g++.dg/abi/param2.C:9:1: note: the ABI of passing aggregates with 16-byte alignment has changed in GCC 4.10
> struct S { union {} a; } __attribute__((aligned));
>
> In both cases, we have a struct of size 16 bytes (in the second case due to
> alignment padding) and alignment requirement 16 bytes, which is recognized
> as TImode and not BLKmode by common code, and thus used to be 8-byte aligned
> but is now 16-byte aligned.
>
> The tests still PASS since the dg-test infrastructure prunes all informational
> messages from the GCC output.
>
> Based on the discussion after the previous patch submission, the patch
> leaves the Darwin ABI unchanged.
>
> Tested on powerpc64-linux and powerpc64le-linux.
>
> OK for mainline?
>
> Bye,
> Ulrich
>
>
> gcc/ChangeLog:
>
>         * config/rs6000/rs6000.c (rs6000_function_arg_boundary): In the AIX
>         and ELFv2 ABI, do not use the "mode == BLKmode" check to test for
>         aggregate types.  Instead, *all* aggregate types, except for single-
>         element or homogeneous float/vector aggregates, are quadword-aligned
>         if required by their type alignment.  Issue -Wpsabi note when a type
>         is now treated differently than before.
>
> gcc/testsuite/ChangLog:
>
>         * gcc.target/powerpc/ppc64-abi-warn-2.c: New test.
>
>
> Index: gcc/config/rs6000/rs6000.c
> ===================================================================
> --- gcc/config/rs6000/rs6000.c  (revision 212147)
> +++ gcc/config/rs6000/rs6000.c  (working copy)
> @@ -9182,14 +9182,48 @@
>            || (type && TREE_CODE (type) == VECTOR_TYPE
>                && int_size_in_bytes (type) >= 16))
>      return 128;
> -  else if (((TARGET_MACHO && rs6000_darwin64_abi)
> -           || DEFAULT_ABI == ABI_ELFv2
> -            || (DEFAULT_ABI == ABI_AIX && !rs6000_compat_align_parm))
> -          && mode == BLKmode
> -          && type && TYPE_ALIGN (type) > 64)
> +
> +  /* Aggregate types that need > 8 byte alignment are quadword-aligned
> +     in the parameter area in the ELFv2 ABI, and in the AIX ABI unless
> +     -mcompat-align-parm is used.  */
> +  if (((DEFAULT_ABI == ABI_AIX && !rs6000_compat_align_parm)
> +       || DEFAULT_ABI == ABI_ELFv2)
> +      && type && TYPE_ALIGN (type) > 64)
> +    {
> +      /* "Aggregate" means any AGGREGATE_TYPE except for single-element
> +         or homogeneous float/vector aggregates here.  We already handled
> +         vector aggregates above, but still need to check for float here. */
> +      bool aggregate_p = (AGGREGATE_TYPE_P (type)
> +                         && !SCALAR_FLOAT_MODE_P (elt_mode));
> +
> +      /* We used to check for BLKmode instead of the above aggregate type
> +        check.  Warn when this results in any difference to the ABI.  */
> +      if (aggregate_p != (mode == BLKmode))
> +       {
> +         static bool warned;
> +         if (!warned && warn_psabi)
> +           {
> +             warned = true;
> +             inform (input_location,
> +                     "the ABI of passing aggregates with %d-byte alignment"
> +                     " has changed in GCC 4.10",
> +                     (int) TYPE_ALIGN (type) / BITS_PER_UNIT);
> +           }
> +       }
> +
> +      if (aggregate_p)
> +       return 128;
> +    }
> +
> +  /* Similar for the Darwin64 ABI.  Note that for historical reasons we
> +     implement the "aggregate type" check as a BLKmode check here; this
> +     means certain aggregate types are in fact not aligned.  */
> +  if (TARGET_MACHO && rs6000_darwin64_abi
> +      && mode == BLKmode
> +      && type && TYPE_ALIGN (type) > 64)
>      return 128;
> -  else
> -    return PARM_BOUNDARY;
> +
> +  return PARM_BOUNDARY;
>  }
>
>  /* The offset in words to the start of the parameter save area.  */
> Index: gcc/testsuite/gcc.target/powerpc/ppc64-abi-warn-2.c
> ===================================================================
> --- /dev/null
> +++ gcc/testsuite/gcc.target/powerpc/ppc64-abi-warn-2.c (revision 0)
> @@ -0,0 +1,11 @@
> +/* { dg-do compile { target { powerpc*-*-linux* && lp64 } } } */
> +
> +struct test
> +  {
> +    long a __attribute__((aligned (16)));
> +  };
> +
> +void test (struct test a) /* { dg-message "note: the ABI of passing aggregates with 16-byte alignment has changed" } */
> +{
> +}
> +
> --
>   Dr. Ulrich Weigand
>   GNU/Linux compilers and toolchain
>   Ulrich.Weigand@de.ibm.com
>


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]