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] Fix aggregate alignment ABI issue


On Wed, Jul 9, 2014 at 12:04 PM, Ulrich Weigand <uweigand@de.ibm.com> wrote:
> Hello,
>
> last year, Bill added a patch to address PR 57949 by aligning aggregates
> requiring at least 128-bit alignment at a quadword boundary in the
> parameter save area:
> https://gcc.gnu.org/ml/gcc-patches/2013-08/msg00803.html
>
> Unfortunately, to implement this check, Bill's patch used a pre-existing
> piece of code originally used only on Darwin, which uses a "mode == BLKmode"
> check to test for aggregate types.
>
> However, GCC may sometimes choose non-BLKmode modes to represent aggregate
> types.  One case the single-element float/vector aggregates; that's OK,
> because those are handled separately by the ABI anyway.
>
> But there are more cases: many structures get some integer mode simply
> because their size happen to be 1, 2, 4, 8, or 16 bytes.  The precise
> rules *which* aggregates GCC uses such a mode for are intricate, and
> even differ slightly between types created by the C vs. C++ front-ends.
>
> This normally doesn't matter since the mode used to back an aggregate
> type only matters for internal code generation (basically, whether GCC
> may use a register to hold a local variable of that type, or whether
> they must all go to memory).
>
> Due to this check in rs6000_function_arg_boundary, however, those GCC
> internal details have now leaked into the public ABI.  We have thought
> of simply accepting that ABI as the de-facto ABI now and documenting
> it, but that turned out to be too fragile; it is hard to precisely
> describe the mode selection in a way that it can be reliably implemented
> by another (non-GCC based) compiler.
>
> After various off-line discussions, we came to the conclusion that the
> best way is to fix the GCC implementation to actually align *all* aggregate
> types, not just those backed by BLKmode.  [ The exception remain single-
> element (or ELFv2 homogeneous) float/vector aggregates, which are handled
> as before: float is doubleword aligned, vector is quadword aligned. ]
>
> This change does break the ABI in certain cases.  However, we hope that
> this is acceptable because:
>
> - The change only affects rare cases: passing a struct by value that is
>    * not a float/vector special case, and
>    * has a size of 1, 2, 4, 8, or 16 bytes, and
>    * has an alignment requirement of 16 bytes or more
>   [ Not *all* these cases will see change, but all cases that change
>   will share these properties.  ]
>
> - This aspect of the ABI already changed recently with Bill's patch,
>   and the current version hasn't seen very widespread use yet.
>
> Note that patch below only changes the ABI for the AIX/ELFv1 and ELFv2
> cases; the Darwin ABI (which shared the same problem) is left as-is.
> It's up to the Darwin maintainers whether they prefer to change as well
> or rather keep everything as it has been on Darwin for a long time.
>
> Tested on powerpc64-linux and powerpc64le-linux.
>
> OK for mainline?
> [ The patch should then also go into the 4.8 and 4.9 branches for
> consistency. ]
>
> Bye,
> Ulrich
>
>
> 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.

Okay.

I copied the Darwin maintainers and active testers so that they are
explicitly aware of the ABI issue. They can decide if they want to fix
the ABI alignment issue on Darwin.

Thanks, David


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