Re: [PATCH] PR target/79004, Fix char/short -> _Float128 on PowerPC -mcpu=power9
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
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
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
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
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
