This is the mail archive of the
libffi-discuss@sourceware.org
mailing list for the libffi project.
Re: [PATCH, PowerPC] Fix PR57949 (ABI alignment issue)
- From: Alan Modra <amodra at gmail dot com>
- To: Bill Schmidt <wschmidt at linux dot vnet dot ibm dot com>
- Cc: gcc-patches at gcc dot gnu dot org, libffi-discuss at sourceware dot org, dje at gcc dot gnu dot org
- Date: Thu, 12 Sep 2013 11:41:49 +0930
- Subject: Re: [PATCH, PowerPC] Fix PR57949 (ABI alignment issue)
- Authentication-results: sourceware.org; auth=none
- References: <1376494321 dot 17852 dot 17 dot camel at oc8801110288 dot ibm dot com> <20130911113845 dot GF2643 at bubble dot grove dot modra dot org> <1378904143 dot 3730 dot 46 dot camel at gnopaine>
On Wed, Sep 11, 2013 at 07:55:43AM -0500, Bill Schmidt wrote:
> On Wed, 2013-09-11 at 21:08 +0930, Alan Modra wrote:
> > On Wed, Aug 14, 2013 at 10:32:01AM -0500, Bill Schmidt wrote:
> > > This fixes a long-standing problem with GCC's implementation of the
> > > PPC64 ELF ABI. If a structure contains a member requiring 128-bit
> > > alignment, and that structure is passed as a parameter, the parameter
> > > currently receives only 64-bit alignment. This is an error, and is
> > > incompatible with correct code generated by the IBM XL compilers.
> >
> > This caused multiple failures in the libffi testsuite:
> > libffi.call/cls_align_longdouble.c
> > libffi.call/cls_align_longdouble_split.c
> > libffi.call/cls_align_longdouble_split2.c
> > libffi.call/nested_struct5.c
> >
> > Fixed by making the same alignment adjustment in libffi to structures
> > passed by value. Bill, I think your patch needs to go on all active
> > gcc branches as otherwise we'll need different versions of libffi for
> > the next gcc releases.
>
> Hm, the libffi case is unfortunate. :(
>
> The alternative is to leave libffi alone, and require code that calls
> these interfaces with "bad" structs passed by value to be built using
> -mcompat-align-parm, which was provided for such compatibility issues.
> Hopefully there is a small number of cases where this can happen, and
> this could be documented with libffi and gcc. What do you think?
We have precedent for compiling libffi based on gcc preprocessor
defines, eg. __NO_FPRS__, so here's a way of making upstream libffi
compatible with the various versions of gcc out there. I've taken the
condition under which we align aggregates from
rs6000_function_arg_boundary, and defined a macro with a value of the
maximum alignment.
Bootstrapped and regression tested powerpc64-linux. OK for mainline?
gcc/
* config/rs6000/rs6000-c.c (rs6000_cpu_cpp_builtins): Define
__STRUCT_PARM_ALIGN__.
libffi/
* src/powerpc/ffi.c (ffi_prep_args64): Align FFI_TYPE_STRUCT.
(ffi_closure_helper_LINUX64): Likewise.
Index: gcc/config/rs6000/rs6000-c.c
===================================================================
--- gcc/config/rs6000/rs6000-c.c (revision 202428)
+++ gcc/config/rs6000/rs6000-c.c (working copy)
@@ -473,6 +473,12 @@ rs6000_cpu_cpp_builtins (cpp_reader *pfile)
if (TARGET_SOFT_FLOAT || !TARGET_FPRS)
builtin_define ("__NO_FPRS__");
+ /* Whether aggregates passed by value are aligned to a 16 byte boundary
+ if their alignment is 16 bytes or larger. */
+ if ((TARGET_MACHO && rs6000_darwin64_abi)
+ || (DEFAULT_ABI == ABI_AIX && !rs6000_compat_align_parm))
+ builtin_define ("__STRUCT_PARM_ALIGN__=16");
+
/* Generate defines for Xilinx FPU. */
if (rs6000_xilinx_fpu)
{
Index: libffi/src/powerpc/ffi.c
===================================================================
--- libffi/src/powerpc/ffi.c (revision 202428)
+++ libffi/src/powerpc/ffi.c (working copy)
@@ -462,6 +462,9 @@ ffi_prep_args64 (extended_cif *ecif, unsigned long
double **d;
} p_argv;
unsigned long gprvalue;
+#ifdef __STRUCT_PARM_ALIGN__
+ unsigned long align;
+#endif
stacktop.c = (char *) stack + bytes;
gpr_base.ul = stacktop.ul - ASM_NEEDS_REGISTERS64 - NUM_GPR_ARG_REGISTERS64;
@@ -532,6 +535,12 @@ ffi_prep_args64 (extended_cif *ecif, unsigned long
#endif
case FFI_TYPE_STRUCT:
+#ifdef __STRUCT_PARM_ALIGN__
+ align = (*ptr)->alignment;
+ if (align > __STRUCT_PARM_ALIGN__)
+ align = __STRUCT_PARM_ALIGN__;
+ next_arg.ul = ALIGN (next_arg.ul, align);
+#endif
words = ((*ptr)->size + 7) / 8;
if (next_arg.ul >= gpr_base.ul && next_arg.ul + words > gpr_end.ul)
{
@@ -1349,6 +1358,9 @@ ffi_closure_helper_LINUX64 (ffi_closure *closure,
long i, avn;
ffi_cif *cif;
ffi_dblfl *end_pfr = pfr + NUM_FPR_ARG_REGISTERS64;
+#ifdef __STRUCT_PARM_ALIGN__
+ unsigned long align;
+#endif
cif = closure->cif;
avalue = alloca (cif->nargs * sizeof (void *));
@@ -1399,6 +1411,12 @@ ffi_closure_helper_LINUX64 (ffi_closure *closure,
break;
case FFI_TYPE_STRUCT:
+#ifdef __STRUCT_PARM_ALIGN__
+ align = arg_types[i]->alignment;
+ if (align > __STRUCT_PARM_ALIGN__)
+ align = __STRUCT_PARM_ALIGN__;
+ pst = ALIGN (pst, align);
+#endif
#ifndef __LITTLE_ENDIAN__
/* Structures with size less than eight bytes are passed
left-padded. */
--
Alan Modra
Australia Development Lab, IBM