[3/3] PowerPC future: Add IEEE 128-bit min, max, compare.

Message ID 1591041713-24527-4-git-send-email-meissner@linux.ibm.com
State New
Headers show
Series
  • [1/3] PowerPC future: Add byte swap insns
Related show

Commit Message

Kees Cook via Gcc-patches June 1, 2020, 8:01 p.m.
Add support for the new IEEE 128-bit minimum, maximum, and set compare mask
instructions when -mcpu=future was used.

gcc/
2020-06-01  Michael Meissner  <meissner@linux.ibm.com>

	* config/rs6000/rs6000.c (rs6000_emit_hw_fp_minmax): Update
	comment.
	(rs6000_emit_hw_fp_cmove): Update comment.
	(rs6000_emit_cmove): Add support for IEEE 128-bit min, max, and
	comparisons with -mcpu=future.
	(rs6000_emit_minmax): Add support for IEEE 128-bit min/max with
	-mcpu=future.
	* config/rs6000/rs6000.md (s<minmax><mode>3, IEEE128 iterator):
	New insns for IEEE 128-bit min/max.
	(mov<mode>cc, IEEE128 iterator): New insns for IEEE 128-bit
	conditional move.
	(mov<mode>cc_future, IEEE128 iterator): New insns for IEEE 128-bit
	conditional move.
	(mov<mode>cc_invert_future, IEEE128 iterator): New insns for IEEE
	128-bit conditional move.
	(fpmask<mode>, IEEE128 iterator): New insns for IEEE 128-bit
	conditional move.

testsuite/
2020-06-01  Michael Meissner  <meissner@linux.ibm.com>

	* gcc.target/powerpc/float128-minmax-2.c: New test.
---
 gcc/config/rs6000/rs6000.c                         |  26 ++++-
 gcc/config/rs6000/rs6000.md                        | 121 +++++++++++++++++++++
 .../gcc.target/powerpc/float128-minmax-2.c         |  70 ++++++++++++
 3 files changed, 214 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/powerpc/float128-minmax-2.c

-- 
1.8.3.1

Comments

Kees Cook via Gcc-patches June 1, 2020, 11:24 p.m. | #1
On Mon, 2020-06-01 at 16:01 -0400, Michael Meissner via Gcc-patches wrote:
> Add support for the new IEEE 128-bit minimum, maximum, and set compare mask

> instructions when -mcpu=future was used.

> 

> gcc/

> 2020-06-01  Michael Meissner  <meissner@linux.ibm.com>

> 

> 	* config/rs6000/rs6000.c (rs6000_emit_hw_fp_minmax): Update

> 	comment.

> 	(rs6000_emit_hw_fp_cmove): Update comment.

> 	(rs6000_emit_cmove): Add support for IEEE 128-bit min, max, and

> 	comparisons with -mcpu=future.

> 	(rs6000_emit_minmax): Add support for IEEE 128-bit min/max with

> 	-mcpu=future.

> 	* config/rs6000/rs6000.md (s<minmax><mode>3, IEEE128 iterator):

> 	New insns for IEEE 128-bit min/max.

> 	(mov<mode>cc, IEEE128 iterator): New insns for IEEE 128-bit

> 	conditional move.

> 	(mov<mode>cc_future, IEEE128 iterator): New insns for IEEE 128-bit

> 	conditional move.

> 	(mov<mode>cc_invert_future, IEEE128 iterator): New insns for IEEE

> 	128-bit conditional move.

> 	(fpmask<mode>, IEEE128 iterator): New insns for IEEE 128-bit

> 	conditional move.


Include the leading wildcard here?  
	(*fpmask<mode> ...
and missing an entry for this one:
	(*xxsel<mode> ...


> 

> testsuite/

> 2020-06-01  Michael Meissner  <meissner@linux.ibm.com>

> 

> 	* gcc.target/powerpc/float128-minmax-2.c: New test.

> ---

>  gcc/config/rs6000/rs6000.c                         |  26 ++++-

>  gcc/config/rs6000/rs6000.md                        | 121 +++++++++++++++++++++

>  .../gcc.target/powerpc/float128-minmax-2.c         |  70 ++++++++++++

>  3 files changed, 214 insertions(+), 3 deletions(-)

>  create mode 100644 gcc/testsuite/gcc.target/powerpc/float128-minmax-2.c

> 

> diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c

> index 0921328..bbba8f1 100644

> --- a/gcc/config/rs6000/rs6000.c

> +++ b/gcc/config/rs6000/rs6000.c

> @@ -14847,7 +14847,9 @@ rs6000_emit_vector_cond_expr (rtx dest, rtx op_true, rtx op_false,

>  /* ISA 3.0 (power9) minmax subcase to emit a XSMAXCDP or XSMINCDP instruction

>     for SF/DF scalars.  Move TRUE_COND to DEST if OP of the operands of the last

>     comparison is nonzero/true, FALSE_COND if it is zero/false.  Return 0 if the

> -   hardware has no such operation.  */

> +   hardware has no such operation.

> +

> +   Under FUTURE, also handle IEEE 128-bit floating point.  */


> 

>  static int

>  rs6000_emit_hw_fp_minmax (rtx dest, rtx op, rtx true_cond, rtx false_cond)

> @@ -14889,7 +14891,9 @@ rs6000_emit_hw_fp_minmax (rtx dest, rtx op, rtx true_cond, rtx false_cond)

>  /* ISA 3.0 (power9) conditional move subcase to emit XSCMP{EQ,GE,GT,NE}DP and

>     XXSEL instructions for SF/DF scalars.  Move TRUE_COND to DEST if OP of the

>     operands of the last comparison is nonzero/true, FALSE_COND if it is

> -   zero/false.  Return 0 if the hardware has no such operation.  */

> +   zero/false.  Return 0 if the hardware has no such operation.

> +

> +   Under FUTURE, also handle IEEE 128-bit conditional moves.  */

> 

>  static int

>  rs6000_emit_hw_fp_cmove (rtx dest, rtx op, rtx true_cond, rtx false_cond)

> @@ -14981,6 +14985,21 @@ rs6000_emit_cmove (rtx dest, rtx op, rtx true_cond, rtx false_cond)

>  	return 1;

>      }

> 

> +  /* See if we can use the FUTURE min/max/compare instructions for IEEE 128-bit

> +     floating point.  At present, don't worry about doing conditional moves

> +     with different types for the comparison and movement (unlike SF/DF, where

> +     you can do a conditional test between double and use float as the if/then

> +     parts. */


Why don't we worry about that now?  Should this be a 'future todo'
comment here?


Beyond those nits and questions, lgtm, 
Thanks,
-Will
Segher Boessenkool June 9, 2020, 10:10 p.m. | #2
Hi!

Everything Will said included by reference :-)

On Mon, Jun 01, 2020 at 04:01:53PM -0400, Michael Meissner wrote:
> Add support for the new IEEE 128-bit minimum, maximum, and set compare mask

> instructions when -mcpu=future was used.


You can say "ISA 3.1 instructions" now :-)  That may read better in the
commit message?  OTOH, we'll keep using the "future" name for this in
the actual code a little longer, until everything is in, to keep things
easier for everyone (and then change everything at once).

> --- a/gcc/config/rs6000/rs6000.c

> +++ b/gcc/config/rs6000/rs6000.c

> @@ -14847,7 +14847,9 @@ rs6000_emit_vector_cond_expr (rtx dest, rtx op_true, rtx op_false,

>  /* ISA 3.0 (power9) minmax subcase to emit a XSMAXCDP or XSMINCDP instruction

>     for SF/DF scalars.  Move TRUE_COND to DEST if OP of the operands of the last

>     comparison is nonzero/true, FALSE_COND if it is zero/false.  Return 0 if the

> -   hardware has no such operation.  */

> +   hardware has no such operation.

> +

> +   Under FUTURE, also handle IEEE 128-bit floating point.  */


Don't say this please: if you do, eventually we'll end up with a huge
list of how every function does its work for every ISA version.  Instead,
describe it at a higher level, if you want to say anything at all?
Replace the "SF/DF" with "supported scalar floating point modes" or such.

> @@ -14981,6 +14985,21 @@ rs6000_emit_cmove (rtx dest, rtx op, rtx true_cond, rtx false_cond)

>  	return 1;

>      }

>  

> +  /* See if we can use the FUTURE min/max/compare instructions for IEEE 128-bit

> +     floating point.  At present, don't worry about doing conditional moves

> +     with different types for the comparison and movement (unlike SF/DF, where

> +     you can do a conditional test between double and use float as the if/then

> +     parts. */

> +  if (TARGET_FUTURE && FLOAT128_IEEE_P (compare_mode)

> +      && compare_mode == result_mode)

> +    {

> +      if (rs6000_emit_hw_fp_minmax (dest, op, true_cond, false_cond))

> +	return 1;

> +

> +      if (rs6000_emit_hw_fp_cmove (dest, op, true_cond, false_cond))

> +	return 1;

> +    }


"emit_hw_minmax" sounds completely wrong to implement a conditional move.
Presumably the name of that function isn't really what it does?  It would
make a lot more sense already if there was "try" in the name.  A function
called "emit" should not return a bool saying if it did the job -- if it
didn't it should ICE!

> +(define_insn "*xxsel<mode>"

> +  [(set (match_operand:IEEE128 0 "vsx_register_operand" "=wa")

> +	(if_then_else:IEEE128

> +	 (ne (match_operand:V2DI 1 "vsx_register_operand" "wa")

> +	     (match_operand:V2DI 2 "zero_constant" ""))


Leave out the constraint string completely?

> --- /dev/null

> +++ b/gcc/testsuite/gcc.target/powerpc/float128-minmax-2.c

> @@ -0,0 +1,70 @@

> +/* { dg-do compile { target { powerpc*-*-* && lp64 } } } */


Everything in this dir is always only used for powerpc targets; don't
test for that again per testcase please.

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

> +/* { dg-options "-mdejagnu-cpu=future -O2 -ffast-math" } */

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

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

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

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

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

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

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


\m and \M please.  And could you use scan-assembler-times here?  If so,
please do (all of this is very general advice).


Segher

Patch

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 0921328..bbba8f1 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -14847,7 +14847,9 @@  rs6000_emit_vector_cond_expr (rtx dest, rtx op_true, rtx op_false,
 /* ISA 3.0 (power9) minmax subcase to emit a XSMAXCDP or XSMINCDP instruction
    for SF/DF scalars.  Move TRUE_COND to DEST if OP of the operands of the last
    comparison is nonzero/true, FALSE_COND if it is zero/false.  Return 0 if the
-   hardware has no such operation.  */
+   hardware has no such operation.
+
+   Under FUTURE, also handle IEEE 128-bit floating point.  */
 
 static int
 rs6000_emit_hw_fp_minmax (rtx dest, rtx op, rtx true_cond, rtx false_cond)
@@ -14889,7 +14891,9 @@  rs6000_emit_hw_fp_minmax (rtx dest, rtx op, rtx true_cond, rtx false_cond)
 /* ISA 3.0 (power9) conditional move subcase to emit XSCMP{EQ,GE,GT,NE}DP and
    XXSEL instructions for SF/DF scalars.  Move TRUE_COND to DEST if OP of the
    operands of the last comparison is nonzero/true, FALSE_COND if it is
-   zero/false.  Return 0 if the hardware has no such operation.  */
+   zero/false.  Return 0 if the hardware has no such operation.
+
+   Under FUTURE, also handle IEEE 128-bit conditional moves.  */
 
 static int
 rs6000_emit_hw_fp_cmove (rtx dest, rtx op, rtx true_cond, rtx false_cond)
@@ -14981,6 +14985,21 @@  rs6000_emit_cmove (rtx dest, rtx op, rtx true_cond, rtx false_cond)
 	return 1;
     }
 
+  /* See if we can use the FUTURE min/max/compare instructions for IEEE 128-bit
+     floating point.  At present, don't worry about doing conditional moves
+     with different types for the comparison and movement (unlike SF/DF, where
+     you can do a conditional test between double and use float as the if/then
+     parts. */
+  if (TARGET_FUTURE && FLOAT128_IEEE_P (compare_mode)
+      && compare_mode == result_mode)
+    {
+      if (rs6000_emit_hw_fp_minmax (dest, op, true_cond, false_cond))
+	return 1;
+
+      if (rs6000_emit_hw_fp_cmove (dest, op, true_cond, false_cond))
+	return 1;
+    }
+
   /* Don't allow using floating point comparisons for integer results for
      now.  */
   if (FLOAT_MODE_P (compare_mode) && !FLOAT_MODE_P (result_mode))
@@ -15204,7 +15223,8 @@  rs6000_emit_minmax (rtx dest, enum rtx_code code, rtx op0, rtx op1)
   /* VSX/altivec have direct min/max insns.  */
   if ((code == SMAX || code == SMIN)
       && (VECTOR_UNIT_ALTIVEC_OR_VSX_P (mode)
-	  || (mode == SFmode && VECTOR_UNIT_VSX_P (DFmode))))
+	  || (mode == SFmode && VECTOR_UNIT_VSX_P (DFmode))
+	  || (TARGET_FUTURE && FLOAT128_IEEE_P (mode))))
     {
       emit_insn (gen_rtx_SET (dest, gen_rtx_fmt_ee (code, mode, op0, op1)));
       return;
diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
index 3310b4b..ef82f11 100644
--- a/gcc/config/rs6000/rs6000.md
+++ b/gcc/config/rs6000/rs6000.md
@@ -14645,6 +14645,127 @@  (define_insn "*cmp<mode>_hw"
    "xscmpuqp %0,%1,%2"
   [(set_attr "type" "veccmp")
    (set_attr "size" "128")])
+
+;; IEEE 128-bit min/max
+(define_insn "s<minmax><mode>3"
+  [(set (match_operand:IEEE128 0 "altivec_register_operand" "=v")
+	(fp_minmax:IEEE128
+	 (match_operand:IEEE128 1 "altivec_register_operand" "v")
+	 (match_operand:IEEE128 2 "altivec_register_operand" "v")))]
+  "TARGET_FUTURE && FLOAT128_IEEE_P (<MODE>mode)"
+  "xs<minmax>cqp %0,%1,%2"
+  [(set_attr "type" "fp")
+   (set_attr "size" "128")])
+
+;; IEEE 128-bit conditional move.  At present, don't worry about doing
+;; conditional moves with different types for the comparison and movement
+;; (unlike SF/DF, where you can do a conditional test between double and use
+;; float as the if/then parts.
+(define_expand "mov<mode>cc"
+   [(set (match_operand:IEEE128 0 "gpc_reg_operand")
+	 (if_then_else:IEEE128 (match_operand 1 "comparison_operator")
+			       (match_operand:IEEE128 2 "gpc_reg_operand")
+			       (match_operand:IEEE128 3 "gpc_reg_operand")))]
+  "TARGET_FUTURE && FLOAT128_IEEE_P (<MODE>mode)"
+{
+  if (rs6000_emit_cmove (operands[0], operands[1], operands[2], operands[3]))
+    DONE;
+  else
+    FAIL;
+})
+
+(define_insn_and_split "*mov<mode>cc_future"
+  [(set (match_operand:IEEE128 0 "altivec_register_operand" "=&v,v")
+	(if_then_else:IEEE128
+	 (match_operator:CCFP 1 "fpmask_comparison_operator"
+		[(match_operand:IEEE128 2 "altivec_register_operand" "v,v")
+		 (match_operand:IEEE128 3 "altivec_register_operand" "v,v")])
+	 (match_operand:IEEE128 4 "vsx_register_operand" "wa,wa")
+	 (match_operand:IEEE128 5 "vsx_register_operand" "wa,wa")))
+   (clobber (match_scratch:V2DI 6 "=0,&wa"))]
+  "TARGET_FUTURE"
+  "#"
+  ""
+  [(set (match_dup 6)
+	(if_then_else:V2DI (match_dup 1)
+			   (match_dup 7)
+			   (match_dup 8)))
+   (set (match_dup 0)
+	(if_then_else:IEEE128 (ne (match_dup 6)
+				  (match_dup 8))
+			      (match_dup 4)
+			      (match_dup 5)))]
+{
+  if (GET_CODE (operands[6]) == SCRATCH)
+    operands[6] = gen_reg_rtx (V2DImode);
+
+  operands[7] = CONSTM1_RTX (V2DImode);
+  operands[8] = CONST0_RTX (V2DImode);
+}
+ [(set_attr "length" "8")
+  (set_attr "type" "vecperm")])
+
+;; Handle inverting the fpmask comparisons.
+(define_insn_and_split "*mov<mode>cc_invert_future"
+  [(set (match_operand:IEEE128 0 "altivec_register_operand" "=&v,v")
+	(if_then_else:IEEE128
+	 (match_operator:CCFP 1 "invert_fpmask_comparison_operator"
+		[(match_operand:IEEE128 2 "altivec_register_operand" "v,v")
+		 (match_operand:IEEE128 3 "altivec_register_operand" "v,v")])
+	 (match_operand:IEEE128 4 "vsx_register_operand" "wa,wa")
+	 (match_operand:IEEE128 5 "vsx_register_operand" "wa,wa")))
+   (clobber (match_scratch:V2DI 6 "=0,&wa"))]
+  "TARGET_FUTURE"
+  "#"
+  "&& 1"
+  [(set (match_dup 6)
+	(if_then_else:V2DI (match_dup 9)
+			   (match_dup 7)
+			   (match_dup 8)))
+   (set (match_dup 0)
+	(if_then_else:IEEE128 (ne (match_dup 6)
+				  (match_dup 8))
+			      (match_dup 5)
+			      (match_dup 4)))]
+{
+  rtx op1 = operands[1];
+  enum rtx_code cond = reverse_condition_maybe_unordered (GET_CODE (op1));
+
+  if (GET_CODE (operands[6]) == SCRATCH)
+    operands[6] = gen_reg_rtx (V2DImode);
+
+  operands[7] = CONSTM1_RTX (V2DImode);
+  operands[8] = CONST0_RTX (V2DImode);
+
+  operands[9] = gen_rtx_fmt_ee (cond, CCFPmode, operands[2], operands[3]);
+}
+ [(set_attr "length" "8")
+  (set_attr "type" "vecperm")])
+
+(define_insn "*fpmask<mode>"
+  [(set (match_operand:V2DI 0 "altivec_register_operand" "=v")
+	(if_then_else:V2DI
+	 (match_operator:CCFP 1 "fpmask_comparison_operator"
+		[(match_operand:IEEE128 2 "altivec_register_operand" "v")
+		 (match_operand:IEEE128 3 "altivec_register_operand" "v")])
+	 (match_operand:V2DI 4 "all_ones_constant" "")
+	 (match_operand:V2DI 5 "zero_constant" "")))]
+  "TARGET_FUTURE && FLOAT128_IEEE_P (<MODE>mode)"
+  "xscmp%V1qp %0,%2,%3"
+  [(set_attr "type" "fpcompare")
+   (set_attr "size" "128")])
+
+(define_insn "*xxsel<mode>"
+  [(set (match_operand:IEEE128 0 "vsx_register_operand" "=wa")
+	(if_then_else:IEEE128
+	 (ne (match_operand:V2DI 1 "vsx_register_operand" "wa")
+	     (match_operand:V2DI 2 "zero_constant" ""))
+	 (match_operand:IEEE128 3 "vsx_register_operand" "wa")
+	 (match_operand:IEEE128 4 "vsx_register_operand" "wa")))]
+  "TARGET_FUTURE && FLOAT128_IEEE_P (<MODE>mode)"
+  "xxsel %x0,%x4,%x3,%x1"
+  [(set_attr "type" "vecmove")
+   (set_attr "size" "128")])
 
 ;; Miscellaneous ISA 3.0 (power9) instructions
 
diff --git a/gcc/testsuite/gcc.target/powerpc/float128-minmax-2.c b/gcc/testsuite/gcc.target/powerpc/float128-minmax-2.c
new file mode 100644
index 0000000..a8db336
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/float128-minmax-2.c
@@ -0,0 +1,70 @@ 
+/* { dg-do compile { target { powerpc*-*-* && lp64 } } } */
+/* { dg-require-effective-target powerpc_future_ok } */
+/* { dg-options "-mdejagnu-cpu=future -O2 -ffast-math" } */
+/* { dg-final { scan-assembler-not "xscmpuqp"  } } */
+/* { dg-final { scan-assembler     "xscmpeqqp" } } */
+/* { dg-final { scan-assembler     "xscmpgtqp" } } */
+/* { dg-final { scan-assembler     "xscmpgeqp" } } */
+/* { dg-final { scan-assembler     "xsmaxcqp"  } } */
+/* { dg-final { scan-assembler     "xsmincqp"  } } */
+/* { dg-final { scan-assembler     "xxsel"     } } */
+
+__float128
+f128_max1 (__float128 a, __float128 b)
+{
+  return (a >= b) ? a : b;
+}
+
+__float128
+f128_max2 (__float128 a, __float128 b)
+{
+  return (a > b) ? a : b;
+}
+
+__float128
+f128_min1 (__float128 a, __float128 b)
+{
+  return (a < b) ? a : b;
+}
+
+__float128
+f128_min2 (__float128 a, __float128 b)
+{
+  return (a <= b) ? a : b;
+}
+
+__float128
+f128_cmp_eq (__float128 a, __float128 b, __float128 c, __float128 d)
+{
+  return (a == b) ? c : d;
+}
+
+__float128
+f128_cmp_ne (__float128 a, __float128 b, __float128 c, __float128 d)
+{
+  return (a != b) ? c : d;
+}
+
+__float128
+f128_cmp_gt (__float128 a, __float128 b, __float128 c, __float128 d)
+{
+  return (a > b) ? c : d;
+}
+
+__float128
+f128_cmp_ge (__float128 a, __float128 b, __float128 c, __float128 d)
+{
+  return (a >= b) ? c : d;
+}
+
+__float128
+f128_cmp_lt (__float128 a, __float128 b, __float128 c, __float128 d)
+{
+  return (a < b) ? c : d;
+}
+
+__float128
+f128_cmp_le (__float128 a, __float128 b, __float128 c, __float128 d)
+{
+  return (a <= b) ? c : d;
+}