Re: [PATCH 4/4] PowerPC: Add power10 xscmp{eq,gt,ge}qp support
On Wed, Aug 26, 2020 at 10:46:37PM -0400, Michael Meissner wrote: > diff --git a/gcc/config/rs6000/predicates.md b/gcc/config/rs6000/predicates.md > index 2709e46f7e5..60b45601e9b 100644 > --- a/gcc/config/rs6000/predicates.md > +++ b/gcc/config/rs6000/predicates.md > @@ -1225,6 +1225,11 @@ (define_predicate "fpmask_comparison_operator" > (define_predicate "invert_fpmask_comparison_operator" >(match_code "ne,unlt,unle")) > > +;; Return 1 if OP is either a fpmask_comparison_operator or > +;; invert_fpmask_comparsion_operator. > +(define_predicate "fpmask_normal_or_invert_operator" > + (match_code "eq,gt,ge,ne,unlt,unle")) Keep "comparison" in the name? Maybe "any_fpmask_comparison_operator", we have other things named in that scheme already. > --- a/gcc/config/rs6000/rs6000.c > +++ b/gcc/config/rs6000/rs6000.c > @@ -15177,6 +15177,10 @@ have_compare_and_set_mask (machine_mode mode) > case DFmode: >return TARGET_P9_MINMAX; > > +case KFmode: > +case TFmode: > + return FLOAT128_IEEE_MINMAX_P (mode); That needs the E_ stuff as well. > +;; Secondary iterator for scalar binary floating point operations. This is > +;; used for the conditional moves when we have a compare and set mask > +;; instruction. Using this instruction allows us to do a conditional move > +;; where the comparison type might be different from the values being moved. > +(define_mode_iterator FSCALAR2 [SF > + DF > + (KF "FLOAT128_IEEE_MINMAX_P (KFmode)") > + (TF "FLOAT128_IEEE_MINMAX_P (TFmode)")]) Needs a name change just like FSCALAR. Maybe BFP? Or better, just FP, and rename the current FP to something else (it is only used for cstore and cbranch, it should use a much less generic name there). Please cut down the comment. See GPR/GPR2 for example: ; This mode iterator allows :GPR to be used to indicate the allowable size ; of whole values in GPRs. (define_mode_iterator GPR [SI (DI "TARGET_POWERPC64")]) ; And again, for patterns that need two (potentially) different integer modes. (define_mode_iterator GPR2 [SI (DI "TARGET_POWERPC64")]) It should not talk about an example where it is used: it can much easier say something much more generic! (And then send a patch first doing FP just as SFDF and replacing it where we want it; and then a later patch adding KF. That way, your patch might be readable!) Thanks, Segher
Re: [PATCH 4/4] PowerPC: Add power10 xscmp{eq,gt,ge}qp support
On Wed, 2020-08-26 at 22:46 -0400, Michael Meissner via Gcc-patches wrote:
> PowerPC: Add power10 xscmp{eq,gt,ge}qp support.
>
> This patch adds the conditional move support. In adding the conditional move
> support, the optimizers will be able to convert things like:
>
> a = (b > c) ? b : c;
>
> into the instructions. This patch merges together the scalar SF/DF
> conditional
> move with the scalar KF/TF conditional move. It extends the optimization that
> was previously used for SFmode and DFmode to allow the comparison to be a
> different scalar floating point mode than the move. I.e.
>
> __float128 a, b, c;
> float x, y;
>
> /* ... */
>
> a = (x == y) ? b : c;
>
> I did have to add an XXPERMDI if the comparison mode was SFmode or DFmode, and
> the move mode is KFmode or TFmode (the XSCMP{EQ,GT,GE}DP instructions
> explicitly set the bottom 64 bits of the vector register to 0).
>
> I have built compilers on a little endian power9 Linux system with all 4
> patches applied. I did bootstrap builds and ran the testsuite, with no
> regressions. Previous versions of the patch was also tested on a little
> endian
> power8 Linux system. I would like to check this patch into the master branch
> for GCC 11. At this time, I do not anticipate needing to backport these
> changes to GCC 10.3.
>
> gcc/
> 2020-08-26 Michael Meissner
>
> * config/rs6000/predicates.md (fpmask_normal_or_invert_operator):
> New predicate.
> * config/rs6000/rs6000.c (have_compare_and_set_mask): Add IEEE
> 128-bit floating point types.
> * config/rs6000/rs6000.md (FSCALAR2): New iterator for floating
> point conditional moves.
> (movcc_p9): Replace with
> mov.
(movcc): Update to use FSCALAR iterator
> (movcc_invert_p9): Replace with
> mov.
> (mov): Combine both
If i followed correctly, that should that be
*movcc " ..?
> movcc_p9 and
> movcc_invert_p9 patterns. Add ISA 3.1
> support for IEEE 128-bit conditional moves. Always use an
> earlyclobber register for the mask. Use XXPERMDI to extend the
> mask if we have a 64-bit comparison and 128-bit move.
> register for the mask.
ok
> (fpmask, xxsel): Add ISA 3.1 support for IEEE 128-bit
> conditional moves. Enable the generator functionality so
> mov can call it. Update constraints
> for 128-bit operations.
ok
reviewed the rest, nothing else jumped out at me. lgtm, thanks
-Will
>
> gcc/testsuite/
> 2020-08-26 Michael Meissner
>
> * gcc.target/powerpc/float128-cmove.c: New test.
> * gcc.target/powerpc/float128-minmax-3.c: New test.
>
> ---
> gcc/config/rs6000/predicates.md | 5 +
> gcc/config/rs6000/rs6000.c| 4 +
> gcc/config/rs6000/rs6000.md | 154 ++
> .../gcc.target/powerpc/float128-cmove.c | 93 +++
> .../gcc.target/powerpc/float128-minmax-3.c| 15 ++
> 5 files changed, 200 insertions(+), 71 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/predicates.md b/gcc/config/rs6000/predicates.md
> index 2709e46f7e5..60b45601e9b 100644
> --- a/gcc/config/rs6000/predicates.md
> +++ b/gcc/config/rs6000/predicates.md
> @@ -1225,6 +1225,11 @@ (define_predicate "fpmask_comparison_operator"
> (define_predicate "invert_fpmask_comparison_operator"
>(match_code "ne,unlt,unle"))
>
> +;; Return 1 if OP is either a fpmask_comparison_operator or
> +;; invert_fpmask_comparsion_operator.
> +(define_predicate "fpmask_normal_or_invert_operator"
> + (match_code "eq,gt,ge,ne,unlt,unle"))
> +
> ;; Return 1 if OP is a comparison operation suitable for integer
> vector/scalar
> ;; comparisons that generate a -1/0 mask.
> (define_predicate "vecint_comparison_operator"
> diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
> index 05eb141a2cd..403897926c5 100644
> --- a/gcc/config/rs6000/rs6000.c
> +++ b/gcc/config/rs6000/rs6000.c
> @@ -15177,6 +15177,10 @@ have_compare_and_set_mask (machine_mode mode)
> case DFmode:
>return TARGET_P9_MINMAX;
>
> +case KFmode:
> +case TFmode:
> + return FLOAT128_IEEE_MINMAX_P (mode);
> +
> default:
>break;
> }
> diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
> index 006e60f09bc..147c635994c 100644
> --- a/gcc/config/rs6000/rs6000.md
> +++ b/gcc/config/rs6000/rs6000.md
> @@ -795,6 +795,15 @@ (define_mode_iterator FSCALAR [SF
> (KF "FLOAT128_IEEE_MINMAX_P (KFmode)")
> (TF "FLOAT128_IEEE_MINMAX_P (TFmode)")])
>
> +;; Secondary iterator for scalar binary floating point operations. This is
> +;; used for the conditional moves when we have a compare and set mask
> +;; instruction. Using this instr
[PATCH 4/4] PowerPC: Add power10 xscmp{eq,gt,ge}qp support
PowerPC: Add power10 xscmp{eq,gt,ge}qp support.
This patch adds the conditional move support. In adding the conditional move
support, the optimizers will be able to convert things like:
a = (b > c) ? b : c;
into the instructions. This patch merges together the scalar SF/DF conditional
move with the scalar KF/TF conditional move. It extends the optimization that
was previously used for SFmode and DFmode to allow the comparison to be a
different scalar floating point mode than the move. I.e.
__float128 a, b, c;
float x, y;
/* ... */
a = (x == y) ? b : c;
I did have to add an XXPERMDI if the comparison mode was SFmode or DFmode, and
the move mode is KFmode or TFmode (the XSCMP{EQ,GT,GE}DP instructions
explicitly set the bottom 64 bits of the vector register to 0).
I have built compilers on a little endian power9 Linux system with all 4
patches applied. I did bootstrap builds and ran the testsuite, with no
regressions. Previous versions of the patch was also tested on a little endian
power8 Linux system. I would like to check this patch into the master branch
for GCC 11. At this time, I do not anticipate needing to backport these
changes to GCC 10.3.
gcc/
2020-08-26 Michael Meissner
* config/rs6000/predicates.md (fpmask_normal_or_invert_operator):
New predicate.
* config/rs6000/rs6000.c (have_compare_and_set_mask): Add IEEE
128-bit floating point types.
* config/rs6000/rs6000.md (FSCALAR2): New iterator for floating
point conditional moves.
(movcc_p9): Replace with
mov.
(movcc_invert_p9): Replace with
mov.
(mov): Combine both
movcc_p9 and
movcc_invert_p9 patterns. Add ISA 3.1
support for IEEE 128-bit conditional moves. Always use an
earlyclobber register for the mask. Use XXPERMDI to extend the
mask if we have a 64-bit comparison and 128-bit move.
register for the mask.
(fpmask, xxsel): Add ISA 3.1 support for IEEE 128-bit
conditional moves. Enable the generator functionality so
mov can call it. Update constraints
for 128-bit operations.
gcc/testsuite/
2020-08-26 Michael Meissner
* gcc.target/powerpc/float128-cmove.c: New test.
* gcc.target/powerpc/float128-minmax-3.c: New test.
---
gcc/config/rs6000/predicates.md | 5 +
gcc/config/rs6000/rs6000.c| 4 +
gcc/config/rs6000/rs6000.md | 154 ++
.../gcc.target/powerpc/float128-cmove.c | 93 +++
.../gcc.target/powerpc/float128-minmax-3.c| 15 ++
5 files changed, 200 insertions(+), 71 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/predicates.md b/gcc/config/rs6000/predicates.md
index 2709e46f7e5..60b45601e9b 100644
--- a/gcc/config/rs6000/predicates.md
+++ b/gcc/config/rs6000/predicates.md
@@ -1225,6 +1225,11 @@ (define_predicate "fpmask_comparison_operator"
(define_predicate "invert_fpmask_comparison_operator"
(match_code "ne,unlt,unle"))
+;; Return 1 if OP is either a fpmask_comparison_operator or
+;; invert_fpmask_comparsion_operator.
+(define_predicate "fpmask_normal_or_invert_operator"
+ (match_code "eq,gt,ge,ne,unlt,unle"))
+
;; Return 1 if OP is a comparison operation suitable for integer vector/scalar
;; comparisons that generate a -1/0 mask.
(define_predicate "vecint_comparison_operator"
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 05eb141a2cd..403897926c5 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -15177,6 +15177,10 @@ have_compare_and_set_mask (machine_mode mode)
case DFmode:
return TARGET_P9_MINMAX;
+case KFmode:
+case TFmode:
+ return FLOAT128_IEEE_MINMAX_P (mode);
+
default:
break;
}
diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
index 006e60f09bc..147c635994c 100644
--- a/gcc/config/rs6000/rs6000.md
+++ b/gcc/config/rs6000/rs6000.md
@@ -795,6 +795,15 @@ (define_mode_iterator FSCALAR [SF
(KF "FLOAT128_IEEE_MINMAX_P (KFmode)")
(TF "FLOAT128_IEEE_MINMAX_P (TFmode)")])
+;; Secondary iterator for scalar binary floating point operations. This is
+;; used for the conditional moves when we have a compare and set mask
+;; instruction. Using this instruction allows us to do a conditional move
+;; where the comparison type might be different from the values being moved.
+(define_mode_iterator FSCALAR2 [SF
+ DF
+ (KF "FLOAT128_IEEE_MINMAX_P (KFmode)")
+ (TF "FLOAT128_IEEE_MINMAX_P (TFmode)")])
+
;; Constraints to use for scalar FP operations
(define_mode_attr Fm [(SF "wa")
