, Set PowerPC .gnu_attribute for long double use if no call

Message ID 20180111181105.GA20208@ibm-tiger.the-meissners.org
State New
Headers show
Series
  • , Set PowerPC .gnu_attribute for long double use if no call
Related show

Commit Message

Michael Meissner Jan. 11, 2018, 6:11 p.m.
In working on the transition of PowerPC long double from using the IBM extended
double format to IEEE 128-bit floating point, I noticed that the long double
.gnu_attribute (#4) was not set if the compiler can handle long double directly
without doing the call to an emulator, such as using IEEE 128-bit floating
point on an ISA 3.0 (power9) 64-bit system.  This patch sets the attribute if
there is a move of the appropriate type.  I only check TF/TCmode for the normal
case, and DF/DCmode for -mlong-double-64, since IFmode is used for __ibm128
when long double is IEEE and KFmode is used for __float128 when long double is
IEEE.

I have checked this on a little endian power8 system with bootstrap and make
check.  There were no regressions, and I verified that the three new tests are
run and pass.  Can I check this into the trunk?

[gcc]
2018-01-11  Michael Meissner  <meissner@linux.vnet.ibm.com>

	(rs6000_emit_move): If we load or store a long double type, set
	the flags for noting the default long double type, even if we
	don't pass or return a long double type.

[gcc/testsuite]
2018-01-11  Michael Meissner  <meissner@linux.vnet.ibm.com>

	* gcc.target/powerpc/gnuattr1.c: New test to make sure we set the
	appropriate .gnu_attribute for the long double type, if we use the
	long double type, but do not generate any calls.
	* gcc.target/powerpc/gnuattr2.c: Likewise.
	* gcc.target/powerpc/gnuattr3.c: Likewise.

Comments

Segher Boessenkool Jan. 12, 2018, 5:21 p.m. | #1
On Thu, Jan 11, 2018 at 01:11:05PM -0500, Michael Meissner wrote:
> In working on the transition of PowerPC long double from using the IBM extended
> double format to IEEE 128-bit floating point, I noticed that the long double
> .gnu_attribute (#4) was not set if the compiler can handle long double directly
> without doing the call to an emulator, such as using IEEE 128-bit floating
> point on an ISA 3.0 (power9) 64-bit system.  This patch sets the attribute if
> there is a move of the appropriate type.  I only check TF/TCmode for the normal
> case, and DF/DCmode for -mlong-double-64, since IFmode is used for __ibm128
> when long double is IEEE and KFmode is used for __float128 when long double is
> IEEE.
> 
> I have checked this on a little endian power8 system with bootstrap and make
> check.  There were no regressions, and I verified that the three new tests are
> run and pass.  Can I check this into the trunk?

> [gcc]
> 2018-01-11  Michael Meissner  <meissner@linux.vnet.ibm.com>
> 
> 	(rs6000_emit_move): If we load or store a long double type, set
> 	the flags for noting the default long double type, even if we
> 	don't pass or return a long double type.
> 
> [gcc/testsuite]
> 2018-01-11  Michael Meissner  <meissner@linux.vnet.ibm.com>
> 
> 	* gcc.target/powerpc/gnuattr1.c: New test to make sure we set the
> 	appropriate .gnu_attribute for the long double type, if we use the
> 	long double type, but do not generate any calls.
> 	* gcc.target/powerpc/gnuattr2.c: Likewise.
> 	* gcc.target/powerpc/gnuattr3.c: Likewise.


> +  if (rs6000_gnu_attr
> +      && ((HAVE_LD_PPC_GNU_ATTR_LONG_DOUBLE || TARGET_64BIT))

One pair of parens is enough ;-)

> +      && ((TARGET_LONG_DOUBLE_128
> +	   && (mode == TFmode || mode == TCmode))
> +	  || (!TARGET_LONG_DOUBLE_128
> +	      && (mode == DFmode || mode == DCmode))))

It's easier to read if you join these lines pairwise:

> +      && ((TARGET_LONG_DOUBLE_128 && (mode == TFmode || mode == TCmode))
> +	  || (!TARGET_LONG_DOUBLE_128 && (mode == DFmode || mode == DCmode))))

Or maybe something with ?:, or break the statement into multiple.

Okay for trunk if you make it a bit more readable :-)  Thanks,


Segher
Michael Meissner Jan. 17, 2018, 11:23 p.m. | #2
On Fri, Jan 12, 2018 at 11:21:04AM -0600, Segher Boessenkool wrote:
> On Thu, Jan 11, 2018 at 01:11:05PM -0500, Michael Meissner wrote:
> > In working on the transition of PowerPC long double from using the IBM extended
> > double format to IEEE 128-bit floating point, I noticed that the long double
> > .gnu_attribute (#4) was not set if the compiler can handle long double directly
> > without doing the call to an emulator, such as using IEEE 128-bit floating
> > point on an ISA 3.0 (power9) 64-bit system.  This patch sets the attribute if
> > there is a move of the appropriate type.  I only check TF/TCmode for the normal
> > case, and DF/DCmode for -mlong-double-64, since IFmode is used for __ibm128
> > when long double is IEEE and KFmode is used for __float128 when long double is
> > IEEE.
> > 
> > I have checked this on a little endian power8 system with bootstrap and make
> > check.  There were no regressions, and I verified that the three new tests are
> > run and pass.  Can I check this into the trunk?
> 
> > [gcc]
> > 2018-01-11  Michael Meissner  <meissner@linux.vnet.ibm.com>
> > 
> > 	(rs6000_emit_move): If we load or store a long double type, set
> > 	the flags for noting the default long double type, even if we
> > 	don't pass or return a long double type.
> > 
> > [gcc/testsuite]
> > 2018-01-11  Michael Meissner  <meissner@linux.vnet.ibm.com>
> > 
> > 	* gcc.target/powerpc/gnuattr1.c: New test to make sure we set the
> > 	appropriate .gnu_attribute for the long double type, if we use the
> > 	long double type, but do not generate any calls.
> > 	* gcc.target/powerpc/gnuattr2.c: Likewise.
> > 	* gcc.target/powerpc/gnuattr3.c: Likewise.
> 
> 
> > +  if (rs6000_gnu_attr
> > +      && ((HAVE_LD_PPC_GNU_ATTR_LONG_DOUBLE || TARGET_64BIT))
> 
> One pair of parens is enough ;-)
> 
> > +      && ((TARGET_LONG_DOUBLE_128
> > +	   && (mode == TFmode || mode == TCmode))
> > +	  || (!TARGET_LONG_DOUBLE_128
> > +	      && (mode == DFmode || mode == DCmode))))
> 
> It's easier to read if you join these lines pairwise:
> 
> > +      && ((TARGET_LONG_DOUBLE_128 && (mode == TFmode || mode == TCmode))
> > +	  || (!TARGET_LONG_DOUBLE_128 && (mode == DFmode || mode == DCmode))))
> 
> Or maybe something with ?:, or break the statement into multiple.
> 
> Okay for trunk if you make it a bit more readable :-)  Thanks,

This is what I just checked in:

Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c	(revision 256810)
+++ gcc/config/rs6000/rs6000.c	(working copy)
@@ -10493,6 +10493,23 @@ rs6000_emit_move (rtx dest, rtx source,
       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))

Patch

Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c	(revision 256534)
+++ gcc/config/rs6000/rs6000.c	(working copy)
@@ -10505,6 +10505,22 @@  rs6000_emit_move (rtx dest, rtx source,
       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))
+      && ((TARGET_LONG_DOUBLE_128
+	   && (mode == TFmode || mode == TCmode))
+	  || (!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))
Index: gcc/testsuite/gcc.target/powerpc/gnuattr1.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/gnuattr1.c	(nonexistent)
+++ gcc/testsuite/gcc.target/powerpc/gnuattr1.c	(working copy)
@@ -0,0 +1,15 @@ 
+/* { 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++;
+}
Index: gcc/testsuite/gcc.target/powerpc/gnuattr2.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/gnuattr2.c	(nonexistent)
+++ gcc/testsuite/gcc.target/powerpc/gnuattr2.c	(working copy)
@@ -0,0 +1,17 @@ 
+/* { 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++;
+}
Index: gcc/testsuite/gcc.target/powerpc/gnuattr3.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/gnuattr3.c	(nonexistent)
+++ gcc/testsuite/gcc.target/powerpc/gnuattr3.c	(working copy)
@@ -0,0 +1,15 @@ 
+/* { 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);
+}