On Thu, Sep 19, 2024 at 10:50 PM Jakub Jelinek <ja...@redhat.com> wrote:
>
> Hi!
>
> min/max patterns for intrinsics which on x86 result in the second
> input operand if the two operands are both zeros or one or both of them
> are a NaN shouldn't use SMIN/SMAX RTL, because that is similarly to
> MIN_EXPR/MAX_EXPR undefined what will be the result in those cases.
>
> The following patch adds an expander which uses either a new pattern with
> UNSPEC_IEEE_M{AX,IN} or use the S{MIN,MAX} representation of the same.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> P.S. I have a patch to replace UNSPEC_IEEE_M{AX,IN} with IF_THEN_ELSE
> (except for the 3dNOW! PFMIN/MAX, those actually are documented to behave
> differently), but it actually doesn't improve anything much, as
> simplify_const_relational_operation nor simplify_ternary_operation aren't
> able to fold comparisons with two CONST_VECTOR operands or IF_THEN_ELSE
> with 3 CONST_VECTOR operands.
> So, maybe better approach will be to generic fold the builtins with constant
> arguments (maybe leaving NaNs to runtime).
It would be possible to fold them in the gimple folding hook to VEC_COND_EXPRs
with the chance the min/max operation being lost when expanding to RTL.
Richard.
>
> 2024-09-19 Uros Bizjak <ubiz...@gmail.com>
> Jakub Jelinek <ja...@redhat.com>
>
> PR target/116738
> * config/i386/subst.md (mask_scalar_operand_arg34,
> mask_scalar_expand_op3, round_saeonly_scalar_mask_arg3): New
> subst attributes.
> * config/i386/sse.md
> (<sse>_vm<code><mode>3<mask_scalar_name><round_saeonly_scalar_name>):
> Change from define_insn to define_expand, rename the old define_insn
> to ...
> (*<sse>_vm<code><mode>3<mask_scalar_name><round_saeonly_scalar_name>):
> ... this.
>
> (<sse>_ieee_vm<ieee_maxmin><mode>3<mask_scalar_name><round_saeonly_scalar_name>):
> New define_insn.
>
> * gcc.target/i386/sse-pr116738.c: New test.
>
> --- gcc/config/i386/subst.md.jj 2024-09-18 15:49:42.200791315 +0200
> +++ gcc/config/i386/subst.md 2024-09-19 12:32:51.048626421 +0200
> @@ -366,6 +366,8 @@ (define_subst_attr "mask_scalar_operand4
> (define_subst_attr "mask_scalarcz_operand4" "mask_scalarcz" "" "%{%5%}%N4")
> (define_subst_attr "mask_scalar4_dest_false_dep_for_glc_cond" "mask_scalar"
> "1" "operands[4] == CONST0_RTX(<MODE>mode)")
> (define_subst_attr "mask_scalarc_dest_false_dep_for_glc_cond" "mask_scalarc"
> "1" "operands[3] == CONST0_RTX(V8HFmode)")
> +(define_subst_attr "mask_scalar_operand_arg34" "mask_scalar" "" ",
> operands[3], operands[4]")
> +(define_subst_attr "mask_scalar_expand_op3" "mask_scalar" "3" "5")
>
> (define_subst "mask_scalar"
> [(set (match_operand:SUBST_V 0)
> @@ -473,6 +475,7 @@ (define_subst_attr "round_saeonly_scalar
> (define_subst_attr "round_saeonly_scalar_constraint" "round_saeonly_scalar"
> "vm" "v")
> (define_subst_attr "round_saeonly_scalar_prefix" "round_saeonly_scalar"
> "vex" "evex")
> (define_subst_attr "round_saeonly_scalar_nimm_predicate"
> "round_saeonly_scalar" "nonimmediate_operand" "register_operand")
> +(define_subst_attr "round_saeonly_scalar_mask_arg3" "round_saeonly_scalar"
> "" ", operands[<mask_scalar_expand_op3>]")
>
> (define_subst "round_saeonly_scalar"
> [(set (match_operand:SUBST_V 0)
> --- gcc/config/i386/sse.md.jj 2024-09-10 16:26:02.875151133 +0200
> +++ gcc/config/i386/sse.md 2024-09-19 12:43:31.693030695 +0200
> @@ -3333,7 +3333,27 @@ (define_insn "*ieee_<ieee_maxmin><mode>3
> (const_string "*")))
> (set_attr "mode" "<ssescalarmode>")])
>
> -(define_insn
> "<sse>_vm<code><mode>3<mask_scalar_name><round_saeonly_scalar_name>"
> +(define_expand
> "<sse>_vm<code><mode>3<mask_scalar_name><round_saeonly_scalar_name>"
> + [(set (match_operand:VFH_128 0 "register_operand")
> + (vec_merge:VFH_128
> + (smaxmin:VFH_128
> + (match_operand:VFH_128 1 "register_operand")
> + (match_operand:VFH_128 2 "nonimmediate_operand"))
> + (match_dup 1)
> + (const_int 1)))]
> + "TARGET_SSE"
> +{
> + if (!flag_finite_math_only || flag_signed_zeros)
> + {
> + emit_insn
> (gen_<sse>_ieee_vm<maxmin_float><mode>3<mask_scalar_name><round_saeonly_scalar_name>
> + (operands[0], operands[1], operands[2]
> + <mask_scalar_operand_arg34>
> + <round_saeonly_scalar_mask_arg3>));
> + DONE;
> + }
> +})
> +
> +(define_insn
> "*<sse>_vm<code><mode>3<mask_scalar_name><round_saeonly_scalar_name>"
> [(set (match_operand:VFH_128 0 "register_operand" "=x,v")
> (vec_merge:VFH_128
> (smaxmin:VFH_128
> @@ -3348,6 +3368,25 @@ (define_insn "<sse>_vm<code><mode>3<mask
> [(set_attr "isa" "noavx,avx")
> (set_attr "type" "sse")
> (set_attr "btver2_sse_attr" "maxmin")
> + (set_attr "prefix" "<round_saeonly_scalar_prefix>")
> + (set_attr "mode" "<ssescalarmode>")])
> +
> +(define_insn
> "<sse>_ieee_vm<ieee_maxmin><mode>3<mask_scalar_name><round_saeonly_scalar_name>"
> + [(set (match_operand:VFH_128 0 "register_operand" "=x,v")
> + (vec_merge:VFH_128
> + (unspec:VFH_128
> + [(match_operand:VFH_128 1 "register_operand" "0,v")
> + (match_operand:VFH_128 2 "nonimmediate_operand"
> "xm,<round_saeonly_scalar_constraint>")]
> + IEEE_MAXMIN)
> + (match_dup 1)
> + (const_int 1)))]
> + "TARGET_SSE"
> + "@
> + <ieee_maxmin><ssescalarmodesuffix>\t{%2, %0|%0, %<iptr>2}
> + v<ieee_maxmin><ssescalarmodesuffix>\t{<round_saeonly_scalar_mask_op3>%2,
> %1, %0<mask_scalar_operand3>|%0<mask_scalar_operand3>, %1,
> %<iptr>2<round_saeonly_scalar_mask_op3>}"
> + [(set_attr "isa" "noavx,avx")
> + (set_attr "type" "sse")
> + (set_attr "btver2_sse_attr" "maxmin")
> (set_attr "prefix" "<round_saeonly_scalar_prefix>")
> (set_attr "mode" "<ssescalarmode>")])
>
> --- gcc/testsuite/gcc.target/i386/sse-pr116738.c.jj 2024-09-19
> 12:52:33.502681950 +0200
> +++ gcc/testsuite/gcc.target/i386/sse-pr116738.c 2024-09-19
> 12:54:20.938219741 +0200
> @@ -0,0 +1,28 @@
> +/* PR target/116738 */
> +/* { dg-do run } */
> +/* { dg-options "-O2 -msse" } */
> +/* { dg-require-effective-target sse } */
> +
> +#include "sse-check.h"
> +
> +static inline float
> +clamp (float f)
> +{
> + __m128 v = _mm_set_ss (f);
> + __m128 zero = _mm_setzero_ps ();
> + __m128 greatest = _mm_set_ss (__FLT_MAX__);
> + v = _mm_min_ss (v, greatest);
> + v = _mm_max_ss (v, zero);
> + return _mm_cvtss_f32 (v);
> +}
> +
> +static void
> +sse_test (void)
> +{
> + float f = clamp (-0.0f);
> + if (f != 0.0f || __builtin_signbitf (f))
> + abort ();
> + f = clamp (__builtin_nanf (""));
> + if (__builtin_isnanf (f) || f != __FLT_MAX__)
> + abort ();
> +}
>
> Jakub
>
