From patchwork Wed Nov 25 14:54:35 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kyrylo Tkachov X-Patchwork-Id: 57305 Delivered-To: patch@linaro.org Received: by 10.112.155.196 with SMTP id vy4csp2785480lbb; Wed, 25 Nov 2015 06:54:55 -0800 (PST) X-Received: by 10.98.67.201 with SMTP id l70mr31670822pfi.29.1448463295015; Wed, 25 Nov 2015 06:54:55 -0800 (PST) Return-Path: Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id yf4si1608968pab.34.2015.11.25.06.54.54 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 25 Nov 2015 06:54:54 -0800 (PST) Received-SPF: pass (google.com: domain of gcc-patches-return-415367-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) client-ip=209.132.180.131; Authentication-Results: mx.google.com; spf=pass (google.com: domain of gcc-patches-return-415367-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) smtp.mailfrom=gcc-patches-return-415367-patch=linaro.org@gcc.gnu.org; dkim=pass header.i=@gcc.gnu.org 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:date:from:mime-version:to:subject:references :in-reply-to:content-type; q=dns; s=default; b=DzvrFKI++Ci4EGkPE b/WsJBkxyws6wAFeawusGWBBjNDTp9eBXLE/Y1RdeLAKvODcmepPYLeEy4KcsuzC FByl4oJix7Bce55IDm9eH7o5g82hTy4OCIf4t/OOS0tv9g2k/3TNPcrwBApbjDl3 16gPBV4Herx7ayGwu4j3XuzFAw= 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:date:from:mime-version:to:subject:references :in-reply-to:content-type; s=default; bh=IugzIx3NbflvLoh4HVn+aSR 9Vl0=; b=Cd4vF0iYZkJACGSERTUP6c5fYm5A6bagOnW7iKa9Ube79eipx7esh4j mDx0SMGBjjC4GEVplEtkeVPy7wleYe/AVm8GwucEqEYoVV1/Dn908IK4mGGiIOGz NG8MWIzlTG1vlJ5Y4JvNV6Df2NvuwDLmQNrxNainFNqo9mQpy/pA= Received: (qmail 90516 invoked by alias); 25 Nov 2015 14:54:41 -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 90391 invoked by uid 89); 25 Nov 2015 14:54:41 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.7 required=5.0 tests=AWL, BAYES_00, SPF_PASS autolearn=ham version=3.3.2 X-HELO: eu-smtp-delivery-143.mimecast.com Received: from eu-smtp-delivery-143.mimecast.com (HELO eu-smtp-delivery-143.mimecast.com) (146.101.78.143) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 25 Nov 2015 14:54:40 +0000 Received: from cam-owa1.Emea.Arm.com (fw-tnat.cambridge.arm.com [217.140.96.140]) by eu-smtp-1.mimecast.com with ESMTP id uk-mta-35-qEMx9jjRQIqvHOl3cQp8dA-1; Wed, 25 Nov 2015 14:54:35 +0000 Received: from [10.2.206.200] ([10.1.2.79]) by cam-owa1.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.3959); Wed, 25 Nov 2015 14:54:34 +0000 Message-ID: <5655CBAB.9000800@arm.com> Date: Wed, 25 Nov 2015 14:54:35 +0000 From: Kyrill Tkachov User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 MIME-Version: 1.0 To: Bernd Schmidt , GCC Patches Subject: Re: [PATCH][calls.c] PR rtl-optimization/67226: Take into account pretend_args_size when checking stack offsets for sibcall optimisation References: <56547E60.6010702@arm.com> <5655AC64.3070601@redhat.com> <5655AD83.2000603@redhat.com> <5655C171.9030204@arm.com> <5655C1B4.4080605@redhat.com> <5655C23B.1090309@arm.com> In-Reply-To: <5655C23B.1090309@arm.com> X-MC-Unique: qEMx9jjRQIqvHOl3cQp8dA-1 X-IsSubscribed: yes On 25/11/15 14:14, Kyrill Tkachov wrote: > > On 25/11/15 14:12, Bernd Schmidt wrote: >> On 11/25/2015 03:10 PM, Kyrill Tkachov wrote: >> >>> What should we do when we don't have STACK_GROWS_DOWNWARD? Do we need to >>> write: >>> if (STACK_GROWS_DOWNWARD) >>> i -= crtl->args.pretend_args_size; >>> else >>> i += crtl->args.pretend_args_size; >> >> I think so. I mean, this should mirror this code here, right? >> >> /* The argument block when performing a sibling call is the >> incoming argument block. */ >> if (pass == 0) >> { >> argblock = crtl->args.internal_arg_pointer; >> if (STACK_GROWS_DOWNWARD) >> argblock >> = plus_constant (Pmode, argblock, crtl->args.pretend_args_size); >> else >> argblock >> = plus_constant (Pmode, argblock, -crtl->args.pretend_args_size); >> > > Yes, you're right. > I'll send out an updated version of the patch shortly. > And here it is. This fixes the bug on arm and tests on arm look ok. I've kicked off bootstraps and tests on arm, aarch64 and x86_64. Ok for trunk if they come back clean? The first version of the patch that I posted should be equivalent to this one on those targets (as you noted). I'll prepare a backport for GCC 5 and 4.9 as it occurs there as well. We'll have to use the #ifdef check for STACK_GROWS_DOWNWARD on those branches... Thanks, Kyrill 2015-11-25 Kyrylo Tkachov Bernd Schmidt PR rtl-optimization/67226 * calls.c (store_one_arg): Take into account crtl->args.pretend_args_size when checking for overlap between arg->value and argblock + arg->locate.offset during sibcall optimization. 2015-11-25 Kyrylo Tkachov PR rtl-optimization/67226 * gcc.c-torture/execute/pr67226.c: New test. > Kyrill > >> >> Bernd >> > commit cec1dd7490b3a5931aefac5aeafdb1380af9437f Author: Kyrylo Tkachov Date: Mon Nov 23 13:19:57 2015 +0000 PR rtl-optimization/67226: Take into account pretend_args_size when checking stack offsets for sibcall optimisation diff --git a/gcc/calls.c b/gcc/calls.c index b56556a..6cbe019 100644 --- a/gcc/calls.c +++ b/gcc/calls.c @@ -4939,6 +4939,13 @@ store_one_arg (struct arg_data *arg, rtx argblock, int flags, if (XEXP (x, 0) != crtl->args.internal_arg_pointer) i = INTVAL (XEXP (XEXP (x, 0), 1)); + /* arg.locate doesn't contain the pretend_args_size offset, + it's part of argblock. Ensure we don't count it in I. */ + if (STACK_GROWS_DOWNWARD) + i -= crtl->args.pretend_args_size; + else + i += crtl->args.pretend_args_size; + /* expand_call should ensure this. */ gcc_assert (!arg->locate.offset.var && arg->locate.size.var == 0 diff --git a/gcc/testsuite/gcc.c-torture/execute/pr67226.c b/gcc/testsuite/gcc.c-torture/execute/pr67226.c new file mode 100644 index 0000000..c533496 --- /dev/null +++ b/gcc/testsuite/gcc.c-torture/execute/pr67226.c @@ -0,0 +1,42 @@ +struct assembly_operand +{ + int type, value, symtype, symflags, marker; +}; + +struct assembly_operand to_input, from_input; + +void __attribute__ ((__noinline__, __noclone__)) +assemblez_1 (int internal_number, struct assembly_operand o1) +{ + if (o1.type != from_input.type) + __builtin_abort (); +} + +void __attribute__ ((__noinline__, __noclone__)) +t0 (struct assembly_operand to, struct assembly_operand from) +{ + if (to.value == 0) + assemblez_1 (32, from); + else + __builtin_abort (); +} + +int +main (void) +{ + to_input.value = 0; + to_input.type = 1; + to_input.symtype = 2; + to_input.symflags = 3; + to_input.marker = 4; + + from_input.value = 5; + from_input.type = 6; + from_input.symtype = 7; + from_input.symflags = 8; + from_input.marker = 9; + + t0 (to_input, from_input); + + return 0; +}