diff mbox

, PR target/79038, Improve char/short -> _Float128 conversions

Message ID 20170303181018.GA18208@ibm-tiger.the-meissners.org
State New
Headers show

Commit Message

Michael Meissner March 3, 2017, 6:10 p.m. UTC 
On January 31st I commited a patch that improves conversion of signed/unsigned
char/short values to 32-bit and 64-bit floating point on Power9, particularly
when the values are coming from memory.  This adds similar support to
_Float128/__float128 (i.e. IEEE 128-bit floating point).  I have tested this
patch via bootstrap and make check and there were no regressions.  Can I check
this patch into the trunk?

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

	PR target/79038
	* config/rs6000/rs6000.md (float<QHI:mode><IEEE128:mode>2): Define
	insns to convert from signed/unsigned char/short to IEEE 128-bit
	floating point.
	(floatuns<QHI:mode><IEEE128:mode>2): Likewise.

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

	PR target/79038
	* gcc.target/powerpc/pr79038-1.c: New test.

Comments

Segher Boessenkool March 9, 2017, 6:45 p.m. UTC | #1 
On Fri, Mar 03, 2017 at 01:10:18PM -0500, Michael Meissner wrote:
> On January 31st I commited a patch that improves conversion of signed/unsigned
> char/short values to 32-bit and 64-bit floating point on Power9, particularly
> when the values are coming from memory.  This adds similar support to
> _Float128/__float128 (i.e. IEEE 128-bit floating point).  I have tested this
> patch via bootstrap and make check and there were no regressions.  Can I check
> this patch into the trunk?

Okay.  If it breaks anything it will only be ieee128.

The usual question about the testcase:

> --- gcc/testsuite/gcc.target/powerpc/pr79038-1.c	(revision 0)
> +++ gcc/testsuite/gcc.target/powerpc/pr79038-1.c	(revision 0)
> @@ -0,0 +1,39 @@
> +/* { dg-do compile { target { powerpc*-*-* && lp64 } } } */

Why does it only work on 64-bit?

> +/* { dg-require-effective-target powerpc_p9vector_ok } */

This is a compile-only test; why this then?

> +/* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } { "-mcpu=power9" } } */
> +/* { dg-options "-mcpu=power9 -O2 -mfloat128" } */


Segher
Michael Meissner March 9, 2017, 10:58 p.m. UTC | #2 
On Thu, Mar 09, 2017 at 12:45:00PM -0600, Segher Boessenkool wrote:
> On Fri, Mar 03, 2017 at 01:10:18PM -0500, Michael Meissner wrote:
> > On January 31st I commited a patch that improves conversion of signed/unsigned
> > char/short values to 32-bit and 64-bit floating point on Power9, particularly
> > when the values are coming from memory.  This adds similar support to
> > _Float128/__float128 (i.e. IEEE 128-bit floating point).  I have tested this
> > patch via bootstrap and make check and there were no regressions.  Can I check
> > this patch into the trunk?
> 
> Okay.  If it breaks anything it will only be ieee128.
> 
> The usual question about the testcase:
> 
> > --- gcc/testsuite/gcc.target/powerpc/pr79038-1.c	(revision 0)
> > +++ gcc/testsuite/gcc.target/powerpc/pr79038-1.c	(revision 0)
> > @@ -0,0 +1,39 @@
> > +/* { dg-do compile { target { powerpc*-*-* && lp64 } } } */
> 
> Why does it only work on 64-bit?

Because the particular optimization requires QImode and HImode values to be
allowed in vector registers, and that in turn depends on 64-bit because we
don't have all of the necessary bits for doing 64-bit direct moves on 32-bit
systems.

> 
> > +/* { dg-require-effective-target powerpc_p9vector_ok } */
> 
> This is a compile-only test; why this then?

I meant to use the target for system has power9 target support.
> 
> > +/* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } { "-mcpu=power9" } } */
> > +/* { dg-options "-mcpu=power9 -O2 -mfloat128" } */
> 
> 
> Segher
>
diff mbox

Patch

Index: gcc/config/rs6000/rs6000.md
===================================================================
--- gcc/config/rs6000/rs6000.md	(revision 245815)
+++ gcc/config/rs6000/rs6000.md	(working copy)
@@ -14467,6 +14467,43 @@  (define_insn_and_split "float_<mode>si2_
     operands[2] = gen_reg_rtx (DImode);
 })
 
+(define_insn_and_split "float<QHI:mode><IEEE128:mode>2"
+  [(set (match_operand:IEEE128 0 "altivec_register_operand" "=v,v,v")
+	(float:IEEE128 (match_operand:QHI 1 "nonimmediate_operand" "v,r,Z")))
+   (clobber (match_scratch:DI 2 "=X,r,X"))]
+  "TARGET_FLOAT128_HW && FLOAT128_IEEE_P (<IEEE128:MODE>mode)"
+  "#"
+  "&& reload_completed"
+  [(const_int 0)]
+{
+  rtx dest = operands[0];
+  rtx src = operands[1];
+  rtx dest_di = gen_rtx_REG (DImode, REGNO (dest));
+
+  if (altivec_register_operand (src, <QHI:MODE>mode))
+    emit_insn (gen_extend<QHI:mode>di2 (dest_di, src));
+  else if (int_reg_operand (src, <QHI:MODE>mode))
+    {
+      rtx ext_di = operands[2];
+      emit_insn (gen_extend<QHI:mode>di2 (ext_di, src));
+      emit_move_insn (dest_di, ext_di);
+    }
+  else if (MEM_P (src))
+    {
+      rtx dest_qhi = gen_rtx_REG (<QHI:MODE>mode, REGNO (dest));
+      emit_move_insn (dest_qhi, src);
+      emit_insn (gen_extend<QHI:mode>di2 (dest_di, dest_qhi));
+    }
+  else
+    gcc_unreachable ();
+
+  emit_insn (gen_float_<IEEE128:mode>di2_hw (dest, dest_di));
+  DONE;
+}
+  [(set_attr "length" "8,12,12")
+   (set_attr "type" "vecfloat")
+   (set_attr "size" "128")])
+
 (define_insn "floatuns_<mode>di2_hw"
   [(set (match_operand:IEEE128 0 "altivec_register_operand" "=v")
 	(unsigned_float:IEEE128
@@ -14493,6 +14530,38 @@  (define_insn_and_split "floatuns_<mode>s
     operands[2] = gen_reg_rtx (DImode);
 })
 
+(define_insn_and_split "floatuns<QHI:mode><IEEE128:mode>2"
+  [(set (match_operand:IEEE128 0 "altivec_register_operand" "=v,v,v")
+	(unsigned_float:IEEE128
+	 (match_operand:QHI 1 "nonimmediate_operand" "v,r,Z")))
+   (clobber (match_scratch:DI 2 "=X,r,X"))]
+  "TARGET_FLOAT128_HW && FLOAT128_IEEE_P (<IEEE128:MODE>mode)"
+  "#"
+  "&& reload_completed"
+  [(const_int 0)]
+{
+  rtx dest = operands[0];
+  rtx src = operands[1];
+  rtx dest_di = gen_rtx_REG (DImode, REGNO (dest));
+
+  if (altivec_register_operand (src, <QHI:MODE>mode) || MEM_P (src))
+    emit_insn (gen_zero_extend<QHI:mode>di2 (dest_di, src));
+  else if (int_reg_operand (src, <QHI:MODE>mode))
+    {
+      rtx ext_di = operands[2];
+      emit_insn (gen_zero_extend<QHI:mode>di2 (ext_di, src));
+      emit_move_insn (dest_di, ext_di);
+    }
+  else
+    gcc_unreachable ();
+
+  emit_insn (gen_floatuns_<IEEE128:mode>di2_hw (dest, dest_di));
+  DONE;
+}
+  [(set_attr "length" "8,12,8")
+   (set_attr "type" "vecfloat")
+   (set_attr "size" "128")])
+
 ;; IEEE 128-bit instructions with round to odd semantics
 (define_insn "*trunc<mode>df2_odd"
   [(set (match_operand:DF 0 "vsx_register_operand" "=v")
Index: gcc/testsuite/gcc.target/powerpc/pr79038-1.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/pr79038-1.c	(revision 0)
+++ gcc/testsuite/gcc.target/powerpc/pr79038-1.c	(revision 0)
@@ -0,0 +1,39 @@ 
+/* { 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 -mfloat128" } */
+
+#ifndef TYPE
+#define TYPE _Float128
+#endif
+
+TYPE
+return_convert_uchar_to_float128_mem (unsigned char *p_uc)
+{
+  return (TYPE) p_uc[10];
+}
+
+TYPE
+return_convert_schar_to_float128_mem (signed char *p_sc)
+{
+  return (TYPE) p_sc[10];
+}
+
+TYPE
+return_convert_ushort_to_float128_mem (unsigned short *p_us)
+{
+  return (TYPE) p_us[10];
+}
+
+TYPE
+return_convert_sshort_to_float128_mem (short *p_ss)
+{
+  return (TYPE) p_ss[10];
+}
+
+/* { dg-final { scan-assembler     {\mlxsi[bh]zx\M}  } } */
+/* { dg-final { scan-assembler     {\mvexts[bh]2d\M} } } */
+/* { dg-final { scan-assembler-not {\mextsb\M}       } } */
+/* { dg-final { scan-assembler-not {\ml[bh][az]\M}   } } */
+/* { dg-final { scan-assembler-not {\mmtvsrw[az]\M}  } } */
+