On Tue, 2021-05-18 at 16:26 -0400, Michael Meissner wrote:
> [PATCH 1/2] Add IEEE 128-bit min/max support on PowerPC.
>
Hi,
> 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 ?: operations into using min/max instructions provided in
I'd throw the ternary term in there, easier to search for later.
s/?: operations/ternary (?:) operations /
> 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
> ?: conversion in many cases.
>
> I have done bootstrap builds with this patch on the following 3 systems:
> 1) power9 running LE Linux using --with-cpu=power9
> 2) power8 running BE Linux using --with-cpu=power8, testing both
> 32/64-bit.
> 3) power10 prototype running LE Linux using --with-cpu=power10.
>
> There were no regressions to the tests, and the new test added passed. Can I
> check these patches into trunk branch for GCC 12?
>
> I would like to check these patches into GCC 11 after a cooling off period,
> but
> I can also not do the backport if desired.
>
> gcc/
> 2021-05-18 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.
> * config/rs6000/rs6000.md (s<minmax><mode>3, IEEE128 iterator):
> New insns.
ok
>
> gcc/testsuite/
> 2021-05-18 Michael Meissner <meiss...@linux.ibm.com>
>
> * gcc.target/powerpc/float128-minmax-2.c: New test.
> * gcc.target/powerpc/float128-minmax.c: Turn off power10 code
> generation.
So, presumably the float128-minmax-2.c test adds/replaces the power10
code gen tests that were removed or disabled from float128-minmax.c.
> ---
> gcc/config/rs6000/rs6000.c | 3 ++-
> gcc/config/rs6000/rs6000.md | 11 +++++++++++
> .../gcc.target/powerpc/float128-minmax-2.c | 15 +++++++++++++++
> .../gcc.target/powerpc/float128-minmax.c | 7 +++++++
> 4 files changed, 35 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 0d0595dddd6..fdaf12aeda0 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))
> + || (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 0bfeb24d9e8..3a1bc1f8547 100644
> --- a/gcc/config/rs6000/rs6000.md
> +++ b/gcc/config/rs6000/rs6000.md
> @@ -5196,6 +5196,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
> diff --git a/gcc/testsuite/gcc.target/powerpc/float128-minmax.c
> b/gcc/testsuite/gcc.target/powerpc/float128-minmax.c
> index fe397518f2f..c3af759c0b9 100644
> --- a/gcc/testsuite/gcc.target/powerpc/float128-minmax.c
> +++ b/gcc/testsuite/gcc.target/powerpc/float128-minmax.c
> @@ -3,6 +3,13 @@
> /* { dg-require-effective-target float128 } */
> /* { dg-options "-mpower9-vector -O2 -ffast-math" } */
>
> +/* If the compiler was configured to automatically generate power10 support
> with
> + --with-cpu=power10, turn it off. Otherwise, it will generate XXMAXCQP and
> + XXMINCQP instructions. */
> +#ifdef _ARCH_PWR10
> +#pragma GCC target ("cpu=power9")
> +#endif
Probably fine.. It's good to exercise the pragma target stuff, thoguh
I wonder if it would be better to just specify -mcpu=power9 in the
options since we are already specifying (redundant?) -mpower9-vector.
I see similar changes in a later patch, probably OK there since those
tests do not appear to be specifying -mcpu=foo options that are already
pointed at power9 features...
> +
> #ifndef TYPE
> #define TYPE _Float128
> #endif
> --
> 2.31.1
lgtm,
thanks
-Will
>
>
