On Tue, 22 Jun 2021, Jakub Jelinek wrote:
> On Mon, Jun 14, 2021 at 11:24:22PM -0400, Jason Merrill via Gcc-patches wrote:
> > The x86_64 psABI says that an empty class isn't passed or returned in
> > memory or
> > registers, so we shouldn't set %eax in this function. Is this a reasonable
> > place to implement that? Another possibility would be to remove the hack to
> > prevent i386.c:function_value_64 from returning NULL in this case and fix
> > the
> > callers to deal, but that seems like more work.
> >
> > The df-scan hunk catches the case where we look at a 0-length reg and build
> > a range the length of unsigned int, which happened before I changed
> > assign_parms to match expand_function_end.
>
> The assign_params change unfortunately breaks e.g. the following testcase.
> The problem is that some passes (e.g. subreg lowering but assign_parms
> comments also talk about delayed slot scheduling) rely on crtl->return_rtx
> not to contain pseudo registers, and the assign_parms change results
> in the pseudo in there not being replaced with a hard register.
>
> The following patch instead clears the crtl->return_rtx if a function
> returns TYPE_EMPTY_P structure, that way (use (pseudo)) is not emitted
> into the IL and it is treated like more like functions returning void.
>
> I've also changed the effective target on the empty-class1.C testcase, so
> that it doesn't fail on x86_64-linux with -m32 testing.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK.
> 2021-06-22 Jakub Jelinek <ja...@redhat.com>
>
> PR middle-end/101160
> * function.c (assign_parms): For decl_result with TYPE_EMPTY_P type
> clear crtl->return_rtx instead of keeping it referencing a pseudo.
>
> * g++.target/i386/empty-class1.C: Require lp64 effective target
> instead of x86_64-*-*.
> * g++.target/i386/empty-class2.C: New test.
>
> --- gcc/function.c.jj 2021-06-22 10:04:46.000000000 +0200
> +++ gcc/function.c 2021-06-22 11:30:36.615264498 +0200
> @@ -3821,17 +3821,22 @@ assign_parms (tree fndecl)
> tree decl_result = DECL_RESULT (fndecl);
> rtx decl_rtl = DECL_RTL (decl_result);
>
> - if ((REG_P (decl_rtl)
> - ? REGNO (decl_rtl) >= FIRST_PSEUDO_REGISTER
> - : DECL_REGISTER (decl_result))
> - /* Unless the psABI says not to. */
> - && !TYPE_EMPTY_P (TREE_TYPE (decl_result)))
> + if (REG_P (decl_rtl)
> + ? REGNO (decl_rtl) >= FIRST_PSEUDO_REGISTER
> + : DECL_REGISTER (decl_result))
> {
> rtx real_decl_rtl;
>
> - real_decl_rtl = targetm.calls.function_value (TREE_TYPE (decl_result),
> - fndecl, true);
> - REG_FUNCTION_VALUE_P (real_decl_rtl) = 1;
> + /* Unless the psABI says not to. */
> + if (TYPE_EMPTY_P (TREE_TYPE (decl_result)))
> + real_decl_rtl = NULL_RTX;
> + else
> + {
> + real_decl_rtl
> + = targetm.calls.function_value (TREE_TYPE (decl_result),
> + fndecl, true);
> + REG_FUNCTION_VALUE_P (real_decl_rtl) = 1;
> + }
> /* The delay slot scheduler assumes that crtl->return_rtx
> holds the hard register containing the return value, not a
> temporary pseudo. */
> --- gcc/testsuite/g++.target/i386/empty-class1.C.jj 2021-06-22
> 10:04:46.377208914 +0200
> +++ gcc/testsuite/g++.target/i386/empty-class1.C 2021-06-22
> 11:37:53.463375502 +0200
> @@ -1,5 +1,5 @@
> // PR target/88529
> -// { dg-do compile { target { c++11 && x86_64-*-* } } }
> +// { dg-do compile { target { c++11 && lp64 } } }
> // { dg-additional-options -fdump-rtl-expand }
> // { dg-final { scan-rtl-dump-not "set" "expand" } }
> // The x86_64 psABI says that f() doesn't put the return value anywhere.
> --- gcc/testsuite/g++.target/i386/empty-class2.C.jj 2021-06-22
> 11:34:53.422805115 +0200
> +++ gcc/testsuite/g++.target/i386/empty-class2.C 2021-06-22
> 11:35:34.048257864 +0200
> @@ -0,0 +1,20 @@
> +// PR middle-end/101160
> +// Test passing aligned empty aggregate
> +// { dg-do compile }
> +// { dg-options "-O2" }
> +// { dg-additional-options "-Wno-psabi" { target { { i?86-*-* x86_64-*-* }
> && ilp32 } } }
> +
> +struct S { union {} a; } __attribute__((aligned));
> +
> +S
> +foo (S arg)
> +{
> + return arg;
> +}
> +
> +void
> +bar (void)
> +{
> + S arg;
> + foo (arg);
> +}
>
>
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
