From patchwork Fri Nov 11 09:33:16 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dominik Vogt X-Patchwork-Id: 693616 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 3tFZVQ3PwPz9sD6 for ; Fri, 11 Nov 2016 20:33:49 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="xh8LIt13"; dkim-atps=neutral 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:reply-to:references:mime-version :content-type:in-reply-to:message-id; q=dns; s=default; b=vQms3T +y+bJk8Gpopzg46KbrBRNnLKi8pJygTWQCMAuzUG7leTzTIfVjVe4nUFOiyKDArk Rz6jjplJ9OFUD5HaWL0BZY874nx2XRwRlvBaqdjPeVI0uiWXRLewGq6RB1VhQZF4 5l2cWapLsW8oZ+4eaHp7wkxU/pOFF0JTISqY0= 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:reply-to:references:mime-version :content-type:in-reply-to:message-id; s=default; bh=oyINSbOrekOp 6tZLS8l10RR+fNA=; b=xh8LIt13O8xHXPtCP1lH7QxxWdcQADGhi4YxargiBBR3 JqkLKAmOQpHjgUThkEZnCWSHoXDPjcSecg92ymJr01uGNzf0yNN0Udq4OeaXVsla XzzfVqJ4KQBLA2D3IBnrB1Dkm9Htp5v0C81nvmCwLlk6XKu2oQkTnSqH12EirMI= Received: (qmail 12000 invoked by alias); 11 Nov 2016 09:33:38 -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 11975 invoked by uid 89); 11 Nov 2016 09:33:36 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=2.2 required=5.0 tests=AWL, BAYES_50, KAM_ASCII_DIVIDERS, KAM_LAZY_DOMAIN_SECURITY, RCVD_IN_DNSWL_LOW, RCVD_IN_SEMBACKSCATTER autolearn=no version=3.3.2 spammy=advances, ciao, 6000, 2.3.0 X-HELO: mx0a-001b2d01.pphosted.com Received: from mx0a-001b2d01.pphosted.com (HELO mx0a-001b2d01.pphosted.com) (148.163.156.1) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 11 Nov 2016 09:33:26 +0000 Received: from pps.filterd (m0098410.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.17/8.16.0.17) with SMTP id uAB9T6gk020460 for ; Fri, 11 Nov 2016 04:33:24 -0500 Received: from e06smtp06.uk.ibm.com (e06smtp06.uk.ibm.com [195.75.94.102]) by mx0a-001b2d01.pphosted.com with ESMTP id 26na6nbeme-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Fri, 11 Nov 2016 04:33:24 -0500 Received: from localhost by e06smtp06.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 11 Nov 2016 09:33:21 -0000 Received: from d06dlp01.portsmouth.uk.ibm.com (9.149.20.13) by e06smtp06.uk.ibm.com (192.168.101.136) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Fri, 11 Nov 2016 09:33:20 -0000 Received: from b06cxnps4075.portsmouth.uk.ibm.com (d06relay12.portsmouth.uk.ibm.com [9.149.109.197]) by d06dlp01.portsmouth.uk.ibm.com (Postfix) with ESMTP id EC56E17D805A for ; Fri, 11 Nov 2016 09:35:41 +0000 (GMT) Received: from d06av02.portsmouth.uk.ibm.com (d06av02.portsmouth.uk.ibm.com [9.149.37.228]) by b06cxnps4075.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id uAB9XJen11534620 for ; Fri, 11 Nov 2016 09:33:19 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 uAB9XJn1018372 for ; Fri, 11 Nov 2016 02:33:19 -0700 Received: from oc5510024614.ibm.com (sig-9-145-7-56.uk.ibm.com [9.145.7.56]) by d06av02.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVin) with ESMTP id uAB9XJwT018369; Fri, 11 Nov 2016 02:33:19 -0700 Received: by oc5510024614.ibm.com (Postfix, from userid 500) id 0FAF618F50; Fri, 11 Nov 2016 10:33:16 +0100 (CET) Date: Fri, 11 Nov 2016 10:33:16 +0100 From: Dominik Vogt To: David Edelsohn Cc: GCC Patches , Andreas Krebbel , Ulrich Weigand , Segher Boessenkool Subject: Re: [PATCH v3] PR77359: Properly align local variables in functions calling alloca. Reply-To: vogt@linux.vnet.ibm.com Mail-Followup-To: vogt@linux.vnet.ibm.com, David Edelsohn , GCC Patches , Andreas Krebbel , Ulrich Weigand , Segher Boessenkool References: <20161103104044.GA8122@linux.vnet.ibm.com> <20161110234702.GA21356@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.20 (2009-12-10) X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 16111109-0024-0000-0000-0000024A7696 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 16111109-0025-0000-0000-000021287221 Message-Id: <20161111093316.GA24679@linux.vnet.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:, , definitions=2016-11-11_02:, , signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1609300000 definitions=main-1611110175 On Fri, Nov 11, 2016 at 12:11:49AM -0500, David Edelsohn wrote: > On Thu, Nov 10, 2016 at 6:47 PM, Dominik Vogt wrote: > > On Thu, Nov 03, 2016 at 11:40:44AM +0100, Dominik Vogt wrote: > >> The attached patch fixes the stack layout problems on AIX and > >> Power as described here: > >> > >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77359 > >> > >> The patch has been bootstrapped on AIX (32 Bit) and bootstrappend > >> and regression tested on Power (biarch). It needs more testing > >> that I cannot do with the hardware available to me. > >> > >> If the patch is good, this one can be re-applied: > >> > >> https://gcc.gnu.org/ml/gcc-patches/2016-07/msg01730.html > >> https://gcc.gnu.org/ml/gcc-patches/2016-08/msg01616.html > > > > So, is this patch in order to be committed? (Assuming that a > > followup patch will clean up the rs6000.h+aix.h quirks.) > > Please also update the ASCII pictures above the rs6000_stack_info() > function in rs6000.c to show / describe the new padding for alignment. Like in the new patch? (Please double check that I got this right in all three cases). Ciao Dominik ^_^ ^_^ diff --git a/gcc/config/rs6000/aix.h b/gcc/config/rs6000/aix.h index b254236..f6eb122 100644 --- a/gcc/config/rs6000/aix.h +++ b/gcc/config/rs6000/aix.h @@ -40,6 +40,41 @@ #undef STACK_BOUNDARY #define STACK_BOUNDARY 128 +/* Offset within stack frame to start allocating local variables at. + If FRAME_GROWS_DOWNWARD, this is the offset to the END of the + first local allocated. Otherwise, it is the offset to the BEGINNING + of the first local allocated. + + On the RS/6000, the frame pointer is the same as the stack pointer, + except for dynamic allocations. So we start after the fixed area and + outgoing parameter area. + + If the function uses dynamic stack space (CALLS_ALLOCA is set), that + space needs to be aligned to STACK_BOUNDARY, i.e. the sum of the + sizes of the fixed area and the parameter area must be a multiple of + STACK_BOUNDARY. */ + +#undef STARTING_FRAME_OFFSET +#define STARTING_FRAME_OFFSET \ + (FRAME_GROWS_DOWNWARD \ + ? 0 \ + : (cfun->calls_alloca \ + ? RS6000_ALIGN (crtl->outgoing_args_size + RS6000_SAVE_AREA, 16) \ + : (RS6000_ALIGN (crtl->outgoing_args_size, 16) + RS6000_SAVE_AREA))) + +/* Offset from the stack pointer register to an item dynamically + allocated on the stack, e.g., by `alloca'. + + The default value for this macro is `STACK_POINTER_OFFSET' plus the + length of the outgoing arguments. The default is correct for most + machines. See `function.c' for details. + + This value must be a multiple of STACK_BOUNDARY (hard coded in + `emit-rtl.c'). */ +#undef STACK_DYNAMIC_OFFSET +#define STACK_DYNAMIC_OFFSET(FUNDECL) \ + RS6000_ALIGN (crtl->outgoing_args_size + STACK_POINTER_OFFSET, 16) + #undef TARGET_IEEEQUAD #define TARGET_IEEEQUAD 0 diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c index f9e4739..4f3b886 100644 --- a/gcc/config/rs6000/rs6000.c +++ b/gcc/config/rs6000/rs6000.c @@ -25781,7 +25781,7 @@ rs6000_savres_strategy (rs6000_stack_t *info, +---------------------------------------+ | saved TOC pointer | 20 40 +---------------------------------------+ - | Parameter save area (P) | 24 48 + | Parameter save area (+padding*) (P) | 24 48 +---------------------------------------+ | Alloca space (A) | 24+P etc. +---------------------------------------+ @@ -25802,6 +25802,9 @@ rs6000_savres_strategy (rs6000_stack_t *info, old SP->| back chain to caller's caller | +---------------------------------------+ + * If the alloca area is present, the parameter save area is + padded so that the former starts 16-byte aligned. + The required alignment for AIX configurations is two words (i.e., 8 or 16 bytes). @@ -25816,7 +25819,7 @@ rs6000_savres_strategy (rs6000_stack_t *info, +---------------------------------------+ | Saved TOC pointer | 24 +---------------------------------------+ - | Parameter save area (P) | 32 + | Parameter save area (+padding*) (P) | 32 +---------------------------------------+ | Alloca space (A) | 32+P +---------------------------------------+ @@ -25833,6 +25836,8 @@ rs6000_savres_strategy (rs6000_stack_t *info, old SP->| back chain to caller's caller | 32+P+A+L+W+Y+G+F +---------------------------------------+ + * If the alloca area is present, the parameter save area is + padded so that the former starts 16-byte aligned. V.4 stack frames look like: @@ -25841,7 +25846,7 @@ rs6000_savres_strategy (rs6000_stack_t *info, +---------------------------------------+ | caller's saved LR | 4 +---------------------------------------+ - | Parameter save area (P) | 8 + | Parameter save area (+padding*) (P) | 8 +---------------------------------------+ | Alloca space (A) | 8+P +---------------------------------------+ @@ -25870,6 +25875,10 @@ rs6000_savres_strategy (rs6000_stack_t *info, old SP->| back chain to caller's caller | +---------------------------------------+ + * If the alloca area is present and the required alignment is + 16 bytes, the parameter save area is padded so that the + alloca area starts 16-byte aligned. + The required alignment for V.4 is 16 bytes, or 8 bytes if -meabi is given. (But note below and in sysv4.h that we require only 8 and may round up the size of our stack frame anyways. The historical @@ -26004,8 +26013,13 @@ rs6000_stack_info (void) info->reg_size = reg_size; info->fixed_size = RS6000_SAVE_AREA; info->vars_size = RS6000_ALIGN (get_frame_size (), 8); - info->parm_size = RS6000_ALIGN (crtl->outgoing_args_size, - TARGET_ALTIVEC ? 16 : 8); + if (cfun->calls_alloca) + info->parm_size = + RS6000_ALIGN (crtl->outgoing_args_size + info->fixed_size, + STACK_BOUNDARY / BITS_PER_UNIT) - info->fixed_size; + else + info->parm_size = RS6000_ALIGN (crtl->outgoing_args_size, + TARGET_ALTIVEC ? 16 : 8); if (FRAME_GROWS_DOWNWARD) info->vars_size += RS6000_ALIGN (info->fixed_size + info->vars_size + info->parm_size, diff --git a/gcc/config/rs6000/rs6000.h b/gcc/config/rs6000/rs6000.h index 4b83abd..afda416 100644 --- a/gcc/config/rs6000/rs6000.h +++ b/gcc/config/rs6000/rs6000.h @@ -1723,25 +1723,35 @@ extern enum reg_class rs6000_constraints[RS6000_CONSTRAINT_MAX]; On the RS/6000, the frame pointer is the same as the stack pointer, except for dynamic allocations. So we start after the fixed area and - outgoing parameter area. */ + outgoing parameter area. + + If the function uses dynamic stack space (CALLS_ALLOCA is set), that + space needs to be aligned to STACK_BOUNDARY, i.e. the sum of the + sizes of the fixed area and the parameter area must be a multiple of + STACK_BOUNDARY. */ #define STARTING_FRAME_OFFSET \ (FRAME_GROWS_DOWNWARD \ ? 0 \ - : (RS6000_ALIGN (crtl->outgoing_args_size, \ - (TARGET_ALTIVEC || TARGET_VSX) ? 16 : 8) \ - + RS6000_SAVE_AREA)) + : (cfun->calls_alloca \ + ? (RS6000_ALIGN (crtl->outgoing_args_size + RS6000_SAVE_AREA, \ + (TARGET_ALTIVEC || TARGET_VSX) ? 16 : 8 )) \ + : (RS6000_ALIGN (crtl->outgoing_args_size, \ + (TARGET_ALTIVEC || TARGET_VSX) ? 16 : 8) \ + + RS6000_SAVE_AREA))) /* Offset from the stack pointer register to an item dynamically allocated on the stack, e.g., by `alloca'. The default value for this macro is `STACK_POINTER_OFFSET' plus the length of the outgoing arguments. The default is correct for most - machines. See `function.c' for details. */ + machines. See `function.c' for details. + + This value must be a multiple of STACK_BOUNDARY (hard coded in + `emit-rtl.c'). */ #define STACK_DYNAMIC_OFFSET(FUNDECL) \ - (RS6000_ALIGN (crtl->outgoing_args_size, \ - (TARGET_ALTIVEC || TARGET_VSX) ? 16 : 8) \ - + (STACK_POINTER_OFFSET)) + RS6000_ALIGN (crtl->outgoing_args_size + STACK_POINTER_OFFSET, \ + (TARGET_ALTIVEC || TARGET_VSX) ? 16 : 8) /* If we generate an insn to push BYTES bytes, this says how many the stack pointer really advances by.