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

Message ID 20190305223345.734-1-jason@redhat.com
State New
Headers show
Series
  • RFA [PATCH] to expand_function_end for PR target/88529 - empty class return.
Related show

Commit Message

Jason Merrill March 5, 2019, 10:33 p.m.
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


base-commit: 152b1d2a6cb06a70ebfec9af37c134e24f1ba24d
-- 
2.20.1

Comments

Jakub Jelinek March 5, 2019, 10:40 p.m. | #1
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
Jeff Law March 19, 2019, 8:48 p.m. | #2
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

>

Patch

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 ?: