From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 62795 invoked by alias); 23 May 2016 07:53:34 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 62769 invoked by uid 89); 23 May 2016 07:53:33 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.2 required=5.0 tests=AWL,BAYES_05,KAM_LAZY_DOMAIN_SECURITY,RCVD_IN_DNSWL_NONE autolearn=no version=3.3.2 spammy=activated, mainstream, H*r:sk:polaris, rescue X-HELO: smtp.eu.adacore.com Received: from mel.act-europe.fr (HELO smtp.eu.adacore.com) (194.98.77.210) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Mon, 23 May 2016 07:53:31 +0000 Received: from localhost (localhost [127.0.0.1]) by filtered-smtp.eu.adacore.com (Postfix) with ESMTP id 88B648130A; Mon, 23 May 2016 09:53:27 +0200 (CEST) Received: from smtp.eu.adacore.com ([127.0.0.1]) by localhost (smtp.eu.adacore.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 5VZgRaxE26GG; Mon, 23 May 2016 09:53:27 +0200 (CEST) Received: from polaris.localnet (bon31-6-88-161-99-133.fbx.proxad.net [88.161.99.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.eu.adacore.com (Postfix) with ESMTPSA id 57E1C812FF; Mon, 23 May 2016 09:53:27 +0200 (CEST) From: Eric Botcazou To: Jeff Law Cc: gcc-patches@gcc.gnu.org, vogt@linux.vnet.ibm.com, Andreas Krebbel Subject: Re: [RFA] Minor cleanup to allocate_dynamic_stack_space Date: Mon, 23 May 2016 07:53:00 -0000 Message-ID: <3756023.qahbefmBic@polaris> User-Agent: KMail/4.14.10 (Linux/3.16.7-35-desktop; KDE/4.14.9; x86_64; ; ) In-Reply-To: <3374a93a-0f91-4e1d-c5fd-de27abb7f5ed@redhat.com> References: <20160429221242.GA2205@linux.vnet.ibm.com> <1549397.HYKLGkHiIc@polaris> <3374a93a-0f91-4e1d-c5fd-de27abb7f5ed@redhat.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-SW-Source: 2016-05/txt/msg01748.txt.bz2 > I pondered that as a direction, but was scared off by the overall > fragility of this code when I looked back through the old BZs. I > figured cleanup preserving existing behavior was the first step. Setting aside the flag_split_stack stuff, the must_align logic is clear enough and quite localized. > We can go the other way if you prefer. It just makes reasoning about > how this code is supposed to work harder. On second thoughts, there might be a real issue with STACK_DYNAMIC_OFFSET that is papered over by the STACK_POINTER_OFFSET glitch. The big comment explains why, if STACK_DYNAMIC_OFFSET is defined, we need to preventively align, and that it's defined if ACCUMULATE_OUTGOING_ARGS is activated. That's not true for STACK_DYNAMIC_OFFSET overall, but that's indeed true in function.c where STACK_DYNAMIC_OFFSET is forcibly defined. Given that ACCUMULATE_OUTGOING_ARGS is activated by default on most mainstream platforms, it probably makes sense to always preventively align. From there on, I think that what would be left to discuss is the amount of alignment we can start from (extra_align in the current code); maybe BITS_PER_UNIT is also the only sensible value in the end. So I think that your patch is OK if you rescue the main comment, for example: /* We will need to ensure that the address we return is aligned to REQUIRED_ALIGN. At this point in the compilation, we don't always know the final value of the STACK_DYNAMIC_OFFSET used in function.c (it might depend on the size of the outgoing parameter lists, for example), so we must preventively align the value. We leave space in SIZE for the hole that might result from the alignment operation. */ -- Eric Botcazou