diff mbox

PR target/79004, Fix char/short -> _Float128 on PowerPC -mcpu=power9

Message ID 20170110003227.GA16026@ibm-tiger.the-meissners.org
State New
Headers show

Commit Message

Michael Meissner Jan. 10, 2017, 12:32 a.m. UTC 
This patch fixes PR target/79004 by eliminating the optimization of avoiding
direct move if we are converting an 8/16-bit integer value from memory to IEEE
128-bit floating point.

I opened a new bug (PR target/79038) to address the underlying issue that the
IEEE 128-bit floating point integer conversions were written before small
integers were allowed in the traditional Altivec registers.  This meant that we
had to use UNSPEC and explicit temporaries to get the integers into the
appropriate registers.

I have tested this bug by doing a bootstrap build and make check on a little
endian power8 system and using an assembler that knows about ISA 3.0
instructions.  I added a new test to verify the results.  Can I check this into
the trunk?  This is not an issue on GCC 6.x.

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

	PR target/79004
	* config/rs6000/rs6000.md (FP_ISA3): Do not optimize converting
	char or short to __float128/_Float128 directly.

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

	PR target/79004
	* gcc.target/powerpc/pr79004.c: New test.

Comments

Segher Boessenkool Jan. 11, 2017, 10:39 p.m. UTC | #1 
On Mon, Jan 09, 2017 at 07:32:27PM -0500, Michael Meissner wrote:
> This patch fixes PR target/79004 by eliminating the optimization of avoiding
> direct move if we are converting an 8/16-bit integer value from memory to IEEE
> 128-bit floating point.
> 
> I opened a new bug (PR target/79038) to address the underlying issue that the
> IEEE 128-bit floating point integer conversions were written before small
> integers were allowed in the traditional Altivec registers.  This meant that we
> had to use UNSPEC and explicit temporaries to get the integers into the
> appropriate registers.
> 
> I have tested this bug by doing a bootstrap build and make check on a little
> endian power8 system and using an assembler that knows about ISA 3.0
> instructions.  I added a new test to verify the results.  Can I check this into
> the trunk?  This is not an issue on GCC 6.x.

Okay, thanks!  Two comments:

> +/* { dg-final { scan-assembler-not " bl __"    } } */
> +/* { dg-final { scan-assembler     "xscvdpqp"  } } */
> +/* { dg-final { scan-assembler     "xscvqpdp"  } } */

This line always matches if ...

> +/* { dg-final { scan-assembler     "xscvqpdpo" } } */

... this one does.  I recommend \m \M .

> +/* { dg-final { scan-assembler     "xscvqpsdz" } } */
> +/* { dg-final { scan-assembler     "xscvqpswz" } } */
> +/* { dg-final { scan-assembler     "xscvsdqp"  } } */
> +/* { dg-final { scan-assembler     "xscvudqp"  } } */
> +/* { dg-final { scan-assembler     "lxsd"      } } */
> +/* { dg-final { scan-assembler     "lxsiwax"   } } */
> +/* { dg-final { scan-assembler     "lxsiwzx"   } } */
> +/* { dg-final { scan-assembler     "lxssp"     } } */
> +/* { dg-final { scan-assembler     "stxsd"     } } */
> +/* { dg-final { scan-assembler     "stxsiwx"   } } */
> +/* { dg-final { scan-assembler     "stxssp"    } } */

There are many more than 14 instructions generated; maybe you want
scan-assembler-times?


Segher
Michael Meissner Jan. 12, 2017, 10:12 p.m. UTC | #2 
On Wed, Jan 11, 2017 at 04:39:19PM -0600, Segher Boessenkool wrote:
> On Mon, Jan 09, 2017 at 07:32:27PM -0500, Michael Meissner wrote:
> > This patch fixes PR target/79004 by eliminating the optimization of avoiding
> > direct move if we are converting an 8/16-bit integer value from memory to IEEE
> > 128-bit floating point.
> > 
> > I opened a new bug (PR target/79038) to address the underlying issue that the
> > IEEE 128-bit floating point integer conversions were written before small
> > integers were allowed in the traditional Altivec registers.  This meant that we
> > had to use UNSPEC and explicit temporaries to get the integers into the
> > appropriate registers.
> > 
> > I have tested this bug by doing a bootstrap build and make check on a little
> > endian power8 system and using an assembler that knows about ISA 3.0
> > instructions.  I added a new test to verify the results.  Can I check this into
> > the trunk?  This is not an issue on GCC 6.x.
> 
> Okay, thanks!  Two comments:
> 
> > +/* { dg-final { scan-assembler-not " bl __"    } } */
> > +/* { dg-final { scan-assembler     "xscvdpqp"  } } */
> > +/* { dg-final { scan-assembler     "xscvqpdp"  } } */
> 
> This line always matches if ...
> 
> > +/* { dg-final { scan-assembler     "xscvqpdpo" } } */
> 
> ... this one does.  I recommend \m \M .

Ok, I rewrote the test to use \m and \M and checked it in.  Thanks.

> > +/* { dg-final { scan-assembler     "xscvqpsdz" } } */
> > +/* { dg-final { scan-assembler     "xscvqpswz" } } */
> > +/* { dg-final { scan-assembler     "xscvsdqp"  } } */
> > +/* { dg-final { scan-assembler     "xscvudqp"  } } */
> > +/* { dg-final { scan-assembler     "lxsd"      } } */
> > +/* { dg-final { scan-assembler     "lxsiwax"   } } */
> > +/* { dg-final { scan-assembler     "lxsiwzx"   } } */
> > +/* { dg-final { scan-assembler     "lxssp"     } } */
> > +/* { dg-final { scan-assembler     "stxsd"     } } */
> > +/* { dg-final { scan-assembler     "stxsiwx"   } } */
> > +/* { dg-final { scan-assembler     "stxssp"    } } */
> 
> There are many more than 14 instructions generated; maybe you want
> scan-assembler-times?

I had thought about it, but I thought it might impede future optimizations
(i.e. whether short/char are converted to 32-bit word or 64-bit word before
doing the stores).
diff mbox

Patch

Index: gcc/config/rs6000/rs6000.md
===================================================================
--- gcc/config/rs6000/rs6000.md	(revision 244232)
+++ gcc/config/rs6000/rs6000.md	(working copy)
@@ -521,10 +521,7 @@  (define_mode_iterator SIGNBIT [(KF "FLOA
 			       (TF "FLOAT128_VECTOR_P (TFmode)")])
 
 ; Iterator for ISA 3.0 supported floating point types
-(define_mode_iterator FP_ISA3 [SF
-			       DF
-			       (KF "FLOAT128_IEEE_P (KFmode)")
-			       (TF "FLOAT128_IEEE_P (TFmode)")])
+(define_mode_iterator FP_ISA3 [SF DF])
 
 ; SF/DF suffix for traditional floating instructions
 (define_mode_attr Ftrad		[(SF "s") (DF "")])
Index: gcc/testsuite/gcc.target/powerpc/pr79004.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/pr79004.c	(revision 0)
+++ gcc/testsuite/gcc.target/powerpc/pr79004.c	(revision 0)
@@ -0,0 +1,118 @@ 
+/* { dg-do compile { target { powerpc*-*-* && lp64 } } } */
+/* { dg-require-effective-target powerpc_float128_hw_ok } */
+/* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } { "-mcpu=power9" } } */
+/* { dg-options "-mcpu=power9 -O2" } */
+
+#include <math.h>
+
+#ifndef TYPE
+#define TYPE __float128
+#endif
+
+TYPE from_double (double a) { return (TYPE)a; }
+TYPE from_single (float a) { return (TYPE)a; }
+
+TYPE from_double_load (double *a) { return (TYPE)*a; }
+TYPE from_single_load (float *a) { return (TYPE)*a; }
+
+double to_double (TYPE a) { return (double)a; }
+float to_single (TYPE a) { return (float)a; }
+
+void to_double_store (TYPE a, double *p) { *p = (double)a; }
+void to_single_store (TYPE a, float *p) { *p = (float)a; }
+
+TYPE from_sign_char (signed char a) { return (TYPE)a; }
+TYPE from_sign_short (short a) { return (TYPE)a; }
+TYPE from_sign_int (int a) { return (TYPE)a; }
+TYPE from_sign_long (long a) { return (TYPE)a; }
+
+TYPE from_sign_char_load (signed char *a) { return (TYPE)*a; }
+TYPE from_sign_short_load (short *a) { return (TYPE)*a; }
+TYPE from_sign_int_load (int *a) { return (TYPE)*a; }
+TYPE from_sign_long_load (long *a) { return (TYPE)*a; }
+
+TYPE from_sign_char_load_4 (signed char *a) { return (TYPE)a[4]; }
+TYPE from_sign_short_load_4 (short *a) { return (TYPE)a[4]; }
+TYPE from_sign_int_load_4 (int *a) { return (TYPE)a[4]; }
+TYPE from_sign_long_load_4 (long *a) { return (TYPE)a[4]; }
+
+TYPE from_sign_char_load_n (signed char *a, long n) { return (TYPE)a[n]; }
+TYPE from_sign_short_load_n (short *a, long n) { return (TYPE)a[n]; }
+TYPE from_sign_int_load_n (int *a, long n) { return (TYPE)a[n]; }
+TYPE from_sign_long_load_n (long *a, long n) { return (TYPE)a[n]; }
+
+signed char to_sign_char (TYPE a) { return (signed char)a; }
+short to_sign_short (TYPE a) { return (short)a; }
+int to_sign_int (TYPE a) { return (int)a; }
+long to_sign_long (TYPE a) { return (long)a; }
+
+void to_sign_char_store (TYPE a, signed char *p) { *p = (signed char)a; }
+void to_sign_short_store (TYPE a, short *p) { *p = (short)a; }
+void to_sign_int_store (TYPE a, int *p) { *p = (int)a; }
+void to_sign_long_store (TYPE a, long *p) { *p = (long)a; }
+
+void to_sign_char_store_4 (TYPE a, signed char *p) { p[4] = (signed char)a; }
+void to_sign_short_store_4 (TYPE a, short *p) { p[4] = (short)a; }
+void to_sign_int_store_4 (TYPE a, int *p) { p[4] = (int)a; }
+void to_sign_long_store_4 (TYPE a, long *p) { p[4] = (long)a; }
+
+void to_sign_char_store_n (TYPE a, signed char *p, long n) { p[n] = (signed char)a; }
+void to_sign_short_store_n (TYPE a, short *p, long n) { p[n] = (short)a; }
+void to_sign_int_store_n (TYPE a, int *p, long n) { p[n] = (int)a; }
+void to_sign_long_store_n (TYPE a, long *p, long n) { p[n] = (long)a; }
+
+TYPE from_uns_char (unsigned char a) { return (TYPE)a; }
+TYPE from_uns_short (unsigned short a) { return (TYPE)a; }
+TYPE from_uns_int (unsigned int a) { return (TYPE)a; }
+TYPE from_uns_long (unsigned long a) { return (TYPE)a; }
+
+TYPE from_uns_char_load (unsigned char *a) { return (TYPE)*a; }
+TYPE from_uns_short_load (unsigned short *a) { return (TYPE)*a; }
+TYPE from_uns_int_load (unsigned int *a) { return (TYPE)*a; }
+TYPE from_uns_long_load (unsigned long *a) { return (TYPE)*a; }
+
+TYPE from_uns_char_load_4 (unsigned char *a) { return (TYPE)a[4]; }
+TYPE from_uns_short_load_4 (unsigned short *a) { return (TYPE)a[4]; }
+TYPE from_uns_int_load_4 (unsigned int *a) { return (TYPE)a[4]; }
+TYPE from_uns_long_load_4 (unsigned long *a) { return (TYPE)a[4]; }
+
+TYPE from_uns_char_load_n (unsigned char *a, long n) { return (TYPE)a[n]; }
+TYPE from_uns_short_load_n (unsigned short *a, long n) { return (TYPE)a[n]; }
+TYPE from_uns_int_load_n (unsigned int *a, long n) { return (TYPE)a[n]; }
+TYPE from_uns_long_load_n (unsigned long *a, long n) { return (TYPE)a[n]; }
+
+unsigned char to_uns_char (TYPE a) { return (unsigned char)a; }
+unsigned short to_uns_short (TYPE a) { return (unsigned short)a; }
+unsigned int to_uns_int (TYPE a) { return (unsigned int)a; }
+unsigned long to_uns_long (TYPE a) { return (unsigned long)a; }
+
+void to_uns_char_store (TYPE a, unsigned char *p) { *p = (unsigned char)a; }
+void to_uns_short_store (TYPE a, unsigned short *p) { *p = (unsigned short)a; }
+void to_uns_int_store (TYPE a, unsigned int *p) { *p = (unsigned int)a; }
+void to_uns_long_store (TYPE a, unsigned long *p) { *p = (unsigned long)a; }
+
+void to_uns_char_store_4 (TYPE a, unsigned char *p) { p[4] = (unsigned char)a; }
+void to_uns_short_store_4 (TYPE a, unsigned short *p) { p[4] = (unsigned short)a; }
+void to_uns_int_store_4 (TYPE a, unsigned int *p) { p[4] = (unsigned int)a; }
+void to_uns_long_store_4 (TYPE a, unsigned long *p) { p[4] = (unsigned long)a; }
+
+void to_uns_char_store_n (TYPE a, unsigned char *p, long n) { p[n] = (unsigned char)a; }
+void to_uns_short_store_n (TYPE a, unsigned short *p, long n) { p[n] = (unsigned short)a; }
+void to_uns_int_store_n (TYPE a, unsigned int *p, long n) { p[n] = (unsigned int)a; }
+void to_uns_long_store_n (TYPE a, unsigned long *p, long n) { p[n] = (unsigned long)a; }
+
+/* { dg-final { scan-assembler-not " bl __"    } } */
+/* { dg-final { scan-assembler     "xscvdpqp"  } } */
+/* { dg-final { scan-assembler     "xscvqpdp"  } } */
+/* { dg-final { scan-assembler     "xscvqpdpo" } } */
+/* { dg-final { scan-assembler     "xscvqpsdz" } } */
+/* { dg-final { scan-assembler     "xscvqpswz" } } */
+/* { dg-final { scan-assembler     "xscvsdqp"  } } */
+/* { dg-final { scan-assembler     "xscvudqp"  } } */
+/* { dg-final { scan-assembler     "lxsd"      } } */
+/* { dg-final { scan-assembler     "lxsiwax"   } } */
+/* { dg-final { scan-assembler     "lxsiwzx"   } } */
+/* { dg-final { scan-assembler     "lxssp"     } } */
+/* { dg-final { scan-assembler     "stxsd"     } } */
+/* { dg-final { scan-assembler     "stxsiwx"   } } */
+/* { dg-final { scan-assembler     "stxssp"    } } */