On Fri, 2021-04-09 at 10:42 -0400, Michael Meissner wrote:
> Add IEEE 128-bit min/max support on PowerPC.
>
> This patch has been posted various times in the past. My memory is the last
> time I changed the patch, I addressed the concerns posted at that time. Since
> then the patch seems to have gone into a limbo state.
Hi,
I'll throw some comments at this below, and see if it will trigger more
follow-up.
>
> 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.
>
> Rather than trying to overload the current SF/DF min/max support, it was
> simpler to just provide the new instructions as a separate insn.
>
> I have tested this patch in various little endian and big endian PowerPC
> builds
> since I've posted. It has no regressions, and it adds the instructions if
> -mcpu=power10 is used.
>
> gcc/
> 2021-04-09 Michael Meissner <meiss...@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.
I don't see any direct reference to xsmaxcqp or xsmincqp with respect
to this change below.
It looks like this change adds the FLOAT128_MIN_MAX_FPMASK_P (mode)
check
as criteria for emitting some form of a SET instruction.
emit_insn (gen_rtx_SET (dest, gen_rtx_fmt_ee (code, mode, op0, op1)));
Ok, I see it now, the instructions are mildly obfuscated by
"xs<minmax>cqp" as part of the rs6000.md change below.
> * config/rs6000/rs60000.h (FLOAT128_MIN_MAX_FPMASK_P): New macro.
which is
#define FLOAT128_MIN_MAX_FPMASK_P(MODE) \
(TARGET_POWER10 && TARGET_FLOAT128_HW && FLOAT128_IEEE_P (MODE))
Are there any non MIN_MAX scenarios that will require the combination
of POWER10,FLOAT128_HW,FLOAT128_IEEE(mode)? I'd wonder if there is a name
not specific to *_MIN_MAX_* that would be a better naming choice.
But, naming is hard. :-)
> * config/rs6000/rs6000.md (s<minmax><mode>3): Add support for the
> ISA 3.1 IEEE 128-bit minimum and maximum instructions.
I'd move the "xsmaxcqp,xsmincqp" instruction references from the rs6000.c
changelog blurb to this changelog blurb.
I've looked over the rest, no further relevant comments below.
thanks
-Will
>
> gcc/testsuite/
> 2021-04-09 Michael Meissner <meiss...@linux.ibm.com>
>
> * gcc.target/powerpc/float128-minmax-2.c: New test.
> ---
> gcc/config/rs6000/rs6000.c | 3 ++-
> gcc/config/rs6000/rs6000.h | 5 +++++
> gcc/config/rs6000/rs6000.md | 11 +++++++++++
> .../gcc.target/powerpc/float128-minmax-2.c | 15 +++++++++++++++
> 4 files changed, 33 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 35f5c332c41..e87686c1c4d 100644
> --- a/gcc/config/rs6000/rs6000.c
> +++ b/gcc/config/rs6000/rs6000.c
> @@ -16111,7 +16111,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))
> + || FLOAT128_MIN_MAX_FPMASK_P (mode)))
> {
> emit_insn (gen_rtx_SET (dest, gen_rtx_fmt_ee (code, mode, op0, op1)));
> return;
ok
> diff --git a/gcc/config/rs6000/rs6000.h b/gcc/config/rs6000/rs6000.h
> index 233a92baf3c..e3fb0798622 100644
> --- a/gcc/config/rs6000/rs6000.h
> +++ b/gcc/config/rs6000/rs6000.h
> @@ -345,6 +345,11 @@ extern const char *host_detect_local_cpu (int argc,
> const char **argv);
> || ((MODE) == TDmode) \
> || (!TARGET_FLOAT128_TYPE && FLOAT128_IEEE_P (MODE)))
>
> +/* Macro whether the float128 minimum, maximum, and set compare mask
> + instructions are enabled. */
> +#define FLOAT128_MIN_MAX_FPMASK_P(MODE)
> \
> + (TARGET_POWER10 && TARGET_FLOAT128_HW && FLOAT128_IEEE_P (MODE))
> +
> /* Return true for floating point that does not use a vector register. */
> #define SCALAR_FLOAT_MODE_NOT_VECTOR_P(MODE) \
> (SCALAR_FLOAT_MODE_P (MODE) && !FLOAT128_VECTOR_P (MODE))
ok
> diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
> index c8cdc42533c..17b2fdc1cdd 100644
> --- a/gcc/config/rs6000/rs6000.md
> +++ b/gcc/config/rs6000/rs6000.md
> @@ -5194,6 +5194,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.
ok
> 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} } } */
ok
> --
> 2.22.0
>
>
