From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pf1-x42c.google.com (mail-pf1-x42c.google.com [IPv6:2607:f8b0:4864:20::42c]) by sourceware.org (Postfix) with ESMTPS id C9AB13858C53 for ; Fri, 1 Mar 2024 04:38:54 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org C9AB13858C53 Authentication-Results: sourceware.org; dmarc=pass (p=quarantine dis=none) header.from=adacore.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=adacore.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org C9AB13858C53 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::42c ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1709267937; cv=none; b=ggbC97K5XBHby/GP/dc/5vzc1QOV7GUpiAJssl+LG8YdJCxsGtf1tzSAKnOPyitmr34TH9ZvJ/nOB+hvG9ThqgzNWJwJ586R9V3ow6qcHEf6o5BrVBQR7m4d9MK2Utp4MXDXsv8Z2UUB7VhSMDtGe8hcVrlifc4m2RnRQUiBTns= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1709267937; c=relaxed/simple; bh=ScNBirm7VtNhWhLneEYs7lEbvnhfusrWNOql1Tjd/JQ=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=HsSIOXxXe/9A/ZDpPMQA+9lCduGolJHVpC0JMNIuxWJVwr4U3gkwOGLUuWX/OjN559ryqeaVK3XsE5LptnBGBL4GDV3ezhrtdX6xSyoRegaABRSdDNzCtp8WONvwYDUEClco+37otz6ZfkaqQuEf7IEj1tKelKT7rfmalSl815I= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-pf1-x42c.google.com with SMTP id d2e1a72fcca58-6e5760eeb7aso1456963b3a.1 for ; Thu, 29 Feb 2024 20:38:54 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=adacore.com; s=google; t=1709267933; x=1709872733; darn=gcc.gnu.org; h=content-transfer-encoding:mime-version:user-agent:message-id :in-reply-to:date:references:organization:subject:cc:to:from:from:to :cc:subject:date:message-id:reply-to; bh=dTQex34wEH3wIPp9Z1In5u5SnOWFAd4irxIrLnKRxeI=; b=Z5aUR88W9Lv2Cm2cPDHMVg0gZnLxsS5jK5xOlNl5Pl1m11sKiLQF2P7YmJV0yZJHJQ R3uT6/eGFirG7wG8/jcbWL+1+8+aq0X0MWZeXqzfGzzuJ2tK6Fg1IclA+sH37vP88QcJ ZlGY6uwWiwM51gjsCkieG7JBe+N5BYpasbDZymUXHQ2dqW3m9W+BCmB7bChQ2BVAqbr2 vMHS7SmJwiaszuKSniECFt+yZKS9S2w2pBz5fsvQqqR52iRJIIgQW3yu9sqGNob4S2tM OBRZLBTzHhR39FwoGSwB4Cm174JmOFAkcIVtw/q9PWw/7LRstBy9iMXlEZ8ZsFW8lcSj Orug== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1709267933; x=1709872733; h=content-transfer-encoding:mime-version:user-agent:message-id :in-reply-to:date:references:organization:subject:cc:to:from :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=dTQex34wEH3wIPp9Z1In5u5SnOWFAd4irxIrLnKRxeI=; b=nDVBTeYbnHMc0raTs0epnCHzOj4Hwl+xlpLGeDsPfOREnqiyFSe4GsvwAftIGGF0Yc gC/U9nOnhIHbU9lntD/8JVW9QrcaeEIEqoDDoCA4o4FVbpUn6c+MbE6ZoxrBlcbIJBmV HeIs6yuZc6lcbCbU63PqJSSrXa6AmMoa853/fmJA/JSOAX8fvzWA31xhQFMST9vf33Gi WsqezEDGEjfh3O4iM4bOeFpbvXSn5qOqiQtMSus1TBv47PhGtuqIhDPJS9TyD6BH/JCw gY4fKSnd8BZgDCo7NJkFCm0HzXF8QjLV4Iq83U+QX1SR0H8S0oOUN+RmD8JoMlZrrmPG YmLw== X-Gm-Message-State: AOJu0Yy3+0euA7tqfFZsVlJrEkNCVQZbcfkLlS0zvujbUllTrmo/9wyk 4xE4+tWLY91SIyQOFR7flyl+GTvMvMtDQEX/5X7azH1luwGemH8p/llbsg+UpQ== X-Google-Smtp-Source: AGHT+IF7QvLZL1pOj6w3qrjg1H9f0heas9GhLVOt2uQ5OAF19lKFIvLR7wccpQe6//RvUEjQw8r5KQ== X-Received: by 2002:a05:6a20:7d86:b0:1a0:60b2:451 with SMTP id v6-20020a056a207d8600b001a060b20451mr544645pzj.7.1709267933645; Thu, 29 Feb 2024 20:38:53 -0800 (PST) Received: from free.home ([2804:7f1:218a:c88b:e868:4eaf:8258:c30b]) by smtp.gmail.com with ESMTPSA id d3-20020a170902cec300b001dcc121c43dsm2375306plg.13.2024.02.29.20.38.52 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 29 Feb 2024 20:38:53 -0800 (PST) Received: from livre (livre.home [172.31.160.2]) by free.home (8.15.2/8.15.2) with ESMTPS id 4214cVGO242857 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Fri, 1 Mar 2024 01:38:32 -0300 From: Alexandre Oliva To: Matthew Malcomson Cc: gcc-patches@gcc.gnu.org, Joseph Myers , Nick Clifton , Richard Earnshaw , Ramana Radhakrishnan , Kyrylo Tkachov , Jakub Jelinek Subject: Re: [PATCH] arm: fix c23 0-named-args caller-side stdarg Organization: Free thinker, does not speak for AdaCore References: Date: Fri, 01 Mar 2024 01:38:30 -0300 In-Reply-To: (Matthew Malcomson's message of "Mon, 26 Feb 2024 16:41:34 +0000") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Scanned-By: MIMEDefang 2.84 X-Spam-Status: No, score=-10.4 required=5.0 tests=BAYES_00,BODY_8BITS,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE,WEIRD_QUOTING autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Hello, Matthew, Thanks for the review. On Feb 26, 2024, Matthew Malcomson wrote: > I think you're right that the AAPCS32 requires all arguments to be passed= in > registers for this testcase. > (Nit on the commit-message: It says that your reading of the AAPCS32 > suggests > that the *caller* is correct -- I believe based on the change you > suggested you > meant *callee* is correct in expecting arguments in registers.) Ugh, yeah, sorry about the typo. > The approach you suggest looks OK to me -- I do notice that it doesn't > fix the > legacy ABI's of `atpcs` and `apcs` and guess it would be nicer to have th= em > working at the same time though would defer to maintainers on how > important that > is. > (For the benefit of others reading) I don't believe there is any ABI conc= ern > with this since it's fixing something that is currently not working at > all and > only applies to c23 (so a change shouldn't have too much of an impact). > You mention you chose to make the change in the arm backend rather > than general > code due to hesitancy to change the generic ABI-affecting code. That makes > sense to me, certainly at this late stage in the development cycle. *nod* I wrote the patch in the following context: I hit the problem on the very first toolchain I started transitioning to gcc-13. I couldn't really fathom the notion that this breakage could have survived an entire release cycle if it affected many targets, and sort of held on to an assumption that the abi used by our arm-eabi toolchain had to be an uncommon one. All of this hypothesizing falls apart by the now apparent knowledge that the test is faling elsewhere as well, even on other ARM ABIs, it just hadn't been addressed yet. I'm glad we're getting there :-) > From a quick check on c23-stdarg-4.c it does look like the below > change ends up > with the same codegen as your patch (except in the case of those > legacy ABI's, > where the below does make the caller and callee ABI match AFAICT): > ``` > =C2=A0 diff --git a/gcc/calls.cc b/gcc/calls.cc > =C2=A0 index 01f44734743..0b302f633ed 100644 > =C2=A0 --- a/gcc/calls.cc > =C2=A0 +++ b/gcc/calls.cc > =C2=A0 @@ -2970,14 +2970,15 @@ expand_call (tree exp, rtx target, int ign= ore) > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 we do not have any reliable wa= y to pass unnamed args in > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 registers, so we must force th= em into memory.=C2=A0 */ > =C2=A0 -=C2=A0 if (type_arg_types !=3D 0 > =C2=A0 +=C2=A0 if ((type_arg_types !=3D 0 || TYPE_NO_NAMED_ARGS_STDARG_P = (funtype)) > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 && targetm.calls.strict_= argument_naming (args_so_far)) > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 ; > =C2=A0=C2=A0=C2=A0=C2=A0 else if (type_arg_types !=3D 0 > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 = && ! targetm.calls.pretend_outgoing_varargs_named > (args_so_far)) > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 /* Don't include the last named arg.= =C2=A0 */ > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 --n_named_args; > =C2=A0 -=C2=A0 else if (TYPE_NO_NAMED_ARGS_STDARG_P (funtype)) > =C2=A0 +=C2=A0 else if (TYPE_NO_NAMED_ARGS_STDARG_P (funtype) > =C2=A0 +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 && ! targetm.calls.pre= tend_outgoing_varargs_named (args_so_far)) > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 n_named_args =3D 0; > =C2=A0=C2=A0=C2=A0=C2=A0 else > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 /* Treat all args as named.=C2=A0 */ > ``` > Do you agree that this makes sense (i.e. is there something I'm > completely missing)? Yeah, your argument is quite convincing, and the target knobs are indeed in line with the change you suggest, whereas the current code seems to deviate from them. With my ABI designer hat on, however, I see that there's room for ABIs to make decisions about 0-args stdargs that go differently from stdargs with leading named args, from prototyped functions, and even from prototypeless functions, and we might end up needing more knobs to deal with such custom decisions. We can cross that bridge if/when we get to it, though. > (lm32 mcore msp430 gcn cris fr30 frv h8300 arm v850 rx pru) Interesting that ppc64le is not on your list. There's PR107453 about that, and another thread is discussing a fix for it that is somewhat different from what you propose (presumably because the way the problem manifests on ppc64le is different), but it also tweaks expand_call. I'll copy you when following up there. --=20 Alexandre Oliva, happy hacker https://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive