diff mbox

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

Message ID 20170118003905.GA19077@ibm-tiger.the-meissners.org
State New
Headers show

Commit Message

Michael Meissner Jan. 18, 2017, 12:39 a.m. UTC 
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  <meissner@linux.vnet.ibm.com>

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

Comments

Segher Boessenkool Jan. 18, 2017, 1 a.m. UTC | #1 
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
Michael Meissner Jan. 18, 2017, 1:18 a.m. UTC | #2 
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. :-)
diff mbox

Patch

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 <math.h>
 
@@ -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}   } } */
+/* { dg-final { scan-assembler     {\mlxssp\M}     } } */
+/* { dg-final { scan-assembler     {\mstxsd\M}     } } */
+/* { dg-final { scan-assembler     {\mstxsiwx\M}   } } */
+/* { dg-final { scan-assembler     {\mstxssp\M}    } } */