[gcc-13 backport PATCH] RISC-V: Fix C23 (...) functions returning large aggregates [PR114175]

[gcc-13 backport PATCH] RISC-V: Fix C23 (...) functions returning large aggregates [PR114175]

[Edwin Lu]

We assume that TYPE_NO_NAMED_ARGS_STDARG_P don't have any named arguments and
there is nothing to advance, but that is not the case for (...) functions
returning by hidden reference which have one such artificial argument.
This causes gcc.dg/c23-stdarg-[68].c to fail

Fix the issue by checking if arg.type is NULL as r14-9503-g218d1749612
explains

Tested on linux rv64gcv.

gcc/ChangeLog:

	PR target/114175
	* config/riscv/riscv.cc (riscv_setup_incoming_varargs): Only skip
	riscv_funciton_arg_advance for TYPE_NO_NAMED_ARGS_STDARG_P functions
	if arg.type is NULL

(cherry picked from commit 60586710b0646efdbbd77a7f53b93fb5edb87a61)
---
 gcc/config/riscv/riscv.cc | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index 01eebc83cc5..cefd3b7b2b2 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -3961,7 +3961,8 @@ riscv_setup_incoming_varargs (cumulative_args_t cum,
      argument.  Advance a local copy of CUM past the last "real" named
      argument, to find out how many registers are left over.  */
   local_cum = *get_cumulative_args (cum);
-  if (!TYPE_NO_NAMED_ARGS_STDARG_P (TREE_TYPE (current_function_decl)))
+  if (!TYPE_NO_NAMED_ARGS_STDARG_P (TREE_TYPE (current_function_decl))
+      || arg.type != NULL_TREE)
     riscv_function_arg_advance (pack_cumulative_args (&local_cum), arg);
 
   /* Found out how many registers we need to save.  */
-- 
2.34.1

Re: [gcc-13 backport PATCH] RISC-V: Fix C23 (...) functions returning large aggregates [PR114175]

[Palmer Dabbelt]

On Wed, 03 Apr 2024 13:17:36 PDT (-0700), ewlu@rivosinc.com wrote:
> We assume that TYPE_NO_NAMED_ARGS_STDARG_P don't have any named arguments and
> there is nothing to advance, but that is not the case for (...) functions
> returning by hidden reference which have one such artificial argument.
> This causes gcc.dg/c23-stdarg-[68].c to fail
>
> Fix the issue by checking if arg.type is NULL as r14-9503-g218d1749612
> explains
>
> Tested on linux rv64gcv.
>
> gcc/ChangeLog:
>
> 	PR target/114175
> 	* config/riscv/riscv.cc (riscv_setup_incoming_varargs): Only skip
> 	riscv_funciton_arg_advance for TYPE_NO_NAMED_ARGS_STDARG_P functions
> 	if arg.type is NULL
>
> (cherry picked from commit 60586710b0646efdbbd77a7f53b93fb5edb87a61)
> ---
>  gcc/config/riscv/riscv.cc | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> index 01eebc83cc5..cefd3b7b2b2 100644
> --- a/gcc/config/riscv/riscv.cc
> +++ b/gcc/config/riscv/riscv.cc
> @@ -3961,7 +3961,8 @@ riscv_setup_incoming_varargs (cumulative_args_t cum,
>       argument.  Advance a local copy of CUM past the last "real" named
>       argument, to find out how many registers are left over.  */
>    local_cum = *get_cumulative_args (cum);
> -  if (!TYPE_NO_NAMED_ARGS_STDARG_P (TREE_TYPE (current_function_decl)))
> +  if (!TYPE_NO_NAMED_ARGS_STDARG_P (TREE_TYPE (current_function_decl))
> +      || arg.type != NULL_TREE)
>      riscv_function_arg_advance (pack_cumulative_args (&local_cum), arg);
>
>    /* Found out how many registers we need to save.  */

Acked-by: Palmer Dabbelt <palmer@rivosinc.com>

I'm not sure if we need release maintainer approval, all I can find is 
the 13.2.1 status report saying 13.3 is expected in the spring 
<https://inbox.sourceware.org/gcc/ZMJeq%2FY5SN+7i8a+@tucnak/>.  My 
allergies certainly indicate it's spring, but that's kind of a wide time 
window...

Maybe Jakub knows?

Re: [gcc-13 backport PATCH] RISC-V: Fix C23 (...) functions returning large aggregates [PR114175]

[Jakub Jelinek]

On Thu, Apr 04, 2024 at 07:28:40AM -0700, Palmer Dabbelt wrote:
> I'm not sure if we need release maintainer approval,

For cherry-picking one's own non-risky bugfixes for regression or
documentation bugs from trunk to release branches no special approval
is needed, or maintainer of the corresponding code can approve that,
release manager approval is only needed when a branch is frozen before a
release.

> all I can find is the
> 13.2.1 status report saying 13.3 is expected in the spring
> <https://inbox.sourceware.org/gcc/ZMJeq%2FY5SN+7i8a+@tucnak/>.  My allergies
> certainly indicate it's spring, but that's kind of a wide time window...
> 
> Maybe Jakub knows?

Most likely some short time after 14.1 is released, so that one can still
cherry-pick whatever was fixed on the 14 branch and there is time for those
cherry-picks and testing.
https://gcc.gnu.org/releases.html#timeline gives some hints...

	Jakub

Re: [gcc-13 backport PATCH] RISC-V: Fix C23 (...) functions returning large aggregates [PR114175]

[Palmer Dabbelt]

On Thu, 04 Apr 2024 07:37:56 PDT (-0700), jakub@redhat.com wrote:
> On Thu, Apr 04, 2024 at 07:28:40AM -0700, Palmer Dabbelt wrote:
>> I'm not sure if we need release maintainer approval,
>
> For cherry-picking one's own non-risky bugfixes for regression or
> documentation bugs from trunk to release branches no special approval
> is needed, or maintainer of the corresponding code can approve that,
> release manager approval is only needed when a branch is frozen before a
> release.

Ya, I'm just never sure when the branch is frozen...

>> all I can find is the
>> 13.2.1 status report saying 13.3 is expected in the spring
>> <https://inbox.sourceware.org/gcc/ZMJeq%2FY5SN+7i8a+@tucnak/>.  My allergies
>> certainly indicate it's spring, but that's kind of a wide time window...
>>
>> Maybe Jakub knows?
>
> Most likely some short time after 14.1 is released, so that one can still
> cherry-pick whatever was fixed on the 14 branch and there is time for those
> cherry-picks and testing.
> https://gcc.gnu.org/releases.html#timeline gives some hints...

OK, so sounds like it's not frozen now and Edwin's OK to commit this on 
the 13 branch.  Thanks.

>
> 	Jakub

Re: [gcc-13 backport Committed] RISC-V: Fix C23 (...) functions returning large aggregates [PR114175]

[Edwin Lu]

On 4/4/2024 7:40 AM, Palmer Dabbelt wrote:
> On Thu, 04 Apr 2024 07:37:56 PDT (-0700), jakub@redhat.com wrote:
>> On Thu, Apr 04, 2024 at 07:28:40AM -0700, Palmer Dabbelt wrote:
>>> I'm not sure if we need release maintainer approval,
>>
>> For cherry-picking one's own non-risky bugfixes for regression or
>> documentation bugs from trunk to release branches no special approval
>> is needed, or maintainer of the corresponding code can approve that,
>> release manager approval is only needed when a branch is frozen before a
>> release.
>
> Ya, I'm just never sure when the branch is frozen...
>
>>> all I can find is the
>>> 13.2.1 status report saying 13.3 is expected in the spring
>>> <https://inbox.sourceware.org/gcc/ZMJeq%2FY5SN+7i8a+@tucnak/>.  My 
>>> allergies
>>> certainly indicate it's spring, but that's kind of a wide time 
>>> window...
>>>
>>> Maybe Jakub knows?
>>
>> Most likely some short time after 14.1 is released, so that one can 
>> still
>> cherry-pick whatever was fixed on the 14 branch and there is time for 
>> those
>> cherry-picks and testing.
>> https://gcc.gnu.org/releases.html#timeline gives some hints...
>
> OK, so sounds like it's not frozen now and Edwin's OK to commit this 
> on the 13 branch.  Thanks.
>
>>
>>     Jakub

Thanks for the clarifications! Committed!

Edwin