> On Wed, Jun 25, 2025 at 10:04:41AM +0200, Juergen Christ wrote: > > On VXE targets, we can directly use the fp min/max instruction instead of > > calling into libm for fmin/fmax etc. > > > > Provide fmin/fmax versions also for vectors even though it cannot be > > called directly. This will be exploited with a follow-up patch when > > reductions are introduced. > > > > Bootstrapped and regtested on s390. Ok for trunk? > > > > gcc/ChangeLog: > > > > * config/s390/s390.md: Update UNSPECs > > * config/s390/vector.md (fmax<mode>3): New expander. > > (fmin<mode>3): New expander. > > * config/s390/vx-builtins.md (*fmin<mode>): New insn. > > (vfmin<mode>): Redefined to use new insn. > > (*fmax<mode>): New insn. > > (vfmax<mode>): Redefined to use new insn. > > > > gcc/testsuite/ChangeLog: > > > > * gcc.target/s390/fminmax-1.c: New test. > > * gcc.target/s390/fminmax-2.c: New test. > > > > Signed-off-by: Juergen Christ <jchr...@linux.ibm.com> > > --- > > gcc/config/s390/s390.md | 6 +- > > gcc/config/s390/vector.md | 25 ++++++++ > > gcc/config/s390/vx-builtins.md | 29 ++++++--- > > gcc/testsuite/gcc.target/s390/fminmax-1.c | 77 +++++++++++++++++++++++ > > gcc/testsuite/gcc.target/s390/fminmax-2.c | 29 +++++++++ > > 5 files changed, 156 insertions(+), 10 deletions(-) > > create mode 100644 gcc/testsuite/gcc.target/s390/fminmax-1.c > > create mode 100644 gcc/testsuite/gcc.target/s390/fminmax-2.c > > > > diff --git a/gcc/config/s390/s390.md b/gcc/config/s390/s390.md > > index 97a4bdf96b2d..1c88c9624b60 100644 > > --- a/gcc/config/s390/s390.md > > +++ b/gcc/config/s390/s390.md > > @@ -241,9 +241,6 @@ > > > > UNSPEC_VEC_MSUM > > > > - UNSPEC_VEC_VFMIN > > - UNSPEC_VEC_VFMAX > > - > > UNSPEC_VEC_VBLEND > > UNSPEC_VEC_VEVAL > > UNSPEC_VEC_VGEM > > @@ -256,6 +253,9 @@ > > > > UNSPEC_NNPA_VCFN_V8HI > > UNSPEC_NNPA_VCNF_V8HI > > + > > + UNSPEC_FMAX > > + UNSPEC_FMIN > > ]) > > > > ;; > > diff --git a/gcc/config/s390/vector.md b/gcc/config/s390/vector.md > > index 6f4e1929eb80..8bda30624c22 100644 > > --- a/gcc/config/s390/vector.md > > +++ b/gcc/config/s390/vector.md > > @@ -89,6 +89,13 @@ > > (define_mode_iterator VF_HW [(V4SF "TARGET_VXE") V2DF (V1TF "TARGET_VXE") > > (TF "TARGET_VXE")]) > > > > +; FP scalar and vector modes > > +(define_mode_iterator VFT_BFP [SF DF > > + (V1SF "TARGET_VXE") (V2SF "TARGET_VXE") (V4SF > > "TARGET_VXE") > > + V1DF V2DF > > + (V1TF "TARGET_VXE") (TF "TARGET_VXE")]) > > + > > + > > (define_mode_iterator V_8 [V1QI]) > > (define_mode_iterator V_16 [V2QI V1HI]) > > (define_mode_iterator V_32 [V4QI V2HI V1SI V1SF]) > > @@ -3576,3 +3583,21 @@ > > ; vec_unpacks_float_lo > > ; vec_unpacku_float_hi > > ; vec_unpacku_float_lo > > + > > +; fmax > > +(define_expand "fmax<mode>3" > > + [(set (match_operand:VFT_BFP 0 "register_operand" "=v") > > + (unspec:VFT_BFP [(match_operand:VFT_BFP 1 "register_operand" "v") > > + (match_operand:VFT_BFP 2 "register_operand" "v") > > + (const_int 4)] > > + UNSPEC_FMAX))] > > + "TARGET_VXE") > > Expanders don't have constraints and should be removed. > Wrong indentation.
Done. > > + > > +; fmin > > +(define_expand "fmin<mode>3" > > + [(set (match_operand:VFT_BFP 0 "register_operand" "=v") > > + (unspec:VFT_BFP [(match_operand:VFT_BFP 1 "register_operand" "v") > > + (match_operand:VFT_BFP 2 "register_operand" "v") > > + (const_int 4)] > > + UNSPEC_FMIN))] > > + "TARGET_VXE") > > Ditto. Done. > > diff --git a/gcc/config/s390/vx-builtins.md b/gcc/config/s390/vx-builtins.md > > index a7bb7ff92f5e..0508df43b866 100644 > > --- a/gcc/config/s390/vx-builtins.md > > +++ b/gcc/config/s390/vx-builtins.md > > @@ -2136,15 +2136,32 @@ > > "<vw>fche<sdx>bs\t%v2,%v0,%v1" > > [(set_attr "op_type" "VRR")]) > > > > +(define_insn "*fmin<mode>" > > + [(set (match_operand:VFT_BFP 0 "register_operand" "=v") > ^ > ~~~~~~ > Wrong indentation. Fixed. > > + (unspec:VFT_BFP [(match_operand:VFT_BFP 1 "register_operand" "v") > > + (match_operand:VFT_BFP 2 "register_operand" "v") > > + (match_operand:QI 3 "const_mask_operand" "C")] > > + UNSPEC_FMIN))] > > + "TARGET_VXE" > > + "<vw>fmin<sdx>b\t%v0,%v1,%v2,%b3" > > + [(set_attr "op_type" "VRR")]) > > > > -(define_insn "vfmin<mode>" > > +(define_expand "vfmin<mode>" > > [(set (match_operand:VF_HW 0 "register_operand" "=v") > > (unspec:VF_HW [(match_operand:VF_HW 1 "register_operand" "v") > > (match_operand:VF_HW 2 "register_operand" "v") > > (match_operand:QI 3 "const_mask_operand" "C")] > > - UNSPEC_VEC_VFMIN))] > > + UNSPEC_FMIN))] > > + "TARGET_VXE") > > Expanders don't have constraints. > > Anyhow, you could also merge > > define_insn "*fmin<mode>" > > and define_expand "vfmin<mode>" > > into > > define_insn "vfmin<mode>" > > using iterator VFT_BFP since VFT_BFP subsumes VF_HW. Done. > > + > > +(define_insn "*fmax<mode>" > > + [(set (match_operand:VFT_BFP 0 "register_operand" "=v") > > + (unspec:VFT_BFP [(match_operand:VFT_BFP 1 "register_operand" "v") > > + (match_operand:VFT_BFP 2 "register_operand" "v") > > + (match_operand:QI 3 "const_mask_operand" "C")] > > + UNSPEC_FMAX))] > > "TARGET_VXE" > > - "<vw>fmin<sdx>b\t%v0,%v1,%v2,%b3" > > + "<vw>fmax<sdx>b\t%v0,%v1,%v2,%b3" > > [(set_attr "op_type" "VRR")]) > > > > (define_insn "vfmax<mode>" > > @@ -2152,10 +2169,8 @@ > > (unspec:VF_HW [(match_operand:VF_HW 1 "register_operand" "v") > > (match_operand:VF_HW 2 "register_operand" "v") > > (match_operand:QI 3 "const_mask_operand" "C")] > > - UNSPEC_VEC_VFMAX))] > > - "TARGET_VXE" > > - "<vw>fmax<sdx>b\t%v0,%v1,%v2,%b3" > > - [(set_attr "op_type" "VRR")]) > > + UNSPEC_FMAX))] > > + "TARGET_VXE") > > An insn always needs an output template. This was probably meant to be > an expander? If so, then the constraints should be removed. > > Likewise as for min, both could be merged into a single define_insn. Done. Should I send a v3?
