From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by sourceware.org (Postfix) with ESMTP id C88953858C98 for ; Thu, 29 Feb 2024 14:10:48 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org C88953858C98 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=arm.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org C88953858C98 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=217.140.110.172 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1709215859; cv=none; b=SgZNAGZhRx/0meO2XNt+RnJi/cJMd3k0OOevSzmLgQg13vzYn1Du1Ytt46EPv26ENc2MLnTR8yuRB4zTdBBSZEAUwC92CsNF2KGkNW94JBgeVOMwOr4hFqFutQ+FHURO5ZJmzdskg5dMu8d2Ez0U8aEvlKcWK68RvUAyHh5nG4M= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1709215859; c=relaxed/simple; bh=41nxfnBddc4Ve/0S37MpkyA8umLPeoyuHPy31gkAVug=; h=Message-ID:Date:MIME-Version:Subject:To:From; b=hZuCfKJ0JSh+VU2vgETIbiP94Dkbe1/a0oCMkdN5pRpiJDMs2pH6tXUONSDCqeKCexA1AquEA3uODT2dR+2xVJPMK9ug5sKMz+AKp4hSoNzEno5siIGqumCiQIMoCxGPkcViBXQfqirGd4Ic6l/WUMLJFh9hozRL521kvndOirI= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id D2A491FB; Thu, 29 Feb 2024 06:11:26 -0800 (PST) Received: from [10.2.78.54] (e120077-lin.cambridge.arm.com [10.2.78.54]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id DE1413F6C4; Thu, 29 Feb 2024 06:10:46 -0800 (PST) Message-ID: <23c7c873-1954-43b2-80b8-714455eaaf2b@arm.com> Date: Thu, 29 Feb 2024 14:10:45 +0000 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] calls: Fix up TYPE_NO_NAMED_ARGS_STDARG_P handling [PR107453] Content-Language: en-GB To: Jakub Jelinek Cc: "Joseph S. Myers" , Richard Biener , Jeff Law , gcc-patches@gcc.gnu.org, =?UTF-8?Q?Torbj=C3=B6rn_SVENSSON?= , oliva@adacore.com References: <45ac2d54-21df-486c-a085-0a6c1f37a323@arm.com> From: "Richard Earnshaw (lists)" In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-3491.6 required=5.0 tests=BAYES_00,KAM_DMARC_NONE,KAM_DMARC_STATUS,KAM_LAZY_DOMAIN_SECURITY,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 27/02/2024 17:25, Jakub Jelinek wrote: > On Tue, Feb 27, 2024 at 04:41:32PM +0000, Richard Earnshaw wrote: >>> 2023-01-09 Jakub Jelinek >>> >>> PR target/107453 >>> * calls.cc (expand_call): For calls with >>> TYPE_NO_NAMED_ARGS_STDARG_P (funtype) use zero for n_named_args. >>> Formatting fix. >> >> This one has been festering for a while; both Alexandre and Torbjorn have attempted to fix it recently, but I'm not sure either is really right... >> >> On Arm this is causing all anonymous arguments to be passed on the stack, >> which is incorrect per the ABI. On a target that uses >> 'pretend_outgoing_vararg_named', why is it correct to set n_named_args to >> zero? Is it enough to guard both the statements you've added with >> !targetm.calls.pretend_outgoing_args_named? > > I'm afraid I haven't heard of that target hook before. > All I was doing with that change was fixing a regression reported in the PR > for ppc64le/sparc/nvptx/loongarch at least. > > The TYPE_NO_NAMED_ARGS_STDARG_P functions (C23 fns like void foo (...) {}) > have NULL type_arg_types, so the list_length (type_arg_types) isn't done for > it, but it should be handled as if it was non-NULL but list length was 0. > > So, for the > if (type_arg_types != 0) > n_named_args > = (list_length (type_arg_types) > /* Count the struct value address, if it is passed as a parm. */ > + structure_value_addr_parm); > else if (TYPE_NO_NAMED_ARGS_STDARG_P (funtype)) > n_named_args = 0; > else > /* If we know nothing, treat all args as named. */ > n_named_args = num_actuals; > case, I think guarding it by any target hooks is wrong, although > I guess it should have been > n_named_args = structure_value_addr_parm; > instead of > n_named_args = 0; > > For the second > if (type_arg_types != 0 > && targetm.calls.strict_argument_naming (args_so_far)) > ; > else if (type_arg_types != 0 > && ! targetm.calls.pretend_outgoing_varargs_named (args_so_far)) > /* Don't include the last named arg. */ > --n_named_args; > else if (TYPE_NO_NAMED_ARGS_STDARG_P (funtype)) > n_named_args = 0; > else > /* Treat all args as named. */ > n_named_args = num_actuals; > bet (but no testing done, don't even know which targets return what for > those hooks) we should treat those as if type_arg_types was non-NULL > with 0 elements in the list, except the --n_named_args doesn't make sense > because that would decrease it to -1. > So perhaps > if ((type_arg_types != 0 || TYPE_NO_NAMED_ARGS_STDARG_P (funtype)) > && targetm.calls.strict_argument_naming (args_so_far)) > ; > else if (type_arg_types != 0 > && ! targetm.calls.pretend_outgoing_varargs_named (args_so_far)) > /* Don't include the last named arg. */ > --n_named_args; > else if (TYPE_NO_NAMED_ARGS_STDARG_P (funtype) > && ! targetm.calls.pretend_outgoing_varargs_named (args_so_far))) > ; > else > /* Treat all args as named. */ > n_named_args = num_actuals; I tried the above on arm, aarch64 and x86_64 and that seems fine, including the new testcase you added. R. > > (or n_named_args = 0; instead of ; before the final else? Dunno). > I guess we need some testsuite coverage for caller/callee ABI match of > struct S { char p[64]; }; > struct S foo (...); > > Jakub >