From patchwork Thu Sep 12 02:11:49 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alan Modra X-Patchwork-Id: 274401 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id F3F4A2C0200 for ; Thu, 12 Sep 2013 12:12:09 +1000 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; q=dns; s=default; b=JXDdpdcsgHTlzhTa3 dZe9jtzWqEt+gorx1cyTmy3OwKM1cPI6otFR9jMISBdCUMizgdji0/DreHbNxc1H a6q7t257wJa/nbe7CfsdRsEzo7Ph3O/HIivi6CYYrzoJmC86m93DwDu3jDBod1WY 29gVGWn7TeHS1R4LYoNbucN4P4= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; s=default; bh=PJYmAtg5MjSKQqT29p0fID8 XuH8=; b=c3+2PClRBbS8mgq7PHJ5KrpVPogL7UUq3ROdoPLj1jiQfyA0jYFV8wQ zZ/546s97BfoSWEXKWtNa9l9C1WnQICS78yWtF1pP3yenxvyeOULRXO4d3Qskewh cWL1e1YTOo6sqr94enm0ugEt2IJtFlbnCQZPepGuYgjGYMe2oUjs= Received: (qmail 7527 invoked by alias); 12 Sep 2013 02:11:58 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 7498 invoked by uid 89); 12 Sep 2013 02:11:58 -0000 Received: from mail-pb0-f50.google.com (HELO mail-pb0-f50.google.com) (209.85.160.50) 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=unavailable version=3.3.2 X-Spam-User: qpsmtpd, 2 recipients X-HELO: mail-pb0-f50.google.com Received: by mail-pb0-f50.google.com with SMTP id uo5so9979544pbc.9 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 11:41:49 +0930 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-Disposition: inline In-Reply-To: <1378904143.3730.46.camel@gnopaine> User-Agent: Mutt/1.5.21 (2010-09-15) X-IsSubscribed: yes 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. */