Re: RFA [PATCH] to expand_function_end for PR target/88529 - empty class return.

Re: RFA [PATCH] to expand_function_end for PR target/88529 - empty class return.

[Jakub Jelinek]

On Tue, Mar 05, 2019 at 05:33:45PM -0500, Jason Merrill 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.  This seems like a
> missed case from the GCC 8 TYPE_EMPTY_P changes.
> 
> Shall I apply this for GCC 9, or hold it for stage 1?
> 
> 	* function.c (expand_function_end): Don't copy a TYPE_EMPTY_P
>         result.

I'd go for it now.  See below for a testsuite nit:

> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/opt/empty3.C
> @@ -0,0 +1,8 @@
> +// PR c++/88529
> +// { dg-do compile { target c++11 } }
> +// { dg-final { scan-assembler-not mov { target x86_64-*-* } } }

But this probably needs to be { target { { i?86-*-* x86_64-*-* } && { ! { ia32 } } } }
or similar instead, so that it is done for x86_64 -m64 and -mx32, but not
-m32.

> +// The x86_64 psABI says that f() doesn't put the return value anywhere.
> +
> +class A{};
> +
> +A f() { return {}; }

	Jakub

RFA [PATCH] to expand_function_end for PR target/88529 - empty class return.

[Jason Merrill]

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.  This seems like a
missed case from the GCC 8 TYPE_EMPTY_P changes.

Shall I apply this for GCC 9, or hold it for stage 1?

	* function.c (expand_function_end): Don't copy a TYPE_EMPTY_P
        result.
---
 gcc/function.c                    | 8 +++++---
 gcc/testsuite/g++.dg/opt/empty3.C | 8 ++++++++
 gcc/ChangeLog                     | 6 ++++++
 3 files changed, 19 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/opt/empty3.C

diff --git a/gcc/function.c b/gcc/function.c
index dc035707c20..f5998bbeb5e 100644
--- a/gcc/function.c
+++ b/gcc/function.c
@@ -5338,9 +5338,11 @@ expand_function_end (void)
       tree decl_result = DECL_RESULT (current_function_decl);
       rtx decl_rtl = DECL_RTL (decl_result);
 
-      if (REG_P (decl_rtl)
-	  ? REGNO (decl_rtl) >= FIRST_PSEUDO_REGISTER
-	  : DECL_REGISTER (decl_result))
+      if ((REG_P (decl_rtl)
+	   ? REGNO (decl_rtl) >= FIRST_PSEUDO_REGISTER
+	   : DECL_REGISTER (decl_result))
+	  /* Unless the ABI says not to.  */
+	  && !TYPE_EMPTY_P (TREE_TYPE (decl_result)))
 	{
 	  rtx real_decl_rtl = crtl->return_rtx;
 	  complex_mode cmode;
diff --git a/gcc/testsuite/g++.dg/opt/empty3.C b/gcc/testsuite/g++.dg/opt/empty3.C
new file mode 100644
index 00000000000..93ca9abc22c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/opt/empty3.C
@@ -0,0 +1,8 @@
+// PR c++/88529
+// { dg-do compile { target c++11 } }
+// { dg-final { scan-assembler-not mov { target x86_64-*-* } } }
+// The x86_64 psABI says that f() doesn't put the return value anywhere.
+
+class A{};
+
+A f() { return {}; }
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index df50f599d1a..adfe1835340 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,9 @@
+2019-03-05  Jason Merrill  <jason@redhat.com>
+
+	PR target/88529 - empty class return.
+	* function.c (expand_function_end): Don't copy a TYPE_EMPTY_P
+	result.
+
 2019-03-02  Jason Merrill  <jason@redhat.com>
 
 	PR c++/86485 - -Wmaybe-unused with empty class ?:

base-commit: 152b1d2a6cb06a70ebfec9af37c134e24f1ba24d
-- 
2.20.1

Re: RFA [PATCH] to expand_function_end for PR target/88529 - empty class return.

[Jeff Law]

On 3/5/19 3:40 PM, Jakub Jelinek wrote:
> On Tue, Mar 05, 2019 at 05:33:45PM -0500, Jason Merrill 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.  This seems like a
>> missed case from the GCC 8 TYPE_EMPTY_P changes.
>>
>> Shall I apply this for GCC 9, or hold it for stage 1?
>>
>> 	* function.c (expand_function_end): Don't copy a TYPE_EMPTY_P
>>         result.
> 
> I'd go for it now.  See below for a testsuite nit:
Agreed.

> 
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/opt/empty3.C
>> @@ -0,0 +1,8 @@
>> +// PR c++/88529
>> +// { dg-do compile { target c++11 } }
>> +// { dg-final { scan-assembler-not mov { target x86_64-*-* } } }
> 
> But this probably needs to be { target { { i?86-*-* x86_64-*-* } && { ! { ia32 } } } }
> or similar instead, so that it is done for x86_64 -m64 and -mx32, but not
> -m32.
> 
>> +// The x86_64 psABI says that f() doesn't put the return value anywhere.
>> +
>> +class A{};
>> +
>> +A f() { return {}; }
> 
> 	Jakub
>