From patchwork Mon Feb 11 01:05:11 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans-Peter Nilsson X-Patchwork-Id: 1039516 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-495758-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=axis.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="E/WcM+mo"; dkim-atps=neutral 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 43ySJQ0lHhz9s1l for ; Mon, 11 Feb 2019 12:05:25 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :message-id:from:to:cc:in-reply-to:subject:mime-version :content-type:content-transfer-encoding; q=dns; s=default; b=tb0 Yk6E2tGszURwZ4YOdgnpqShrzi31SOoVBSxh4MHW1Sk9CV2ELD8bSfRslY4FVwXW P7kHf+ttt/HqFfKkaJfOrh7tTPPvcnk82LFYhkGrW79ECWWqPKkCCKJwzKG91Cay uK+kImqRHfcdO4j2rhCYQixXwv5M899tXUY4k3qg= 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 :message-id:from:to:cc:in-reply-to:subject:mime-version :content-type:content-transfer-encoding; s=default; bh=iZyGCuk8+ 15rpWSXT1MW2Bhj3/c=; b=E/WcM+mokMM7SCQ0RXwOpO6eECQHaL71jCl9ocodc ZThSTxpOf8Dp1ZLXuBMQYPcxzoFO44Qu6Dz/V5dVzAI0B05NJKlSFP/XXkXVNukW OV3AoxzJ3Z3abjnT5gcGOvy4k//cL8thtPStMWoKVRvYhTOigLq3Vcz0WsvslR9B YE= Received: (qmail 95303 invoked by alias); 11 Feb 2019 01:05:19 -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 95292 invoked by uid 89); 11 Feb 2019 01:05:18 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-11.9 required=5.0 tests=BAYES_00, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=H*Ad:U*ebotcazou, forming, HCC:D*com, bearing X-HELO: bastet.se.axis.com Received: from bastet.se.axis.com (HELO bastet.se.axis.com) (195.60.68.11) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 11 Feb 2019 01:05:16 +0000 Received: from localhost (localhost [127.0.0.1]) by bastet.se.axis.com (Postfix) with ESMTP id 751211828A; Mon, 11 Feb 2019 02:05:14 +0100 (CET) X-Axis-User: NO X-Axis-NonUser: YES Received: from bastet.se.axis.com ([IPv6:::ffff:127.0.0.1]) by localhost (bastet.se.axis.com [::ffff:127.0.0.1]) (amavisd-new, port 10024) with LMTP id wwu16S3Wn_qM; Mon, 11 Feb 2019 02:05:13 +0100 (CET) Received: from boulder03.se.axis.com (boulder03.se.axis.com [10.0.8.17]) by bastet.se.axis.com (Postfix) with ESMTPS id 4CA0C181DE; Mon, 11 Feb 2019 02:05:13 +0100 (CET) Received: from boulder03.se.axis.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 1E35D1E064; Mon, 11 Feb 2019 02:05:13 +0100 (CET) Received: from boulder03.se.axis.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 1148A1E060; Mon, 11 Feb 2019 02:05:13 +0100 (CET) Received: from seth.se.axis.com (unknown [10.0.2.172]) by boulder03.se.axis.com (Postfix) with ESMTP; Mon, 11 Feb 2019 02:05:13 +0100 (CET) Received: from ignucius.se.axis.com (ignucius.se.axis.com [10.88.21.50]) by seth.se.axis.com (Postfix) with ESMTP id 019B12BE3; Mon, 11 Feb 2019 02:05:13 +0100 (CET) Received: from ignucius.se.axis.com (localhost [127.0.0.1]) by ignucius.se.axis.com (8.12.8p1/8.12.8/Debian-2woody1) with ESMTP id x1B15CDl021614; Mon, 11 Feb 2019 02:05:12 +0100 Received: (from hp@localhost) by ignucius.se.axis.com (8.12.8p1/8.12.8/Debian-2woody1) id x1B15BPC021610; Mon, 11 Feb 2019 02:05:11 +0100 Date: Mon, 11 Feb 2019 02:05:11 +0100 Message-Id: <201902110105.x1B15BPC021610@ignucius.se.axis.com> From: Hans-Peter Nilsson To: jakub@redhat.com CC: rguenther@suse.de, law@redhat.com, ebotcazou@adacore.com, renlin.li@foss.arm.com, gcc-patches@gcc.gnu.org In-reply-to: <20190109230601.GR30353@tucnak> (message from Jakub Jelinek on Thu, 10 Jan 2019 00:06:01 +0100) Subject: Follow-up-fix to "[PATCH] Move PR84877 fix elsewhere (PR bootstrap/88450)" MIME-Version: 1.0 > Date: Thu, 10 Jan 2019 00:06:01 +0100 > From: Jakub Jelinek > 2019-01-09 Jakub Jelinek > > PR middle-end/84877 > PR bootstrap/88450 > * function.c (assign_stack_local_1): Revert the 2018-11-21 changes. > (assign_parm_setup_block): Do the argument slot realignment here > instead. When this was committed as r267812, results for cris-elf went from regress-8 to regress-160 (now at r268749, regress-154). I analyzed one of the simpler cases, gcc.c-torture/execute/930126-1.c at -O0. It looks like the code added in r267812 doesn't handle targets where MAX_SUPPORTED_STACK_ALIGNMENT is less than BITS_PER_WORD (by necessity, non-STRICT_ALIGNMENT targets); the bug is in not adding necessary alignment to the allocated space. cris-elf is a non-STRICT_ALIGNMENT target and MAX_SUPPORTED_STACK_ALIGNMENT is defaulted from STACK_BOUNDARY, 16 (bits). The only difference with r267812 is that before, 10 bytes (excluding 4 for the saved frame-pointer-address) was allocated for the stack-frame of the function f and at r267812, 8 bytes; the literally only differences are two instructions, one when allocating stack and one when forming the pointer to the stack-parameter. The sizeof the actual object is 5 bytes, but padded to 8 bytes due to copying from incoming registers (which is IMO a valid reason, maybe not worthwhile to improve). Please remember that size-padding is different from alignment-padding. The setting of DECL_ALIGN (parm) to BITS_PER_WORD (originating at r94104) is wrong too, but not fatal. I'll fix that in a follow-up change and let's pretend for the moment that there's a valid reason for aligning "parm" in this case (for example, a user-provided overalignment). At function entry, after saving the frame-pointer-register, the stack-pointer is 0x3dfffece (both versions). The parameter- pointer-align instructions align this to a multiple of 4 for both versions, and this of course fails as there's no padding with r267812. The actual failure is due to overwriting the saved frame-pointer-register, with the wrong value then used in main to verify the result. To fix the r267812 incorrectness we need to use the *stored* size, i.e. that which will be stored into using register-sized writes. It's seems like the bug is just a typo, so the fix is as simple as follows. Note the usage of "diff -U 10" to show that size_stored is used in the "then"-arm. Regtested on cris-elf, where it "introduces" gcc.dg/pr84877.c but on inspection that's a known bug with a bit of hairy churn, reverts and all. See the PR. This patch has no bearing on that PR; this is for incoming parameters, and PR84877 is for (not doing) alignment of pass-by-reference parameters for outgoing parameters. Still, maybe the solution to that PR should have code like in that the context below...which I see you already noted, in the patch-message to which this is a reply. Also regtested on x86_64-pc-linux-gnu (without regressions) together with the followup-change. Ok to commit? gcc/ChangeLog: * function.c (assign_parm_setup_block): Use the stored size, not the passed size, when allocating stack-space, also for a parameter with alignment larger than MAX_SUPPORTED_STACK_ALIGNMENT. both arms of the containing "if".) brgds, H-P --- gcc/function.c.orig Sun Jan 20 19:20:16 2019 +++ gcc/function.c Sat Feb 9 00:53:17 2019 @@ -2907,23 +2907,23 @@ assign_parm_setup_block (struct assign_p } data->stack_parm = NULL; } size = int_size_in_bytes (data->passed_type); size_stored = CEIL_ROUND (size, UNITS_PER_WORD); if (stack_parm == 0) { SET_DECL_ALIGN (parm, MAX (DECL_ALIGN (parm), BITS_PER_WORD)); if (DECL_ALIGN (parm) > MAX_SUPPORTED_STACK_ALIGNMENT) { - rtx allocsize = gen_int_mode (size, Pmode); + rtx allocsize = gen_int_mode (size_stored, Pmode); get_dynamic_stack_size (&allocsize, 0, DECL_ALIGN (parm), NULL); stack_parm = assign_stack_local (BLKmode, UINTVAL (allocsize), MAX_SUPPORTED_STACK_ALIGNMENT); rtx addr = align_dynamic_address (XEXP (stack_parm, 0), DECL_ALIGN (parm)); mark_reg_pointer (addr, DECL_ALIGN (parm)); stack_parm = gen_rtx_MEM (GET_MODE (stack_parm), addr); MEM_NOTRAP_P (stack_parm) = 1; } else stack_parm = assign_stack_local (BLKmode, size_stored,