Re: [PATCH] PR target/79004, Fix char/short -> _Float128 on PowerPC -mcpu=power9

2017-01-17 Thread Michael Meissner 
On Tue, Jan 17, 2017 at 07:00:36PM -0600, Segher Boessenkool wrote:
> On Tue, Jan 17, 2017 at 07:39:05PM -0500, Michael Meissner wrote:
> > It turns out the testcase I submitted for pr79004 failed, since I had the 
> > wrong
> > syntax for \m and \M (you need to use {\m...\M} not "\m...\M".
> 
> You can use quotes, but then it is "\\m...\\M", like with all backslashes
> in quotes.  {} saves you from that headache.  "" has all three kinds of
> substitution applied to it; {} gets none.  "man tcl", the most enlightening
> 204 lines (many empty) about Tcl you'll ever read :-)

Yeah, I figured that I could do it with multiple \'s, but it was simpler to use
{} like the other examples, rather than figure out how many \'s are needed
(BTDT).

I don't remember whether tcl used grep, egrep, or perl style regexps, so I
don't tend to use the more complicated variants.

> 
> > I also forgot to add -mfloat128.  I committed this patch as obvious:
> 
> Thanks!

I did test it before submitting it. :-)

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

Re: [PATCH] PR target/79004, Fix char/short -> _Float128 on PowerPC -mcpu=power9

2017-01-17 Thread Segher Boessenkool 
On Tue, Jan 17, 2017 at 07:39:05PM -0500, Michael Meissner wrote:
> It turns out the testcase I submitted for pr79004 failed, since I had the 
> wrong
> syntax for \m and \M (you need to use {\m...\M} not "\m...\M".

You can use quotes, but then it is "\\m...\\M", like with all backslashes
in quotes.  {} saves you from that headache.  "" has all three kinds of
substitution applied to it; {} gets none.  "man tcl", the most enlightening
204 lines (many empty) about Tcl you'll ever read :-)

> I also forgot to add -mfloat128.  I committed this patch as obvious:

Thanks!


Segher

Re: [PATCH] PR target/79004, Fix char/short -> _Float128 on PowerPC -mcpu=power9

2017-01-17 Thread Michael Meissner 
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 .
> 
> > +/* { 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?

It turns out the testcase I submitted for pr79004 failed, since I had the wrong
syntax for \m and \M (you need to use {\m...\M} not "\m...\M".  I also forgot
to add -mfloat128.  I committed this patch as obvious:

2017-01-17  Michael Meissner  

PR target/79004
* gcc.target/powerpc/pr79004.c: Add -mfloat128 to the test
options.  Fix up the syntax for using \m and \M.

-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meiss...@linux.vnet.ibm.com, phone: +1 (978) 899-4797
Index: gcc/testsuite/gcc.target/powerpc/pr79004.c
===
--- gcc/testsuite/gcc.target/powerpc/pr79004.c  (revision 244555)
+++ gcc/testsuite/gcc.target/powerpc/pr79004.c  (working copy)
@@ -1,7 +1,7 @@
 /* { dg-do compile { target { powerpc*-*-* && lp64 } } } */
 /* { dg-require-effective-target powerpc_p9vector_ok } */
 /* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } { 
"-mcpu=power9" } } */
-/* { dg-options "-mcpu=power9 -O2" } */
+/* { dg-options "-mcpu=power9 -O2 -mfloat128" } */
 
 #include 
 
@@ -101,18 +101,18 @@
 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 "\mbl __"   } } */
-/* { dg-final { scan-assembler "\mxscvdpqp\M"  } } */
-/* { dg-final { scan-assembler "\mxscvqpdp\M"  } } */
-/* { dg-final { scan-assembler "\mxscvqpdpo\M" } } */
-/* { dg-final { scan-assembler "\mxscvqpsdz\M" } } */
-/* { dg-final { scan-assembler "\mxscvqpswz\M" } } */
-/* { dg-final { scan-assembler "\mxscvsdqp\M"  } } */
-/* { dg-final { scan-assembler "\mxscvudqp\M"  } } */
-/* { dg-final { scan-assembler "\mlxsd\M"  } } */
-/* { dg-final { scan-assembler "\mlxsiwax\M"   } } */
-/* { dg-final { scan-assembler "\mlxsiwzx\M"   } } */
-/* { dg-final { scan-assembler "\mlxssp\M" } } */
-/* { dg-final { scan-assembler "\mstxsd\M" } } */
-/* { dg-final { scan-assembler "\mstxsiwx\M"   } } */
-/* { dg-final { scan-assembler "\mstxssp\M"} } */
+/* { dg-final { scan-assembler-not {\mbl __}   } } */
+/* { dg-final { scan-assembler {\mxscvdpqp\M}  } } */
+/* { dg-final { scan-assembler {\mxscvqpdp\M}  } } */
+/* { dg-final { scan-assembler {\mxscvqpdpo\M} } } */
+/* { dg-final { scan-assembler {\mxscvqpsdz\M} } } */
+/* { dg-final { scan-assembler {\mxscvqpswz\M} } } */
+/* { dg-final { scan-assembler {\mxscvsdqp\M}  } } */
+/* { dg-final { scan-assembler {\mxscvudqp\M}  } } */
+/* { dg-final { scan-assembler {\mlxsd\M}  } } */
+/* { dg-final { scan-assembler {\mlxsiwax\M}   } } */
+/* { dg-final { scan-assembler {\mlxsiwzx\M}   } } */

Re: [PATCH] PR target/79004, Fix char/short -> _Float128 on PowerPC -mcpu=power9

2017-01-12 Thread Michael Meissner 
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).

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

Re: [PATCH] PR target/79004, Fix char/short -> _Float128 on PowerPC -mcpu=power9

2017-01-11 Thread Segher Boessenkool 
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

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

2017-01-09 Thread Michael Meissner 
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  

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  

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

-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meiss...@linux.vnet.ibm.com, phone: +1 (978) 899-4797
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 
+
+#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