[PATCH 0/3] Add Power10 IEEE 128-bit min, max, conditional move

[PATCH 0/3] Add Power10 IEEE 128-bit min, max, conditional move

[Michael Meissner]

This is a revision of the patches I sent on May 18th.

I tested it on 3 platforms:

    *	Power9 little endian, --with-code=power9;
    *	Power8 big endian, --with-code=power8, both 32/64-bit tests done;
    *	Power10 little endian, --with-code=power10.

All systems bootstrapped and there were no new regressions.  I believe I have
addressed the issues with the last patch.

The first patch in this set contains the same GCC code and new test as in the
previous patch, since I don't believe there was a problem with those bits.

I moved the changes for the existing test 'float128-minmax.c' to patch number
two.  Rather than using '#pragma GCC target' to force power9 code generation on
power10, instead I used conditional scan-assembler statements to deliniate the
power9 and power10 code generation.

The third patch of this set fixes the complicated test that was complained
about in the previous second patch.

Can I check these patches into the master branch.  Ideally, I think these
should go into GCC 11.2 after a soak-in period.

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

[PATCH 1/3] Add IEEE 128-bit min/max support on PowerPC.

[Michael Meissner]

[PATCH 1/3] Add IEEE 128-bit min/max support on PowerPC.

This patch adds the support for the IEEE 128-bit floating point C minimum and
maximum instructions.  The next patch will add the support for using the
compare and set mask instruction to implement conditional moves.

This patch does not try to re-use the code used for SF/DF min/max
support.  It defines a separate insn for the IEEE 128-bit support.  It
uses the code iterator <minmax> to simplify adding both operations.

GCC will not convert ternary operations into using min/max instructions
provided in this patch unless the user uses -Ofast or similar switches due to
issues with NaNs.  The next patch that adds conditional move instructions will
enable the ternary conversion in many cases.

Note the code for fixing float128-minmax.c has been moved to a separate
patch.

I tested it on 3 platforms:

    *	Power9 little endian, --with-code=power9;
    *	Power8 big endian, --with-code=power8, both 32/64-bit tests done;
    *	Power10 little endian, --with-code=power10.

All systems bootstrapped and there were no new regressions.  I believe I have
addressed the issues with the last patch.

Can I check this into the master branch, and after a soak-in period, back port
it to the GCC 11 branch?

gcc/
2021-06-08  Michael Meissner  <meissner@linux.ibm.com>

	* config/rs6000/rs6000.c (rs6000_emit_minmax): Add support for ISA
	3.1 IEEE 128-bit floating point xsmaxcqp and xsmincqp
	instructions.
	* config/rs6000/rs6000.md (s<minmax><mode>3, IEEE128 iterator):
	New insns.

gcc/testsuite/
2021-06-08  Michael Meissner  <meissner@linux.ibm.com>

	* gcc.target/powerpc/float128-minmax-2.c: New test.
---
 gcc/config/rs6000/rs6000.c                        |  3 ++-
 gcc/config/rs6000/rs6000.md                       | 11 +++++++++++
 .../gcc.target/powerpc/float128-minmax-2.c        | 15 +++++++++++++++
 3 files changed, 28 insertions(+), 1 deletion(-)
 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 b01bb5c8191..1651788df6a 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -16103,7 +16103,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_POWER10 && TARGET_FLOAT128_HW && 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 3f59b544f6a..064c3a2d9d6 100644
--- a/gcc/config/rs6000/rs6000.md
+++ b/gcc/config/rs6000/rs6000.md
@@ -5214,6 +5214,17 @@ (define_insn "*s<minmax><mode>3_vsx"
 }
   [(set_attr "type" "fp")])
 
+;; Min/max for ISA 3.1 IEEE 128-bit floating point
+(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_POWER10"
+  "xs<minmax>cqp %0,%1,%2"
+  [(set_attr "type" "vecfloat")
+   (set_attr "size" "128")])
+
 ;; The conditional move instructions allow us to perform max and min operations
 ;; even when we don't have the appropriate max/min instruction using the FSEL
 ;; instruction.
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 00000000000..c71ba08c9f8
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/float128-minmax-2.c
@@ -0,0 +1,15 @@
+/* { dg-require-effective-target ppc_float128_hw } */
+/* { dg-require-effective-target power10_ok } */
+/* { dg-options "-mdejagnu-cpu=power10 -O2 -ffast-math" } */
+
+#ifndef TYPE
+#define TYPE _Float128
+#endif
+
+/* Test that the fminf128/fmaxf128 functions generate if/then/else and not a
+   call.  */
+TYPE f128_min (TYPE a, TYPE b) { return __builtin_fminf128 (a, b); }
+TYPE f128_max (TYPE a, TYPE b) { return __builtin_fmaxf128 (a, b); }
+
+/* { dg-final { scan-assembler {\mxsmaxcqp\M} } } */
+/* { dg-final { scan-assembler {\mxsmincqp\M} } } */
-- 
2.31.1


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

[PATCH 2/3] Fix IEEE 128-bit min/max test.

[Michael Meissner]

[PATCH 2/3] Fix IEEE 128-bit min/max test.

This patch fixes the float128-minmax.c test so that it can accommodate the
generation of xsmincqp and xsmaxcqp instructions on power10.  I changed
the effective target from 'float128' to 'ppc_float128_hw', since this
needs the IEEE 128-bit float hardware support.

I tested it on 3 platforms:

    *	Power9 little endian, --with-code=power9;
    *	Power8 big endian, --with-code=power8, both 32/64-bit tests done;
    *	Power10 little endian, --with-code=power10.

All systems bootstrapped and there were no new regressions.  I believe I have
addressed the issues with the last patch.

Can I check this into the master branch, and after a soak-in period, back port
it to the GCC 11 branch?

gcc/testsuite/
2021-06-08  Michael Meissner  <meissner@linux.ibm.com>

	* gcc.target/powerpc/float128-minmax.c: Adjust expected code for
	power10.
	* lib/target-supports.exp (check_effective_target_has_arch_pwr10):
	New target support.
---
 gcc/testsuite/gcc.target/powerpc/float128-minmax.c |  8 +++++---
 gcc/testsuite/lib/target-supports.exp              | 10 ++++++++++
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/gcc/testsuite/gcc.target/powerpc/float128-minmax.c b/gcc/testsuite/gcc.target/powerpc/float128-minmax.c
index fe397518f2f..a7d3a3a0b3e 100644
--- a/gcc/testsuite/gcc.target/powerpc/float128-minmax.c
+++ b/gcc/testsuite/gcc.target/powerpc/float128-minmax.c
@@ -1,6 +1,5 @@
-/* { dg-do compile { target lp64 } } */
 /* { dg-require-effective-target powerpc_p9vector_ok } */
-/* { dg-require-effective-target float128 } */
+/* { dg-require-effective-target ppc_float128_hw } */
 /* { dg-options "-mpower9-vector -O2 -ffast-math" } */
 
 #ifndef TYPE
@@ -12,5 +11,8 @@
 TYPE f128_min (TYPE a, TYPE b) { return __builtin_fminf128 (a, b); }
 TYPE f128_max (TYPE a, TYPE b) { return __builtin_fmaxf128 (a, b); }
 
-/* { dg-final { scan-assembler     {\mxscmpuqp\M} } } */
+/* Adjust code power10 which has native min/max instructions.  */
+/* { dg-final { scan-assembler     {\mxscmpuqp\M} { target { ! has_arch_pwr10 } } } } */
+/* { dg-final { scan-assembler     {\mxsmincqp\M} { target {   has_arch_pwr10 } } } } */
+/* { dg-final { scan-assembler     {\mxsmaxcqp\M} { target {   has_arch_pwr10 } } } } */
 /* { dg-final { scan-assembler-not {\mbl\M}       } } */
diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
index 7f78c5593ac..789723fb287 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -6127,6 +6127,16 @@ proc check_effective_target_has_arch_pwr9 { } {
 	}]
 }
 
+proc check_effective_target_has_arch_pwr10 { } {
+	return [check_no_compiler_messages arch_pwr10 assembly {
+		#ifndef _ARCH_PWR10
+		#error does not have power10 support.
+		#else
+		/* "has power10 support" */
+		#endif
+	}]
+}
+
 # Return 1 if this is a PowerPC target supporting -mcpu=power10.
 # Limit this to 64-bit linux systems for now until other targets support
 # power10.
-- 
2.31.1


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

[PATCH 3/3] Add IEEE 128-bit fp conditional move on PowerPC.

[Michael Meissner]

[PATCH 3/3] Add IEEE 128-bit fp conditional move on PowerPC.

This patch adds the support for power10 IEEE 128-bit floating point conditional
move and for automatically generating min/max.

In this patch, I simplified things compared to previous patches.  Instead of
allowing any four of the modes to be used for the conditional move comparison
and the move itself could use different modes, I restricted the conditional
move to just the same mode.  I.e. you can do:

        _Float128 a, b, c, d, e, r;

        r = (a == b) ? c : d;

But you can't do:

        _Float128 c, d, r;
        double a, b;

        r = (a == b) ? c : d;

or:

        _Float128 a, b;
        double c, d, r;

        r = (a == b) ? c : d;

This eliminates a lot of the complexity of the code, because you don't have to
worry about the sizes being different, and the IEEE 128-bit types being
restricted to Altivec registers, while the SF/DF modes can use any VSX
register.

I did not modify the existing support that allowed conditional moves where
SFmode operands are compared and DFmode operands are moved (and vice versa).

Compared to the May 18th patches, this patch replaces the complicated test that
was complained about.

I tested it on 3 platforms:

    *	Power9 little endian, --with-code=power9;
    *	Power8 big endian, --with-code=power8, both 32/64-bit tests done;
    *	Power10 little endian, --with-code=power10.

All systems bootstrapped and there were no new regressions.  I believe I have
addressed the issues with the last patch.

Can I check this into the master branch, and after a soak-in period, back port
it to the GCC 11 branch?

gcc/
2021-06-08 Michael Meissner  <meissner@linux.ibm.com>

        * config/rs6000/rs6000.c (rs6000_maybe_emit_fp_cmove): Add IEEE
	128-bit floating point conditional move support.
	(have_compare_and_set_mask): Add IEEE 128-bit floating point
	types.
	* config/rs6000/rs6000.md (mov<mode>cc, IEEE128 iterator): New insn.
	(mov<mode>cc_p10, IEEE128 iterator): New insn.
	(mov<mode>cc_invert_p10, IEEE128 iterator): New insn.
	(fpmask<mode>, IEEE128 iterator): New insn.
	(xxsel<mode>, IEEE128 iterator): New insn.

gcc/testsuite/
2021-06-08  Michael Meissner  <meissner@linux.ibm.com>

        * gcc.target/powerpc/float128-cmove.c: New test.
        * gcc.target/powerpc/float128-minmax-3.c: New test.
---
 gcc/config/rs6000/rs6000.c                    |  38 ++++++-
 gcc/config/rs6000/rs6000.md                   | 106 ++++++++++++++++++
 .../gcc.target/powerpc/float128-cmove.c       |  58 ++++++++++
 .../gcc.target/powerpc/float128-minmax-3.c    |  15 +++
 4 files changed, 215 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/powerpc/float128-cmove.c
 create mode 100644 gcc/testsuite/gcc.target/powerpc/float128-minmax-3.c

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 1651788df6a..411e7539019 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -15698,8 +15698,8 @@ rs6000_emit_vector_cond_expr (rtx dest, rtx op_true, rtx op_false,
   return 1;
 }
 
-/* Possibly emit the xsmaxcdp and xsmincdp instructions to emit a maximum or
-   minimum with "C" semantics.
+/* Possibly emit the xsmaxc{dp,qp} and xsminc{dp,qp} instructions to emit a
+   maximum or minimum with "C" semantics.
 
    Unless you use -ffast-math, you can't use these instructions to replace
    conditions that implicitly reverse the condition because the comparison
@@ -15775,6 +15775,7 @@ rs6000_maybe_emit_fp_cmove (rtx dest, rtx op, rtx true_cond, rtx false_cond)
   enum rtx_code code = GET_CODE (op);
   rtx op0 = XEXP (op, 0);
   rtx op1 = XEXP (op, 1);
+  machine_mode compare_mode = GET_MODE (op0);
   machine_mode result_mode = GET_MODE (dest);
   rtx compare_rtx;
   rtx cmove_rtx;
@@ -15783,6 +15784,35 @@ rs6000_maybe_emit_fp_cmove (rtx dest, rtx op, rtx true_cond, rtx false_cond)
   if (!can_create_pseudo_p ())
     return 0;
 
+  /* We allow the comparison to be either SFmode/DFmode and the true/false
+     condition to be either SFmode/DFmode.  I.e. we allow:
+
+	float a, b;
+	double c, d, r;
+
+	r = (a == b) ? c : d;
+
+    and:
+
+	double a, b;
+	float c, d, r;
+
+	r = (a == b) ? c : d;
+
+    but we don't allow intermixing the IEEE 128-bit floating point types with
+    the 32/64-bit scalar types.
+
+    It gets too messy where SFmode/DFmode can use any register and TFmode/KFmode
+    can only use Altivec registers.  In addtion, we would need to do a XXPERMDI
+    if we compare SFmode/DFmode and move TFmode/KFmode.  */
+
+  if (compare_mode == result_mode
+      || (compare_mode == SFmode && result_mode == DFmode)
+      || (compare_mode == DFmode && result_mode == SFmode))
+    ;
+  else
+    return false;
+
   switch (code)
     {
     case EQ:
@@ -15835,6 +15865,10 @@ have_compare_and_set_mask (machine_mode mode)
     case E_DFmode:
       return TARGET_P9_MINMAX;
 
+    case E_KFmode:
+    case E_TFmode:
+      return TARGET_POWER10 && TARGET_FLOAT128_HW && FLOAT128_IEEE_P (mode);
+
     default:
       break;
     }
diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
index 064c3a2d9d6..ff87d8c6eaa 100644
--- a/gcc/config/rs6000/rs6000.md
+++ b/gcc/config/rs6000/rs6000.md
@@ -5449,6 +5449,112 @@ (define_insn "*xxsel<mode>"
   "xxsel %x0,%x4,%x3,%x1"
   [(set_attr "type" "vecmove")])
 
+;; Support for ISA 3.1 IEEE 128-bit conditional move.  The mode used in the
+;; comparison must be the same as used in the conditional move.
+(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_POWER10 && TARGET_FLOAT128_HW && 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_p10"
+  [(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 "altivec_register_operand" "v,v")
+	 (match_operand:IEEE128 5 "altivec_register_operand" "v,v")))
+   (clobber (match_scratch:V2DI 6 "=0,&v"))]
+  "TARGET_POWER10 && TARGET_FLOAT128_HW && FLOAT128_IEEE_P (<MODE>mode)"
+  "#"
+  "&& 1"
+  [(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_p10"
+  [(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 "altivec_register_operand" "v,v")
+	 (match_operand:IEEE128 5 "altivec_register_operand" "v,v")))
+   (clobber (match_scratch:V2DI 6 "=0,&v"))]
+  "TARGET_POWER10 && TARGET_FLOAT128_HW && FLOAT128_IEEE_P (<MODE>mode)"
+  "#"
+  "&& 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_POWER10 && TARGET_FLOAT128_HW && FLOAT128_IEEE_P (<MODE>mode)"
+  "xscmp%V1qp %0,%2,%3"
+  [(set_attr "type" "fpcompare")])
+
+(define_insn "*xxsel<mode>"
+  [(set (match_operand:IEEE128 0 "altivec_register_operand" "=v")
+	(if_then_else:IEEE128
+	 (ne (match_operand:V2DI 1 "altivec_register_operand" "v")
+	     (match_operand:V2DI 2 "zero_constant" ""))
+	 (match_operand:IEEE128 3 "altivec_register_operand" "v")
+	 (match_operand:IEEE128 4 "altivec_register_operand" "v")))]
+  "TARGET_POWER10 && TARGET_FLOAT128_HW && FLOAT128_IEEE_P (<MODE>mode)"
+  "xxsel %x0,%x4,%x3,%x1"
+  [(set_attr "type" "vecmove")])
+
 \f
 ;; Conversions to and from floating-point.
 
diff --git a/gcc/testsuite/gcc.target/powerpc/float128-cmove.c b/gcc/testsuite/gcc.target/powerpc/float128-cmove.c
new file mode 100644
index 00000000000..2fae8dc23bc
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/float128-cmove.c
@@ -0,0 +1,58 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target ppc_float128_hw } */
+/* { dg-require-effective-target power10_ok } */
+/* { dg-options "-mdejagnu-cpu=power10 -O2" } */
+
+#ifndef TYPE
+#ifdef __LONG_DOUBLE_IEEE128__
+#define TYPE long double
+
+#else
+#define TYPE _Float128
+#endif
+#endif
+
+/* Verify that the ISA 3.1 (power10) IEEE 128-bit conditional move instructions
+   are generated.  */
+
+TYPE
+eq (TYPE a, TYPE b, TYPE c, TYPE d)
+{
+  return (a == b) ? c : d;
+}
+
+TYPE
+ne (TYPE a, TYPE b, TYPE c, TYPE d)
+{
+  return (a != b) ? c : d;
+}
+
+TYPE
+lt (TYPE a, TYPE b, TYPE c, TYPE d)
+{
+  return (a < b) ? c : d;
+}
+
+TYPE
+le (TYPE a, TYPE b, TYPE c, TYPE d)
+{
+  return (a <= b) ? c : d;
+}
+
+TYPE
+gt (TYPE a, TYPE b, TYPE c, TYPE d)
+{
+  return (a > b) ? c : d;
+}
+
+TYPE
+ge (TYPE a, TYPE b, TYPE c, TYPE d)
+{
+  return (a >= b) ? c : d;
+}
+
+/* { dg-final { scan-assembler-times {\mxscmpeqqp\M} 2 } } */
+/* { dg-final { scan-assembler-times {\mxscmpgeqp\M} 2 } } */
+/* { dg-final { scan-assembler-times {\mxscmpgtqp\M} 2 } } */
+/* { dg-final { scan-assembler-times {\mxxsel\M}     6 } } */
+/* { dg-final { scan-assembler-not   {\mxscmpuqp\M}    } } */
diff --git a/gcc/testsuite/gcc.target/powerpc/float128-minmax-3.c b/gcc/testsuite/gcc.target/powerpc/float128-minmax-3.c
new file mode 100644
index 00000000000..6f7627c0f2a
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/float128-minmax-3.c
@@ -0,0 +1,15 @@
+/* { dg-require-effective-target ppc_float128_hw } */
+/* { dg-require-effective-target power10_ok } */
+/* { dg-options "-mdejagnu-cpu=power10 -O2" } */
+
+#ifndef TYPE
+#define TYPE _Float128
+#endif
+
+/* Test that the fminf128/fmaxf128 functions generate if/then/else and not a
+   call.  */
+TYPE f128_min (TYPE a, TYPE b) { return (a < b) ? a : b; }
+TYPE f128_max (TYPE a, TYPE b) { return (b > a) ? b : a; }
+
+/* { dg-final { scan-assembler {\mxsmaxcqp\M} } } */
+/* { dg-final { scan-assembler {\mxsmincqp\M} } } */
-- 
2.31.1


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

Ping: [PATCH 1/3] Add IEEE 128-bit min/max support on PowerPC.

[Michael Meissner]

Ping patch.  In particular, we would like to get this into the GCC 11.2
backport as it is power10 enablement.

| Date: Tue, 8 Jun 2021 20:21:25 -0400
| Subject: [PATCH 1/3] Add IEEE 128-bit min/max support on PowerPC.
| Message-ID: <20210609002125.GA18854@ibm-toto.the-meissners.org>

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

Ping: [PATCH 2/3] Fix IEEE 128-bit min/max test.

[Michael Meissner]

Ping patch.  In particular, we like to get this into GCC 11.2 as part of
power10 enablement.

| Date: Tue, 8 Jun 2021 20:22:40 -0400
| Subject: [PATCH 2/3] Fix IEEE 128-bit min/max test.
| Message-ID: <20210609002240.GB18854@ibm-toto.the-meissners.org>

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

Ping: [PATCH 3/3] Add IEEE 128-bit fp conditional move on PowerPC.

[Michael Meissner]

Ping patch.  In particular, we would like to get this patch into GCC 11.2 for
power10 enablement.

| Date: Tue, 8 Jun 2021 20:24:47 -0400
| Subject: [PATCH 3/3] Add IEEE 128-bit fp conditional move on PowerPC.
| Message-ID: <20210609002447.GC18854@ibm-toto.the-meissners.org>

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

Re: [PATCH 1/3] Add IEEE 128-bit min/max support on PowerPC.

[Segher Boessenkool]

On Tue, Jun 08, 2021 at 08:21:25PM -0400, Michael Meissner wrote:
> GCC will not convert ternary operations into using min/max instructions
> provided in this patch unless the user uses -Ofast or similar switches due to
> issues with NaNs.

It will not do it because it is *incorrect* to do :-)

(The RTL operators smin/smax are undefined for any NaN inputs (unless
both are the same bit pattern), and for different sign zeroes; it isn't
only NaNs even).

> gcc/
> 2021-06-08  Michael Meissner  <meissner@linux.ibm.com>
> 
> 	* config/rs6000/rs6000.c (rs6000_emit_minmax): Add support for ISA
> 	3.1 IEEE 128-bit floating point xsmaxcqp and xsmincqp
> 	instructions.

Please don't randomly break lines.

> --- a/gcc/config/rs6000/rs6000.c
> +++ b/gcc/config/rs6000/rs6000.c
> @@ -16103,7 +16103,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_POWER10 && TARGET_FLOAT128_HW && FLOAT128_IEEE_P (mode))))

The actual insns only check TARGET_POWER10 (so no TARGET_FLOAT128_HW).
Which is right, this or that?

> +/* { dg-require-effective-target ppc_float128_hw } */
> +/* { dg-require-effective-target power10_ok } */
> +/* { dg-options "-mdejagnu-cpu=power10 -O2 -ffast-math" } */

In testcases we can assume that float128_hw is set whenever we have a
p10; we don't manually disable it to make live hard for ourselves ;-)

We could disallow disabling QP float separately from all other float.
We will still need to test if float is enabled at all so it won't help
all that much immediately, alas.

With that TARGET_POWER10 condition fixed: okay for trunk, and for 11
once it is tested for trunk on all systems.  Thanks!


Segher

Re: [PATCH 2/3] Fix IEEE 128-bit min/max test.

[Segher Boessenkool]

On Tue, Jun 08, 2021 at 08:22:40PM -0400, Michael Meissner wrote:
> 
> 	* gcc.target/powerpc/float128-minmax.c: Adjust expected code for
> 	power10.
> 	* lib/target-supports.exp (check_effective_target_has_arch_pwr10):
> 	New target support.
> ---
>  gcc/testsuite/gcc.target/powerpc/float128-minmax.c |  8 +++++---
>  gcc/testsuite/lib/target-supports.exp              | 10 ++++++++++
>  2 files changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/gcc/testsuite/gcc.target/powerpc/float128-minmax.c b/gcc/testsuite/gcc.target/powerpc/float128-minmax.c
> index fe397518f2f..a7d3a3a0b3e 100644
> --- a/gcc/testsuite/gcc.target/powerpc/float128-minmax.c
> +++ b/gcc/testsuite/gcc.target/powerpc/float128-minmax.c
> @@ -1,6 +1,5 @@
> -/* { dg-do compile { target lp64 } } */

Does that work?  Why was it there before?

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

Why is it okay to no longer run this test where it ran before?

> -/* { dg-final { scan-assembler     {\mxscmpuqp\M} } } */
> +/* Adjust code power10 which has native min/max instructions.  */
> +/* { dg-final { scan-assembler     {\mxscmpuqp\M} { target { ! has_arch_pwr10 } } } } */

You need scan-assembler-times here?  (Not that it had that before this
patch, of course).

> +/* { dg-final { scan-assembler     {\mxsmincqp\M} { target {   has_arch_pwr10 } } } } */
> +/* { dg-final { scan-assembler     {\mxsmaxcqp\M} { target {   has_arch_pwr10 } } } } */

You can write just  { target has_arch_pwr10 }  here, I think?  Please do
so (if that works, I haven't actually tested it :-) )


Segher

Re: [PATCH 1/3] Add IEEE 128-bit min/max support on PowerPC.

[Michael Meissner]

On Thu, Jun 17, 2021 at 12:39:04PM -0500, Segher Boessenkool wrote:
> On Tue, Jun 08, 2021 at 08:21:25PM -0400, Michael Meissner wrote:
> > GCC will not convert ternary operations into using min/max instructions
> > provided in this patch unless the user uses -Ofast or similar switches due to
> > issues with NaNs.
> 
> It will not do it because it is *incorrect* to do :-)
> 
> (The RTL operators smin/smax are undefined for any NaN inputs (unless
> both are the same bit pattern), and for different sign zeroes; it isn't
> only NaNs even).
> 
> > gcc/
> > 2021-06-08  Michael Meissner  <meissner@linux.ibm.com>
> > 
> > 	* config/rs6000/rs6000.c (rs6000_emit_minmax): Add support for ISA
> > 	3.1 IEEE 128-bit floating point xsmaxcqp and xsmincqp
> > 	instructions.
> 
> Please don't randomly break lines.
> 
> > --- a/gcc/config/rs6000/rs6000.c
> > +++ b/gcc/config/rs6000/rs6000.c
> > @@ -16103,7 +16103,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_POWER10 && TARGET_FLOAT128_HW && FLOAT128_IEEE_P (mode))))
> 
> The actual insns only check TARGET_POWER10 (so no TARGET_FLOAT128_HW).
> Which is right, this or that?

It should include TARGET_FLOAT128_HW.  The problem area is a power10 running in
big endian mode and running 32-bit code.  Because we don't have TImode, we
can't enable the IEEE 128-bit hardware instructions.  And because we don't have
GLIBC support, we don't enable the FLOAT128 stuff by default on big endian.

> > +/* { dg-require-effective-target ppc_float128_hw } */
> > +/* { dg-require-effective-target power10_ok } */
> > +/* { dg-options "-mdejagnu-cpu=power10 -O2 -ffast-math" } */
> 
> In testcases we can assume that float128_hw is set whenever we have a
> p10; we don't manually disable it to make live hard for ourselves ;-)

Again, I put it in case somebody builds a BE power10 compiler.

> We could disallow disabling QP float separately from all other float.
> We will still need to test if float is enabled at all so it won't help
> all that much immediately, alas.
> 
> With that TARGET_POWER10 condition fixed: okay for trunk, and for 11
> once it is tested for trunk on all systems.  Thanks!

Thanks.

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

Re: [PATCH 2/3] Fix IEEE 128-bit min/max test.

[Michael Meissner]

On Thu, Jun 17, 2021 at 01:11:58PM -0500, Segher Boessenkool wrote:
> On Tue, Jun 08, 2021 at 08:22:40PM -0400, Michael Meissner wrote:
> > 
> > 	* gcc.target/powerpc/float128-minmax.c: Adjust expected code for
> > 	power10.
> > 	* lib/target-supports.exp (check_effective_target_has_arch_pwr10):
> > 	New target support.
> > ---
> >  gcc/testsuite/gcc.target/powerpc/float128-minmax.c |  8 +++++---
> >  gcc/testsuite/lib/target-supports.exp              | 10 ++++++++++
> >  2 files changed, 15 insertions(+), 3 deletions(-)
> > 
> > diff --git a/gcc/testsuite/gcc.target/powerpc/float128-minmax.c b/gcc/testsuite/gcc.target/powerpc/float128-minmax.c
> > index fe397518f2f..a7d3a3a0b3e 100644
> > --- a/gcc/testsuite/gcc.target/powerpc/float128-minmax.c
> > +++ b/gcc/testsuite/gcc.target/powerpc/float128-minmax.c
> > @@ -1,6 +1,5 @@
> > -/* { dg-do compile { target lp64 } } */
> 
> Does that work?  Why was it there before?
> 
> >  /* { dg-require-effective-target powerpc_p9vector_ok } */
> > -/* { dg-require-effective-target float128 } */
> > +/* { dg-require-effective-target ppc_float128_hw } */
> 
> Why is it okay to no longer run this test where it ran before?

I was just being more precise.

> > -/* { dg-final { scan-assembler     {\mxscmpuqp\M} } } */
> > +/* Adjust code power10 which has native min/max instructions.  */
> > +/* { dg-final { scan-assembler     {\mxscmpuqp\M} { target { ! has_arch_pwr10 } } } } */
> 
> You need scan-assembler-times here?  (Not that it had that before this
> patch, of course).

I can change it to scan-assembler-times.

> > +/* { dg-final { scan-assembler     {\mxsmincqp\M} { target {   has_arch_pwr10 } } } } */
> > +/* { dg-final { scan-assembler     {\mxsmaxcqp\M} { target {   has_arch_pwr10 } } } } */
> 
> You can write just  { target has_arch_pwr10 }  here, I think?  Please do
> so (if that works, I haven't actually tested it :-) )

I thought so originally, but it didn't like it, so I added the extra {}'s.  But
I can try it again.

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

Re: [PATCH 2/3] Fix IEEE 128-bit min/max test.

[Michael Meissner]

On Thu, Jun 17, 2021 at 01:11:58PM -0500, Segher Boessenkool wrote:
> On Tue, Jun 08, 2021 at 08:22:40PM -0400, Michael Meissner wrote:
> > 
> > 	* gcc.target/powerpc/float128-minmax.c: Adjust expected code for
> > 	power10.
> > 	* lib/target-supports.exp (check_effective_target_has_arch_pwr10):
> > 	New target support.
> > ---
> >  gcc/testsuite/gcc.target/powerpc/float128-minmax.c |  8 +++++---
> >  gcc/testsuite/lib/target-supports.exp              | 10 ++++++++++
> >  2 files changed, 15 insertions(+), 3 deletions(-)
> > 
> > diff --git a/gcc/testsuite/gcc.target/powerpc/float128-minmax.c b/gcc/testsuite/gcc.target/powerpc/float128-minmax.c
> > index fe397518f2f..a7d3a3a0b3e 100644
> > --- a/gcc/testsuite/gcc.target/powerpc/float128-minmax.c
> > +++ b/gcc/testsuite/gcc.target/powerpc/float128-minmax.c
> > @@ -1,6 +1,5 @@
> > -/* { dg-do compile { target lp64 } } */
> 
> Does that work?  Why was it there before?

The lp64 eliminates 32-bit, which does not support hardware IEEE 128-bit due to
the lack of TImode.  The test was written before the ppc_float128_hw test.  Now
that we have ppc_float128_hw, we don't need an explicit lp64.
 
> >  /* { dg-require-effective-target powerpc_p9vector_ok } */
> > -/* { dg-require-effective-target float128 } */
> > +/* { dg-require-effective-target ppc_float128_hw } */
> 
> Why is it okay to no longer run this test where it ran before?

The ppc_float128_hw test is a more precise test than just float128 and power9.
For 64-bit, it doesn't matter, but it allows us to drop the lp64 test.
 
> > -/* { dg-final { scan-assembler     {\mxscmpuqp\M} } } */
> > +/* Adjust code power10 which has native min/max instructions.  */
> > +/* { dg-final { scan-assembler     {\mxscmpuqp\M} { target { ! has_arch_pwr10 } } } } */
> 
> You need scan-assembler-times here?  (Not that it had that before this
> patch, of course).
> 
> > +/* { dg-final { scan-assembler     {\mxsmincqp\M} { target {   has_arch_pwr10 } } } } */
> > +/* { dg-final { scan-assembler     {\mxsmaxcqp\M} { target {   has_arch_pwr10 } } } } */
> 
> You can write just  { target has_arch_pwr10 }  here, I think?  Please do
> so (if that works, I haven't actually tested it :-) )
> 
> 
> Segher

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

[PATCH 2/3 V2] Fix IEEE 128-bit min/max test.

[Michael Meissner]

Here is a replacement patch.  Can I check this into the master branch, and
eventually backport it to GCC 11?

[PATCH] Fix IEEE 128-bit min/max test.

This patch fixes the float128-minmax.c test so that it can accommodate the
generation of xsmincqp and xsmaxcqp instructions on power10.  I changed
the effective target from 'float128' to 'ppc_float128_hw', since this
needs the IEEE 128-bit float hardware support.  Changing to use
'ppc_float128_hw' allows the 'lp64' test to be dropped.  The 'lp64' test
was needed because big endian 32-bit code cannot enable the IEEE 128-bit
floating point instructions.

gcc/testsuite/
2021-06-17  Michael Meissner  <meissner@linux.ibm.com>

	* gcc.target/powerpc/float128-minmax.c: Adjust expected code for
	power10.
	* lib/target-supports.exp (check_effective_target_has_arch_pwr10):
	New target support.
---
 gcc/testsuite/gcc.target/powerpc/float128-minmax.c | 10 ++++++----
 gcc/testsuite/lib/target-supports.exp              | 10 ++++++++++
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/gcc/testsuite/gcc.target/powerpc/float128-minmax.c b/gcc/testsuite/gcc.target/powerpc/float128-minmax.c
index fe397518f2f..b0e6bd39873 100644
--- a/gcc/testsuite/gcc.target/powerpc/float128-minmax.c
+++ b/gcc/testsuite/gcc.target/powerpc/float128-minmax.c
@@ -1,6 +1,5 @@
-/* { dg-do compile { target lp64 } } */
 /* { dg-require-effective-target powerpc_p9vector_ok } */
-/* { dg-require-effective-target float128 } */
+/* { dg-require-effective-target ppc_float128_hw } */
 /* { dg-options "-mpower9-vector -O2 -ffast-math" } */
 
 #ifndef TYPE
@@ -12,5 +11,8 @@
 TYPE f128_min (TYPE a, TYPE b) { return __builtin_fminf128 (a, b); }
 TYPE f128_max (TYPE a, TYPE b) { return __builtin_fmaxf128 (a, b); }
 
-/* { dg-final { scan-assembler     {\mxscmpuqp\M} } } */
-/* { dg-final { scan-assembler-not {\mbl\M}       } } */
+/* Adjust code power10 which has native min/max instructions.  */
+/* { dg-final { scan-assembler-times {\mxscmpuqp\M} 2 { target { ! has_arch_pwr10 } } } } */
+/* { dg-final { scan-assembler-times {\mxsmincqp\M} 1 { target has_arch_pwr10 } } } */
+/* { dg-final { scan-assembler-times {\mxsmaxcqp\M} 1 { target has_arch_pwr10 } } } */
+/* { dg-final { scan-assembler-not {\mbl\M} } } */
diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
index 7f78c5593ac..789723fb287 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -6127,6 +6127,16 @@ proc check_effective_target_has_arch_pwr9 { } {
 	}]
 }
 
+proc check_effective_target_has_arch_pwr10 { } {
+	return [check_no_compiler_messages arch_pwr10 assembly {
+		#ifndef _ARCH_PWR10
+		#error does not have power10 support.
+		#else
+		/* "has power10 support" */
+		#endif
+	}]
+}
+
 # Return 1 if this is a PowerPC target supporting -mcpu=power10.
 # Limit this to 64-bit linux systems for now until other targets support
 # power10.
-- 
2.31.1


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

Ping: [PATCH 2/3 V2] Fix IEEE 128-bit min/max test.

[Michael Meissner]

Ping this patch:

| Date: Thu, 17 Jun 2021 18:56:09 -0400
| Subject: [PATCH 2/3 V2] Fix IEEE 128-bit min/max test.
| Message-ID: <20210617225609.GA4816@ibm-toto.the-meissners.org>

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

Ping: [PATCH 3/3] Add IEEE 128-bit fp conditional move on PowerPC.

[Michael Meissner]

Ping this patch.

| Date: Tue, 8 Jun 2021 20:24:47 -0400
| Subject: [PATCH 3/3] Add IEEE 128-bit fp conditional move on PowerPC.
| Message-ID: <20210609002447.GC18854@ibm-toto.the-meissners.org>

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

Re: Ping: [PATCH 2/3 V2] Fix IEEE 128-bit min/max test.

[Michael Meissner]

Note, this patch should go into GCC 11 after baking on the master branch.

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

Re: [PATCH 1/3] Add IEEE 128-bit min/max support on PowerPC.

[Segher Boessenkool]

Hi!

On Thu, Jun 17, 2021 at 03:18:48PM -0400, Michael Meissner wrote:
> > The actual insns only check TARGET_POWER10 (so no TARGET_FLOAT128_HW).
> > Which is right, this or that?
> 
> It should include TARGET_FLOAT128_HW.

Okay, so fix that :-)

> The problem area is a power10 running in
> big endian mode and running 32-bit code.  Because we don't have TImode, we
> can't enable the IEEE 128-bit hardware instructions.

I don't see why not?

> > > +/* { dg-require-effective-target ppc_float128_hw } */
> > > +/* { dg-require-effective-target power10_ok } */
> > > +/* { dg-options "-mdejagnu-cpu=power10 -O2 -ffast-math" } */
> > 
> > In testcases we can assume that float128_hw is set whenever we have a
> > p10; we don't manually disable it to make live hard for ourselves ;-)
> 
> Again, I put it in case somebody builds a BE power10 compiler.

This should still be fixed.  And yes, people do test BE p10, of course.
And BE p10 *should* enable the QP float insns.  Does it not currently?


Segher

Re: [PATCH 2/3] Fix IEEE 128-bit min/max test.

[Segher Boessenkool]

On Thu, Jun 17, 2021 at 04:11:40PM -0400, Michael Meissner wrote:
> On Thu, Jun 17, 2021 at 01:11:58PM -0500, Segher Boessenkool wrote:
> > > --- a/gcc/testsuite/gcc.target/powerpc/float128-minmax.c
> > > +++ b/gcc/testsuite/gcc.target/powerpc/float128-minmax.c
> > > @@ -1,6 +1,5 @@
> > > -/* { dg-do compile { target lp64 } } */
> > 
> > Does that work?  Why was it there before?
> 
> The lp64 eliminates 32-bit, which does not support hardware IEEE 128-bit due to
> the lack of TImode.

I still do not understand this.  Why would support for QP float require
TImode?  "Need an integer mode of the same size" is not a convincing
argument, since double-double is a 16 byte mode as well.

> The test was written before the ppc_float128_hw test.  Now
> that we have ppc_float128_hw, we don't need an explicit lp64.

Ah good, some progress.  Well, it *is* an improvement, a better
abstraction, but on the other hand it only hides the actual problems
deeper :-/

> > >  /* { dg-require-effective-target powerpc_p9vector_ok } */
> > > -/* { dg-require-effective-target float128 } */
> > > +/* { dg-require-effective-target ppc_float128_hw } */
> > 
> > Why is it okay to no longer run this test where it ran before?
> 
> The ppc_float128_hw test is a more precise test than just float128 and power9.

You did not delete the p9 test though.


Segher

Re: [PATCH 2/3] Fix IEEE 128-bit min/max test.

[Michael Meissner]

On Fri, Jun 25, 2021 at 12:46:37PM -0500, Segher Boessenkool wrote:
> On Thu, Jun 17, 2021 at 04:11:40PM -0400, Michael Meissner wrote:
> > On Thu, Jun 17, 2021 at 01:11:58PM -0500, Segher Boessenkool wrote:
> > > > --- a/gcc/testsuite/gcc.target/powerpc/float128-minmax.c
> > > > +++ b/gcc/testsuite/gcc.target/powerpc/float128-minmax.c
> > > > @@ -1,6 +1,5 @@
> > > > -/* { dg-do compile { target lp64 } } */
> > > 
> > > Does that work?  Why was it there before?
> > 
> > The lp64 eliminates 32-bit, which does not support hardware IEEE 128-bit due to
> > the lack of TImode.
> 
> I still do not understand this.  Why would support for QP float require
> TImode?  "Need an integer mode of the same size" is not a convincing
> argument, since double-double is a 16 byte mode as well.

I suspect it is because we separate moves for IBM long double before the pass
that wants to use an integer type to do the move, so it doesn't see the 128-bit
type.

> > The test was written before the ppc_float128_hw test.  Now
> > that we have ppc_float128_hw, we don't need an explicit lp64.
> 
> Ah good, some progress.  Well, it *is* an improvement, a better
> abstraction, but on the other hand it only hides the actual problems
> deeper :-/
> 
> > > >  /* { dg-require-effective-target powerpc_p9vector_ok } */
> > > > -/* { dg-require-effective-target float128 } */
> > > > +/* { dg-require-effective-target ppc_float128_hw } */
> > > 
> > > Why is it okay to no longer run this test where it ran before?
> > 
> > The ppc_float128_hw test is a more precise test than just float128 and power9.
> 
> You did not delete the p9 test though.

Yes, I can probably delete the powerpc_p9vector_ok test.

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

Re: [PATCH 1/3] Add IEEE 128-bit min/max support on PowerPC.

[Michael Meissner]

On Wed, Jun 23, 2021 at 06:56:37PM -0500, Segher Boessenkool wrote:
> Hi!
> 
> On Thu, Jun 17, 2021 at 03:18:48PM -0400, Michael Meissner wrote:
> > > The actual insns only check TARGET_POWER10 (so no TARGET_FLOAT128_HW).
> > > Which is right, this or that?
> > 
> > It should include TARGET_FLOAT128_HW.
> 
> Okay, so fix that :-)


> > The problem area is a power10 running in
> > big endian mode and running 32-bit code.  Because we don't have TImode, we
> > can't enable the IEEE 128-bit hardware instructions.
> 
> I don't see why not?
> 
> > > > +/* { dg-require-effective-target ppc_float128_hw } */
> > > > +/* { dg-require-effective-target power10_ok } */
> > > > +/* { dg-options "-mdejagnu-cpu=power10 -O2 -ffast-math" } */
> > > 
> > > In testcases we can assume that float128_hw is set whenever we have a
> > > p10; we don't manually disable it to make live hard for ourselves ;-)
> > 
> > Again, I put it in case somebody builds a BE power10 compiler.
> 
> This should still be fixed.  And yes, people do test BE p10, of course.
> And BE p10 *should* enable the QP float insns.  Does it not currently?

GCC does not enable __float128 by default on BE.  The reason is there are no
plans to enable all of the float128 support in glibc in BE.  Without a library,
it is kind of useless to enable __float128.

If the compiler enabled __float128, It breaks things that check if __float128
is avaiable.  They think __float128 is available, and then they fail when when
they can't anything besides basic arithmetic.

Because the compiler is configured not to enable __float128 in a BE context, we
don't build the __float128 emulator in libgcc.

In addition, BE GCC runs on things that does not have GLIBC (like AIX).  If we
enabled it by default, it would break those environments.

A further complication is BE by default is still power4 or power5.  You need
VSX support to even pass __float128 arguments.  While it is possible to pass
__float128 in GPRs, you run into compatibility issues if one module is compiled
with VSX and another is compiled without setting a base cpu, because one module
will expect things in GPRs and the other in Altivec registers.

And as I've said, the issue with 32-bit move is we don't have TImode support.
Some of the machine indepenent passes want to use an appropriate integer type
to move basic types.

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

Re: [PATCH 2/3 V2] Fix IEEE 128-bit min/max test.

[Segher Boessenkool]

On Thu, Jun 17, 2021 at 06:56:09PM -0400, Michael Meissner wrote:
> The 'lp64' test
> was needed because big endian 32-bit code cannot enable the IEEE 128-bit
> floating point instructions.

No, *does not* enable them.  After

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 2c249e186e1e..d4aac4164cfe 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -4281,7 +4281,7 @@ rs6000_option_override_internal (bool global_init_p)
       rs6000_isa_flags &= ~OPTION_MASK_FLOAT128_HW;
     }
 
-  if (TARGET_FLOAT128_HW && !TARGET_64BIT)
+  if (0&& TARGET_FLOAT128_HW && !TARGET_64BIT)
     {
       if ((rs6000_isa_flags_explicit & OPTION_MASK_FLOAT128_HW) != 0)
        error ("%qs requires %qs", "%<-mfloat128-hardware%>", "-m64");

you can compile fine with -m32 if you add -mfloat128-hardware as well
(it is disabled for BE as well, that should be fixed as well a few lines
up from there).

Can you show any code that will not work please?  Not allowing QP float
with -m32 causes many more problems than just allowing it would.

> 	* gcc.target/powerpc/float128-minmax.c: Adjust expected code for
> 	power10.
> 	* lib/target-supports.exp (check_effective_target_has_arch_pwr10):
> 	New target support.

Just "New." please.

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

Please try whether you can lose that line as well.

Okay for trunk, and for 11 after the usual soak.  Thanks!


Segher

Re: [PATCH 2/3 V2] Fix IEEE 128-bit min/max test.

[Michael Meissner]

On Tue, Jun 29, 2021 at 07:06:14PM -0500, Segher Boessenkool wrote:
> On Thu, Jun 17, 2021 at 06:56:09PM -0400, Michael Meissner wrote:
> > The 'lp64' test
> > was needed because big endian 32-bit code cannot enable the IEEE 128-bit
> > floating point instructions.
> 
> No, *does not* enable them.  After
> 
> diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
> index 2c249e186e1e..d4aac4164cfe 100644
> --- a/gcc/config/rs6000/rs6000.c
> +++ b/gcc/config/rs6000/rs6000.c
> @@ -4281,7 +4281,7 @@ rs6000_option_override_internal (bool global_init_p)
>        rs6000_isa_flags &= ~OPTION_MASK_FLOAT128_HW;
>      }
>  
> -  if (TARGET_FLOAT128_HW && !TARGET_64BIT)
> +  if (0&& TARGET_FLOAT128_HW && !TARGET_64BIT)
>      {
>        if ((rs6000_isa_flags_explicit & OPTION_MASK_FLOAT128_HW) != 0)
>         error ("%qs requires %qs", "%<-mfloat128-hardware%>", "-m64");
> 
> you can compile fine with -m32 if you add -mfloat128-hardware as well
> (it is disabled for BE as well, that should be fixed as well a few lines
> up from there).
> 
> Can you show any code that will not work please?  Not allowing QP float
> with -m32 causes many more problems than just allowing it would.

I will in a bit, but it isn't that simple.  Right now, all of the optimizations
for converting char/short/int -> __float128 (and presumably the other way)
don't work on 32-bit, because they want to split:

	(parallel [(set (reg:KF <n>)
			(float:KF (reg:SI <m>)))
		   (clobber (reg:DI <tmp>))])

Into:

	(set (reg:DI <tmp>)
	     (sign_extend:DI (reg:SI <m>)))

	(set (reg:KF <n>)
	     (float:KF (reg:DI <tmp>)))

And the sign_extend SI->DI doesn't exist in 32-bit.
	

> > 	* gcc.target/powerpc/float128-minmax.c: Adjust expected code for
> > 	power10.
> > 	* lib/target-supports.exp (check_effective_target_has_arch_pwr10):
> > 	New target support.
> 
> Just "New." please.
> 
> >  /* { dg-require-effective-target powerpc_p9vector_ok } */
> 
> Please try whether you can lose that line as well.
> 
> Okay for trunk, and for 11 after the usual soak.  Thanks!
> 
> 
> Segher

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

Re: [PATCH 1/3] Add IEEE 128-bit min/max support on PowerPC.

[Segher Boessenkool]

On Mon, Jun 28, 2021 at 03:00:02PM -0400, Michael Meissner wrote:
> On Wed, Jun 23, 2021 at 06:56:37PM -0500, Segher Boessenkool wrote:
> > > The problem area is a power10 running in
> > > big endian mode and running 32-bit code.  Because we don't have TImode, we
> > > can't enable the IEEE 128-bit hardware instructions.
> > 
> > I don't see why not?

And this is still true, and the core of the problem here.  Please show
any argument for this?

> > > > > +/* { dg-require-effective-target ppc_float128_hw } */
> > > > > +/* { dg-require-effective-target power10_ok } */
> > > > > +/* { dg-options "-mdejagnu-cpu=power10 -O2 -ffast-math" } */
> > > > 
> > > > In testcases we can assume that float128_hw is set whenever we have a
> > > > p10; we don't manually disable it to make live hard for ourselves ;-)
> > > 
> > > Again, I put it in case somebody builds a BE power10 compiler.
> > 
> > This should still be fixed.  And yes, people do test BE p10, of course.
> > And BE p10 *should* enable the QP float insns.  Does it not currently?
> 
> GCC does not enable __float128 by default on BE.

But it does enable _Float128, as it should, and it does work.

> The reason is there are no
> plans to enable all of the float128 support in glibc in BE.  Without a library,
> it is kind of useless to enable __float128.

This is fundamentally wrong.  GCC is a compiler.  It is used without
libraries often (some applications do not want the standard libraries
for a reason, some implement them themselves, some *are* the standard
libraries).  And you can have a lot of useful code without using libm.

> If the compiler enabled __float128, It breaks things that check if __float128
> is avaiable.  They think __float128 is available, and then they fail when when
> they can't anything besides basic arithmetic.

So?  That would be the *correct* behaviour.

> Because the compiler is configured not to enable __float128 in a BE context, we
> don't build the __float128 emulator in libgcc.

Yes, another imperfection.

> In addition, BE GCC runs on things that does not have GLIBC (like AIX).  If we
> enabled it by default, it would break those environments.

How so?  Not anymore than you do now, you cannot use *any* QP float
with the status quo.

> A further complication is BE by default is still power4 or power5.  You need
> VSX support to even pass __float128 arguments.  While it is possible to pass
> __float128 in GPRs, you run into compatibility issues if one module is compiled
> with VSX and another is compiled without setting a base cpu, because one module
> will expect things in GPRs and the other in Altivec registers.

Yes, allowing QP float before p8 would solve a lot more problems, as I
have told you very often; but it is independent of this.  You need p9
to have the machine insns, and you can compile code that needs the
libgcc soft-float emulation functions for QP on power7 already (it needs
basic VSX only, and should be okay with only VMX even).

> And as I've said, the issue with 32-bit move is we don't have TImode support.
> Some of the machine indepenent passes want to use an appropriate integer type
> to move basic types.

So why does it work fine with double-double?

Please show an example.


Segher

Re: [PATCH 3/3] Add IEEE 128-bit fp conditional move on PowerPC.

[Segher Boessenkool]

Hi!

On Tue, Jun 08, 2021 at 08:24:47PM -0400, Michael Meissner wrote:
> In this patch, I simplified things compared to previous patches.  Instead of
> allowing any four of the modes to be used for the conditional move comparison
> and the move itself could use different modes, I restricted the conditional
> move to just the same mode.  I.e. you can do:
> 
>         _Float128 a, b, c, d, e, r;
> 
>         r = (a == b) ? c : d;
> 
> But you can't do:
> 
>         _Float128 c, d, r;
>         double a, b;
> 
>         r = (a == b) ? c : d;
> 
> or:
> 
>         _Float128 a, b;
>         double c, d, r;
> 
>         r = (a == b) ? c : d;

I suspect you mean you *can* do it, but it generates sub-optimal code
for it, promoting everything to QP float?

> This eliminates a lot of the complexity of the code, because you don't have to
> worry about the sizes being different, and the IEEE 128-bit types being
> restricted to Altivec registers, while the SF/DF modes can use any VSX
> register.

It doesn't remove any complexity.  Instead, you postpone having to work
on it to a later date.  Pootentially making it harder for the person who
will eventually have to do it to do a good job :-(

(Or this code is not worth having in the first place, in which case, why
do we want to have it?)

Of course this is the more common case, but it is not so overwhelmingly
more common that we can ignore the rest.

> Compared to the May 18th patches, this patch replaces the complicated test that
> was complained about.

That was explained to you to not be a good idea.  Yes.

> @@ -15783,6 +15784,35 @@ rs6000_maybe_emit_fp_cmove (rtx dest, rtx op, rtx true_cond, rtx false_cond)
>    if (!can_create_pseudo_p ())
>      return 0;
>  
> +  /* We allow the comparison to be either SFmode/DFmode and the true/false
> +     condition to be either SFmode/DFmode.  I.e. we allow:
> +
> +	float a, b;
> +	double c, d, r;
> +
> +	r = (a == b) ? c : d;
> +
> +    and:
> +
> +	double a, b;
> +	float c, d, r;
> +
> +	r = (a == b) ? c : d;
> +
> +    but we don't allow intermixing the IEEE 128-bit floating point types with
> +    the 32/64-bit scalar types.
> +
> +    It gets too messy where SFmode/DFmode can use any register and TFmode/KFmode
> +    can only use Altivec registers.  In addtion, we would need to do a XXPERMDI

(spelling: "addition", "an").

Lose the "gets too messy" comment, people might believe it!

> +  if (compare_mode == result_mode
> +      || (compare_mode == SFmode && result_mode == DFmode)
> +      || (compare_mode == DFmode && result_mode == SFmode))
> +    ;
> +  else
> +    return false;

Use a logical inversion, instead, as said before:

  if (!(compare_mode == result_mode
	|| (compare_mode == SFmode && result_mode == DFmode)
	|| (compare_mode == DFmode && result_mode == SFmode)))
    return false;

> +;; Support for ISA 3.1 IEEE 128-bit conditional move.  The mode used in the
> +;; comparison must be the same as used in the conditional move.

s/conditional //

Okay for trunk with those fixes.  Also okay for 11 after the usual wait.
Thanks!


Segher