diff mbox series

PowerPC: PR 97791: Fix gnu attributes.

Message ID 20201119235429.GA32109@ibm-toto.the-meissners.org
State New
Headers show
Series PowerPC: PR 97791: Fix gnu attributes. | expand

Commit Message

Michael Meissner Nov. 19, 2020, 11:54 p.m. UTC 
[PATCH] PowerPC: PR 97791: Fix gnu attributes.

Note, I originally posted this to an internal IBM mailing list, not to
gcc-patches.  Sorry about that.

This patch does two things to fix setting gnu attribute #4 (long double status)

1) Only set gnu attribute #4 if long double was passed.  Passing __float128
when long double is IBM or __ibm128 when long double is IEEE no longer sets the
attribute.  This resulted in a lot of false positives, such as using __float128
and no long double support.

2) Do not set the gnu attribute if a mode used by long double (TF or DF) is
used in a move.  The moves do not differentiate between the long double type
and similar types.  Delete the three tests that tested this.

I wrote the code for the move several years.  I wanted to flag that an object
that used the appropriate long double type got flagged.  Unfortunately, at the
RTL level, we have lost the type nodes, so we can't tell the difference between
two types that use the same mode (for instance if long double is 64-bit, the
attribute would be set if you used normal doubles, and not long doubles).  Alan
Modra and I discussed this, and we think this is just the right thing to do.

It has been tested on power8 big endian Linux server systems and power9 little
endian Linux server systems, and there were no regressions.

gcc/
2020-11-17  Michael Meissner  <meissner@linux.ibm.com>

	PR gcc/97791
	* config/rs6000/rs6000-call.c (init_cumulative_args): Only set
	that long double was returned if the type is actually long
	double.
	(rs6000_function_arg_advance_1): Only set that long double was
	passed if the type is actually long double.
	* config/rs6000/rs6000.c (rs6000_emit_move): Delete code that sets
	whether long double was passed based on the modes used in moves.

gcc/testsuite/
2020-11-17  Michael Meissner  <meissner@linux.ibm.com>

	PR target/97791
	* gcc.target/powerpc/gnuattr1.c: Delete.
	* gcc.target/powerpc/gnuattr2.c: Delete.
	* gcc.target/powerpc/gnuattr3.c: Delete.
---
 gcc/config/rs6000/rs6000-call.c             | 13 ++++---------
 gcc/config/rs6000/rs6000.c                  | 17 -----------------
 gcc/testsuite/gcc.target/powerpc/gnuattr1.c | 15 ---------------
 gcc/testsuite/gcc.target/powerpc/gnuattr2.c | 17 -----------------
 gcc/testsuite/gcc.target/powerpc/gnuattr3.c | 15 ---------------
 5 files changed, 4 insertions(+), 73 deletions(-)
 delete mode 100644 gcc/testsuite/gcc.target/powerpc/gnuattr1.c
 delete mode 100644 gcc/testsuite/gcc.target/powerpc/gnuattr2.c
 delete mode 100644 gcc/testsuite/gcc.target/powerpc/gnuattr3.c

Comments

Segher Boessenkool Nov. 23, 2020, 11:57 p.m. UTC | #1 
Hi!

On Thu, Nov 19, 2020 at 06:54:29PM -0500, Michael Meissner wrote:
> 1) Only set gnu attribute #4 if long double was passed.  Passing __float128
> when long double is IBM or __ibm128 when long double is IEEE no longer sets the
> attribute.  This resulted in a lot of false positives, such as using __float128
> and no long double support.

Sure, makes sense (and is a bug fix actually).

> 2) Do not set the gnu attribute if a mode used by long double (TF or DF) is
> used in a move.  The moves do not differentiate between the long double type
> and similar types.  Delete the three tests that tested this.

This, too.

> diff --git a/gcc/testsuite/gcc.target/powerpc/gnuattr1.c b/gcc/testsuite/gcc.target/powerpc/gnuattr1.c
> deleted file mode 100644
> index cf46777849a..00000000000
> --- a/gcc/testsuite/gcc.target/powerpc/gnuattr1.c
> +++ /dev/null
> @@ -1,15 +0,0 @@
> -/* { dg-do compile { target { powerpc*-linux-* } } } */
> -/* { dg-require-effective-target powerpc_vsx_ok } */
> -/* { dg-options "-O2 -mvsx -mlong-double-64" } */
> -/* { dg-final { scan-assembler "gnu_attribute 4, 9" } } */
> -
> -/* Check that if we can do the long double operation without doing an emulator
> -   call, such as with 64-bit long double support, that we still set the
> -   appropriate .gnu_attribute.  */
> -
> -long double a;
> -
> -void add1 (void)
> -{
> -  a++;
> -}

But this *does* use long double.  So this testcase is valid, and you
should not delete it.

Instead, it points out you have a deficiency in the code, one that used
to be hidden by how you used moves to set the attribute.

Same for the other testcases you delete.


Segher
diff mbox series

Patch

diff --git a/gcc/config/rs6000/rs6000-call.c b/gcc/config/rs6000/rs6000-call.c
index 3bd89a79bad..8294e22fb85 100644
--- a/gcc/config/rs6000/rs6000-call.c
+++ b/gcc/config/rs6000/rs6000-call.c
@@ -6539,11 +6539,8 @@  init_cumulative_args (CUMULATIVE_ARGS *cum, tree fntype,
 	    {
 	      rs6000_passes_float = true;
 	      if ((HAVE_LD_PPC_GNU_ATTR_LONG_DOUBLE || TARGET_64BIT)
-		  && (FLOAT128_IBM_P (return_mode)
-		      || FLOAT128_IEEE_P (return_mode)
-		      || (return_type != NULL
-			  && (TYPE_MAIN_VARIANT (return_type)
-			      == long_double_type_node))))
+		  && return_type != NULL
+		  && TYPE_MAIN_VARIANT (return_type) == long_double_type_node)
 		rs6000_passes_long_double = true;
 
 	      /* Note if we passed or return a IEEE 128-bit type.  We changed
@@ -7001,10 +6998,8 @@  rs6000_function_arg_advance_1 (CUMULATIVE_ARGS *cum, machine_mode mode,
 	{
 	  rs6000_passes_float = true;
 	  if ((HAVE_LD_PPC_GNU_ATTR_LONG_DOUBLE || TARGET_64BIT)
-	      && (FLOAT128_IBM_P (mode)
-		  || FLOAT128_IEEE_P (mode)
-		  || (type != NULL
-		      && TYPE_MAIN_VARIANT (type) == long_double_type_node)))
+	      && type != NULL
+	      && TYPE_MAIN_VARIANT (type) == long_double_type_node)
 	    rs6000_passes_long_double = true;
 
 	  /* Note if we passed or return a IEEE 128-bit type.  We changed the
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index b6fd21a5d6f..a5188553593 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -10076,23 +10076,6 @@  rs6000_emit_move (rtx dest, rtx source, machine_mode mode)
       && GET_MODE_BITSIZE (mode) <= HOST_BITS_PER_WIDE_INT)
     gcc_unreachable ();
 
-#ifdef HAVE_AS_GNU_ATTRIBUTE
-  /* If we use a long double type, set the flags in .gnu_attribute that say
-     what the long double type is.  This is to allow the linker's warning
-     message for the wrong long double to be useful, even if the function does
-     not do a call (for example, doing a 128-bit add on power9 if the long
-     double type is IEEE 128-bit.  Do not set this if __ibm128 or __floa128 are
-     used if they aren't the default long dobule type.  */
-  if (rs6000_gnu_attr && (HAVE_LD_PPC_GNU_ATTR_LONG_DOUBLE || TARGET_64BIT))
-    {
-      if (TARGET_LONG_DOUBLE_128 && (mode == TFmode || mode == TCmode))
-	rs6000_passes_float = rs6000_passes_long_double = true;
-
-      else if (!TARGET_LONG_DOUBLE_128 && (mode == DFmode || mode == DCmode))
-	rs6000_passes_float = rs6000_passes_long_double = true;
-    }
-#endif
-
   /* See if we need to special case SImode/SFmode SUBREG moves.  */
   if ((mode == SImode || mode == SFmode) && SUBREG_P (source)
       && rs6000_emit_move_si_sf_subreg (dest, source, mode))
diff --git a/gcc/testsuite/gcc.target/powerpc/gnuattr1.c b/gcc/testsuite/gcc.target/powerpc/gnuattr1.c
deleted file mode 100644
index cf46777849a..00000000000
--- a/gcc/testsuite/gcc.target/powerpc/gnuattr1.c
+++ /dev/null
@@ -1,15 +0,0 @@ 
-/* { dg-do compile { target { powerpc*-linux-* } } } */
-/* { dg-require-effective-target powerpc_vsx_ok } */
-/* { dg-options "-O2 -mvsx -mlong-double-64" } */
-/* { dg-final { scan-assembler "gnu_attribute 4, 9" } } */
-
-/* Check that if we can do the long double operation without doing an emulator
-   call, such as with 64-bit long double support, that we still set the
-   appropriate .gnu_attribute.  */
-
-long double a;
-
-void add1 (void)
-{
-  a++;
-}
diff --git a/gcc/testsuite/gcc.target/powerpc/gnuattr2.c b/gcc/testsuite/gcc.target/powerpc/gnuattr2.c
deleted file mode 100644
index 32a4ba255a8..00000000000
--- a/gcc/testsuite/gcc.target/powerpc/gnuattr2.c
+++ /dev/null
@@ -1,17 +0,0 @@ 
-/* { dg-do compile { target { powerpc*-linux-* && lp64 } } } */
-/* { dg-require-effective-target powerpc_p9vector_ok } */
-/* { dg-options "-O2 -mpower9-vector -mabi=ieeelongdouble -Wno-psabi" } */
-/* { dg-final { scan-assembler "gnu_attribute 4, 13" } } */
-
-/* Check that if we can do the long double operation without doing an emulator
-   call, such as with IEEE 128-bit hardware support on power9, that we still
-   set the appropriate .gnu_attribute.  The && lp64 is needed, because we can't
-   enable the IEEE 128-bit hardware instructions on ISA 3.0 (power9) in 32-bit,
-   because we don't have a TImode available.  */
-
-long double a;
-
-void add1 (void)
-{
-  a++;
-}
diff --git a/gcc/testsuite/gcc.target/powerpc/gnuattr3.c b/gcc/testsuite/gcc.target/powerpc/gnuattr3.c
deleted file mode 100644
index bd5a64fe330..00000000000
--- a/gcc/testsuite/gcc.target/powerpc/gnuattr3.c
+++ /dev/null
@@ -1,15 +0,0 @@ 
-/* { dg-do compile { target { powerpc*-linux-* } } } */
-/* { dg-require-effective-target powerpc_vsx_ok } */
-/* { dg-options "-O2 -mvsx -mabi=ibmlongdouble -Wno-psabi" } */
-/* { dg-final { scan-assembler "gnu_attribute 4, 5" } } */
-
-/* Check that if we can do the long double operation without doing an emulator
-   call, such as with copysign, that we still set the appropriate
-   .gnu_attribute.  */
-
-long double a, b, c;
-
-void cs (void)
-{
-  a = __builtin_copysignl (b, c);
-}