From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7529 invoked by alias); 12 Sep 2013 02:11:58 -0000 Mailing-List: contact libffi-discuss-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libffi-discuss-owner@sourceware.org Received: (qmail 7495 invoked by uid 89); 12 Sep 2013 02:11:58 -0000 Received: from mail-pd0-f170.google.com (HELO mail-pd0-f170.google.com) (209.85.192.170) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Thu, 12 Sep 2013 02:11:58 +0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.5 required=5.0 tests=ALL_TRUSTED,AWL,BAYES_00,FREEMAIL_FROM autolearn=ham version=3.3.2 X-HELO: mail-pd0-f170.google.com Received: by mail-pd0-f170.google.com with SMTP id x10so10094977pdj.1 for ; Wed, 11 Sep 2013 19:11:55 -0700 (PDT) X-Received: by 10.68.90.1 with SMTP id bs1mr337676pbb.169.1378951915646; Wed, 11 Sep 2013 19:11:55 -0700 (PDT) Received: from bubble.grove.modra.org ([101.166.26.37]) by mx.google.com with ESMTPSA id gh9sm1124929pbc.40.1969.12.31.16.00.00 (version=TLSv1.1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Wed, 11 Sep 2013 19:11:54 -0700 (PDT) Received: by bubble.grove.modra.org (Postfix, from userid 1000) id D8A65EA0077; Thu, 12 Sep 2013 11:41:49 +0930 (CST) Date: Thu, 12 Sep 2013 02:11:00 -0000 From: Alan Modra To: Bill Schmidt Cc: gcc-patches@gcc.gnu.org, libffi-discuss@sourceware.org, dje@gcc.gnu.org Subject: Re: [PATCH, PowerPC] Fix PR57949 (ABI alignment issue) Message-ID: <20130912021149.GG2643@bubble.grove.modra.org> Mail-Followup-To: Bill Schmidt , gcc-patches@gcc.gnu.org, libffi-discuss@sourceware.org, dje@gcc.gnu.org References: <1376494321.17852.17.camel@oc8801110288.ibm.com> <20130911113845.GF2643@bubble.grove.modra.org> <1378904143.3730.46.camel@gnopaine> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1378904143.3730.46.camel@gnopaine> User-Agent: Mutt/1.5.21 (2010-09-15) X-SW-Source: 2013/txt/msg00162.txt.bz2 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