public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH], PR 71493, Fix PowerPC ABI breakage on GCC trunk/6.1
@ 2016-07-18 23:25 Michael Meissner
  2016-07-18 23:42 ` Segher Boessenkool
  0 siblings, 1 reply; 4+ messages in thread
From: Michael Meissner @ 2016-07-18 23:25 UTC (permalink / raw)
  To: gcc-patches, Segher Boessenkool, David Edelsohn, Bill Schmidt

[-- Attachment #1: Type: text/plain, Size: 3175 bytes --]

When I added the support for __float128 last year, I accidentally broke
returning structures containing a single float or double item using the System
V 32-bit calling sequence.

This patch goes back to using SCALAR_FLOAT_TYPE_P (which looks at the tree
node) instead of SCALAR_FLOAT_MODE_NOT_VECTOR_P (which only looks at the
mode).

I have tested this patch on the trunk on a big endian power7 system, and there
were no regressions.  The same patch applies to the GCC-6 branch, and I am
testing it now.  Assuming there are no regresions on the GCC-6 branch, can I
check this patch into both the trunk and gcc-6-branch?

[gcc]
2016-07-18  Michael Meissner  <meissner@linux.vnet.ibm.com>

	PR target/71493
	* config/rs6000/rs6000.c (rs6000_function_value): Fix
	unintentional System V.4 structure return breakage for structures
	with a single floating point element.

[gcc/testsuite]
2016-07-18  Michael Meissner  <meissner@linux.vnet.ibm.com>

	PR target/71493
	* gcc.target/powerpc/pr71493-1.c: New test.
	* gcc.target/powerpc/pr71493-2.c: Likewise.

Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c	(revision 238438)
+++ gcc/config/rs6000/rs6000.c	(working copy)
@@ -35467,7 +35467,8 @@ rs6000_function_value (const_tree valtyp
   if (DECIMAL_FLOAT_MODE_P (mode) && TARGET_HARD_FLOAT && TARGET_FPRS)
     /* _Decimal128 must use an even/odd register pair.  */
     regno = (mode == TDmode) ? FP_ARG_RETURN + 1 : FP_ARG_RETURN;
-  else if (SCALAR_FLOAT_MODE_NOT_VECTOR_P (mode) && TARGET_HARD_FLOAT && TARGET_FPRS
+  else if (SCALAR_FLOAT_TYPE_P (valtype) && TARGET_HARD_FLOAT && TARGET_FPRS
+	   && !FLOAT128_VECTOR_P (mode)
 	   && ((TARGET_SINGLE_FLOAT && (mode == SFmode)) || TARGET_DOUBLE_FLOAT))
     regno = FP_ARG_RETURN;
   else if (TREE_CODE (valtype) == COMPLEX_TYPE
Index: gcc/testsuite/gcc.target/powerpc/pr71493-2.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/pr71493-2.c	(revision 0)
+++ gcc/testsuite/gcc.target/powerpc/pr71493-2.c	(revision 0)
@@ -0,0 +1,13 @@
+/* { dg-do compile { target { powerpc*-*-linux* && ilp32 } } } */
+/* { dg-options "-O2 -m32 -msvr4-struct-return" } */
+
+struct S2 { double d; };
+
+struct S2 foo2 (void)
+{
+  struct S2 s = { 1.0 };
+  return s;
+}
+
+/* { dg-final { scan-assembler     "lwz" } } */
+/* { dg-final { scan-assembler-not "lfd" } } */
Index: gcc/testsuite/gcc.target/powerpc/pr71493-1.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/pr71493-1.c	(revision 0)
+++ gcc/testsuite/gcc.target/powerpc/pr71493-1.c	(revision 0)
@@ -0,0 +1,13 @@
+/* { dg-do compile { target { powerpc*-*-linux* && ilp32 } } } */
+/* { dg-options "-O2 -m32 -msvr4-struct-return" } */
+
+struct S1 { float f; };
+
+struct S1 foo1 (void)
+{
+  struct S1 s = { 1.0f };
+  return s;
+}
+
+/* { dg-final { scan-assembler     "lwz" } } */
+/* { dg-final { scan-assembler-not "lfs" } } */

-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meissner@linux.vnet.ibm.com, phone: +1 (978) 899-4797

[-- Attachment #2: pr71493.patch01b --]
[-- Type: text/plain, Size: 1971 bytes --]

Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c	(revision 238438)
+++ gcc/config/rs6000/rs6000.c	(working copy)
@@ -35467,7 +35467,8 @@ rs6000_function_value (const_tree valtyp
   if (DECIMAL_FLOAT_MODE_P (mode) && TARGET_HARD_FLOAT && TARGET_FPRS)
     /* _Decimal128 must use an even/odd register pair.  */
     regno = (mode == TDmode) ? FP_ARG_RETURN + 1 : FP_ARG_RETURN;
-  else if (SCALAR_FLOAT_MODE_NOT_VECTOR_P (mode) && TARGET_HARD_FLOAT && TARGET_FPRS
+  else if (SCALAR_FLOAT_TYPE_P (valtype) && TARGET_HARD_FLOAT && TARGET_FPRS
+	   && !FLOAT128_VECTOR_P (mode)
 	   && ((TARGET_SINGLE_FLOAT && (mode == SFmode)) || TARGET_DOUBLE_FLOAT))
     regno = FP_ARG_RETURN;
   else if (TREE_CODE (valtype) == COMPLEX_TYPE
Index: gcc/testsuite/gcc.target/powerpc/pr71493-2.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/pr71493-2.c	(revision 0)
+++ gcc/testsuite/gcc.target/powerpc/pr71493-2.c	(revision 0)
@@ -0,0 +1,13 @@
+/* { dg-do compile { target { powerpc*-*-linux* && ilp32 } } } */
+/* { dg-options "-O2 -m32 -msvr4-struct-return" } */
+
+struct S2 { double d; };
+
+struct S2 foo2 (void)
+{
+  struct S2 s = { 1.0 };
+  return s;
+}
+
+/* { dg-final { scan-assembler     "lwz" } } */
+/* { dg-final { scan-assembler-not "lfd" } } */
Index: gcc/testsuite/gcc.target/powerpc/pr71493-1.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/pr71493-1.c	(revision 0)
+++ gcc/testsuite/gcc.target/powerpc/pr71493-1.c	(revision 0)
@@ -0,0 +1,13 @@
+/* { dg-do compile { target { powerpc*-*-linux* && ilp32 } } } */
+/* { dg-options "-O2 -m32 -msvr4-struct-return" } */
+
+struct S1 { float f; };
+
+struct S1 foo1 (void)
+{
+  struct S1 s = { 1.0f };
+  return s;
+}
+
+/* { dg-final { scan-assembler     "lwz" } } */
+/* { dg-final { scan-assembler-not "lfs" } } */

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH], PR 71493, Fix PowerPC ABI breakage on GCC trunk/6.1
  2016-07-18 23:25 [PATCH], PR 71493, Fix PowerPC ABI breakage on GCC trunk/6.1 Michael Meissner
@ 2016-07-18 23:42 ` Segher Boessenkool
  2016-07-19  3:40   ` Michael Meissner
  0 siblings, 1 reply; 4+ messages in thread
From: Segher Boessenkool @ 2016-07-18 23:42 UTC (permalink / raw)
  To: Michael Meissner, gcc-patches, David Edelsohn, Bill Schmidt

On Mon, Jul 18, 2016 at 07:25:09PM -0400, Michael Meissner wrote:
> When I added the support for __float128 last year, I accidentally broke
> returning structures containing a single float or double item using the System
> V 32-bit calling sequence.
> 
> This patch goes back to using SCALAR_FLOAT_TYPE_P (which looks at the tree
> node) instead of SCALAR_FLOAT_MODE_NOT_VECTOR_P (which only looks at the
> mode).
> 
> I have tested this patch on the trunk on a big endian power7 system, and there
> were no regressions.  The same patch applies to the GCC-6 branch, and I am
> testing it now.  Assuming there are no regresions on the GCC-6 branch, can I
> check this patch into both the trunk and gcc-6-branch?

Did you test with -m32, too?

Ah the testcases (thanks) have it explicitly.  Well.  Does this work?

> +/* { dg-do compile { target { powerpc*-*-linux* && ilp32 } } } */
> +/* { dg-options "-O2 -m32 -msvr4-struct-return" } */

Are dg-options set before the target test or after?  If before, the ilp32
is superfluous; if after, the -m32 is.  Or is there more to it?

I think you can drop the ilp32.

Please sort that out, make sure the testcases are actually run, and then
this is okay for trunk as well as 6.

Thanks for taking care of this!


Segher

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH], PR 71493, Fix PowerPC ABI breakage on GCC trunk/6.1
  2016-07-18 23:42 ` Segher Boessenkool
@ 2016-07-19  3:40   ` Michael Meissner
  2016-07-19 13:50     ` Segher Boessenkool
  0 siblings, 1 reply; 4+ messages in thread
From: Michael Meissner @ 2016-07-19  3:40 UTC (permalink / raw)
  To: Segher Boessenkool
  Cc: Michael Meissner, gcc-patches, David Edelsohn, Bill Schmidt

On Mon, Jul 18, 2016 at 06:42:02PM -0500, Segher Boessenkool wrote:
> On Mon, Jul 18, 2016 at 07:25:09PM -0400, Michael Meissner wrote:
> > When I added the support for __float128 last year, I accidentally broke
> > returning structures containing a single float or double item using the System
> > V 32-bit calling sequence.
> > 
> > This patch goes back to using SCALAR_FLOAT_TYPE_P (which looks at the tree
> > node) instead of SCALAR_FLOAT_MODE_NOT_VECTOR_P (which only looks at the
> > mode).
> > 
> > I have tested this patch on the trunk on a big endian power7 system, and there
> > were no regressions.  The same patch applies to the GCC-6 branch, and I am
> > testing it now.  Assuming there are no regresions on the GCC-6 branch, can I
> > check this patch into both the trunk and gcc-6-branch?
> 
> Did you test with -m32, too?
> 
> Ah the testcases (thanks) have it explicitly.  Well.  Does this work?
> 
> > +/* { dg-do compile { target { powerpc*-*-linux* && ilp32 } } } */
> > +/* { dg-options "-O2 -m32 -msvr4-struct-return" } */

Yes, both test out ok.

> Are dg-options set before the target test or after?  If before, the ilp32
> is superfluous; if after, the -m32 is.  Or is there more to it?

Not really, using ilp32 and explicit -m32 means -m32 is passed twice.  I will
remove the explicit -m32.

> I think you can drop the ilp32.

You cannot use -m32 on a 64-bit little endian system, so the && ilp32 test
guarantees it is only run on a system that supports 32-bit (a pure 32-bit
system, or a big endian 64-bit system that still has the 32-bit libraries
installed).

I also imagine somebody could build a 64-bit big endian compiler that was
configured with --disable-multilib, and you would not be able to do -m32.

> Please sort that out, make sure the testcases are actually run, and then
> this is okay for trunk as well as 6.

As I said, I dropped the explicit -m32 in dg-options.

> Thanks for taking care of this!

-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meissner@linux.vnet.ibm.com, phone: +1 (978) 899-4797

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH], PR 71493, Fix PowerPC ABI breakage on GCC trunk/6.1
  2016-07-19  3:40   ` Michael Meissner
@ 2016-07-19 13:50     ` Segher Boessenkool
  0 siblings, 0 replies; 4+ messages in thread
From: Segher Boessenkool @ 2016-07-19 13:50 UTC (permalink / raw)
  To: Michael Meissner, gcc-patches, David Edelsohn, Bill Schmidt

On Mon, Jul 18, 2016 at 11:40:03PM -0400, Michael Meissner wrote:
> > > +/* { dg-do compile { target { powerpc*-*-linux* && ilp32 } } } */
> > > +/* { dg-options "-O2 -m32 -msvr4-struct-return" } */

> > I think you can drop the ilp32.
> 
> You cannot use -m32 on a 64-bit little endian system, so the && ilp32 test
> guarantees it is only run on a system that supports 32-bit (a pure 32-bit
> system, or a big endian 64-bit system that still has the 32-bit libraries
> installed).

ilp32 tests for a system that is *now* compiling for 32-bit, not one
that *could*.

> I also imagine somebody could build a 64-bit big endian compiler that was
> configured with --disable-multilib, and you would not be able to do -m32.

Ah, right, we would need a test saying "can compile for PowerPC 32-bit
ELF" (this also includes powerpc-elf targets).  There is no such test
yet.  What you do now works, okay.

Thanks,


Segher

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2016-07-19 13:50 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-18 23:25 [PATCH], PR 71493, Fix PowerPC ABI breakage on GCC trunk/6.1 Michael Meissner
2016-07-18 23:42 ` Segher Boessenkool
2016-07-19  3:40   ` Michael Meissner
2016-07-19 13:50     ` Segher Boessenkool

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).