From patchwork Wed Jul 9 16:04:52 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ulrich Weigand X-Patchwork-Id: 368259 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.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 9ECDB14011B for ; Thu, 10 Jul 2014 02:05:10 +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 :message-id:subject:to:date:from:cc:mime-version:content-type :content-transfer-encoding; q=dns; s=default; b=yGx3arT7CqK5e/Rh 2iCtiCdLrqPbN+YYuW7GwOlAwB8W947NOX2x+ZOuGm3mCuY7mVKblS7FGSGqij5K AIUDkW0IojxBZ2GkqkjF4rZXqbUjQDVMGjf2b0NS3X4kmHWElkN7lyGoodGj6sdy Az56/mSBWH6Y1AeWs6V2iSQ6MBQ= 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 :message-id:subject:to:date:from:cc:mime-version:content-type :content-transfer-encoding; s=default; bh=N8hVFR3uUuRHRmOj/JbCCP n0ICY=; b=OPIBsdhhviiQOGWUe3GfH43w0osMdP0/Tgr6QQLiQ1/Z/+YKkxw3MX yCcrJqD+T1W6sMyv1up6c963uDf4Gmz2kbHSsTzOG3RrpPWuuZTSD9Lu+Qu+6phT zh7FhmbJwTvHoe/s2siPXlMOG/bJn9J9eW/PSRrD65SCiWfZdhzPQ= Received: (qmail 12371 invoked by alias); 9 Jul 2014 16:05:03 -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 12352 invoked by uid 89); 9 Jul 2014 16:05:02 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.1 required=5.0 tests=AWL, BAYES_00, MSGID_FROM_MTA_HEADER, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 X-HELO: e06smtp11.uk.ibm.com Received: from e06smtp11.uk.ibm.com (HELO e06smtp11.uk.ibm.com) (195.75.94.107) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Wed, 09 Jul 2014 16:04:59 +0000 Received: from /spool/local by e06smtp11.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 9 Jul 2014 17:04:56 +0100 Received: from d06dlp02.portsmouth.uk.ibm.com (9.149.20.14) by e06smtp11.uk.ibm.com (192.168.101.141) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Wed, 9 Jul 2014 17:04:54 +0100 Received: from b06cxnps3074.portsmouth.uk.ibm.com (d06relay09.portsmouth.uk.ibm.com [9.149.109.194]) by d06dlp02.portsmouth.uk.ibm.com (Postfix) with ESMTP id 776F72190041 for ; Wed, 9 Jul 2014 17:04:40 +0100 (BST) Received: from d06av02.portsmouth.uk.ibm.com (d06av02.portsmouth.uk.ibm.com [9.149.37.228]) by b06cxnps3074.portsmouth.uk.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id s69G4rGO35913774 for ; Wed, 9 Jul 2014 16:04:53 GMT Received: from d06av02.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av02.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id s69G4r2Q020484 for ; Wed, 9 Jul 2014 10:04:53 -0600 Received: from tuxmaker.boeblingen.de.ibm.com (tuxmaker.boeblingen.de.ibm.com [9.152.85.9]) by d06av02.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVin) with SMTP id s69G4qnj020440; Wed, 9 Jul 2014 10:04:52 -0600 Message-Id: <201407091604.s69G4qnj020440@d06av02.portsmouth.uk.ibm.com> Received: by tuxmaker.boeblingen.de.ibm.com (sSMTP sendmail emulation); Wed, 09 Jul 2014 18:04:52 +0200 Subject: [PATCH, rs6000] Fix aggregate alignment ABI issue To: gcc-patches@gcc.gnu.org Date: Wed, 9 Jul 2014 18:04:52 +0200 (CEST) From: "Ulrich Weigand" Cc: dje.gcc@gmail.com, wschmidt@linux.vnet.ibm.com MIME-Version: 1.0 X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 14070916-5024-0000-0000-000000A69794 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. Index: gcc/config/rs6000/rs6000.c =================================================================== --- gcc/config/rs6000/rs6000.c (revision 212147) +++ gcc/config/rs6000/rs6000.c (working copy) @@ -9182,9 +9182,20 @@ || (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)) + /* 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. This does not apply to single-element + (or homogeneous) float/vector aggregrates. We already checked for + vector above; we still need to check for float here. */ + else if (((DEFAULT_ABI == ABI_AIX && !rs6000_compat_align_parm) + || DEFAULT_ABI == ABI_ELFv2) + && type && AGGREGATE_TYPE_P (type) && TYPE_ALIGN (type) > 64 + && !SCALAR_FLOAT_MODE_P (elt_mode)) + 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. */ + else if (TARGET_MACHO && rs6000_darwin64_abi && mode == BLKmode && type && TYPE_ALIGN (type) > 64) return 128;