This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Unconditionally use MAX_EXPR/MIN_EXPR for MAX/MIN intrinsics


PING

On Fri, Aug 10, 2018 at 11:47 PM, Janne Blomqvist <blomqvist.janne@gmail.com
> wrote:

> For floating point types, the question is what MAX(a, NaN) or MIN(a,
> NaN) should return (where "a" is a normal number).  There are valid
> usecases for returning either one, but the Fortran standard doesn't
> specify which one should be chosen.  Also, there is no consensus among
> other tested compilers.  In short, it's a mess.  So lets just do
> whatever is fastest, which is using MAX_EXPR/MIN_EXPR which are not
> defined to do anything in particular if one of the operands is a NaN.
>
> Regtested on x86_64-pc-linux-gnu, Ok for trunk?
>
> gcc/fortran/ChangeLog:
>
> 2018-08-10  Janne Blomqvist  <jb@gcc.gnu.org>
>
>         * trans-intrinsic.c (gfc_conv_intrinsic_minmax): Use
>         MAX_EXPR/MIN_EXPR unconditionally for real arguments.
>
> gcc/testsuite/ChangeLog:
>
> 2018-08-10  Janne Blomqvist  <jb@gcc.gnu.org>
>
>         * gfortran.dg/nan_1.f90: Remove tests that test MAX/MIN with NaNs.
> ---
>  gcc/fortran/trans-intrinsic.c       | 55 ++++++-----------------------
>  gcc/testsuite/gfortran.dg/nan_1.f90 | 35 ------------------
>  2 files changed, 10 insertions(+), 80 deletions(-)
>
> diff --git a/gcc/fortran/trans-intrinsic.c b/gcc/fortran/trans-intrinsic.c
> index c9b5479740c..190fde66a8d 100644
> --- a/gcc/fortran/trans-intrinsic.c
> +++ b/gcc/fortran/trans-intrinsic.c
> @@ -3914,8 +3914,6 @@ gfc_conv_intrinsic_minmax (gfc_se * se, gfc_expr *
> expr, enum tree_code op)
>    mvar = gfc_create_var (type, "M");
>    gfc_add_modify (&se->pre, mvar, args[0]);
>
> -  internal_fn ifn = op == GT_EXPR ? IFN_FMAX : IFN_FMIN;
> -
>    for (i = 1, argexpr = argexpr->next; i < nargs; i++, argexpr =
> argexpr->next)
>      {
>        tree cond = NULL_TREE;
> @@ -3936,49 +3934,16 @@ gfc_conv_intrinsic_minmax (gfc_se * se, gfc_expr *
> expr, enum tree_code op)
>         val = gfc_evaluate_now (val, &se->pre);
>
>        tree calc;
> -      /* If we dealing with integral types or we don't care about NaNs
> -        just do a MIN/MAX_EXPR.  */
> -      if (!HONOR_NANS (type) && !HONOR_SIGNED_ZEROS (type))
> -       {
> -
> -         tree_code code = op == GT_EXPR ? MAX_EXPR : MIN_EXPR;
> -         calc = fold_build2_loc (input_location, code, type,
> -                                 convert (type, val), mvar);
> -         tmp = build2_v (MODIFY_EXPR, mvar, calc);
> -
> -       }
> -      /* If we care about NaNs and we have internal functions available
> for
> -        fmin/fmax to perform the comparison, use those.  */
> -      else if (SCALAR_FLOAT_TYPE_P (type)
> -             && direct_internal_fn_supported_p (ifn, type,
> OPTIMIZE_FOR_SPEED))
> -       {
> -         calc = build_call_expr_internal_loc (input_location, ifn, type,
> -                                               2, mvar, convert (type,
> val));
> -         tmp = build2_v (MODIFY_EXPR, mvar, calc);
> -
> -       }
> -      /* Otherwise expand to:
> -       mvar = a1;
> -       if (a2 .op. mvar || isnan (mvar))
> -         mvar = a2;
> -       if (a3 .op. mvar || isnan (mvar))
> -         mvar = a3;
> -       ...  */
> -      else
> -       {
> -         tree isnan = build_call_expr_loc (input_location,
> -                                       builtin_decl_explicit
> (BUILT_IN_ISNAN),
> -                                       1, mvar);
> -         tmp = fold_build2_loc (input_location, op, logical_type_node,
> -                                convert (type, val), mvar);
> -
> -         tmp = fold_build2_loc (input_location, TRUTH_OR_EXPR,
> -                                 logical_type_node, tmp,
> -                                 fold_convert (logical_type_node, isnan));
> -         tmp = build3_v (COND_EXPR, tmp,
> -                         build2_v (MODIFY_EXPR, mvar, convert (type,
> val)),
> -                         build_empty_stmt (input_location));
> -       }
> +      /* For floating point types, the question is what MAX(a, NaN) or
> +        MIN(a, NaN) should return (where "a" is a normal number).
> +        There are valid usecase for returning either one, but the
> +        Fortran standard doesn't specify which one should be chosen.
> +        Also, there is no consensus among other tested compilers.  In
> +        short, it's a mess.  So lets just do whatever is fastest.  */
> +      tree_code code = op == GT_EXPR ? MAX_EXPR : MIN_EXPR;
> +      calc = fold_build2_loc (input_location, code, type,
> +                             convert (type, val), mvar);
> +      tmp = build2_v (MODIFY_EXPR, mvar, calc);
>
>        if (cond != NULL_TREE)
>         tmp = build3_v (COND_EXPR, cond, tmp,
> diff --git a/gcc/testsuite/gfortran.dg/nan_1.f90
> b/gcc/testsuite/gfortran.dg/nan_1.f90
> index e64b4ce65e1..1b39cc1f21c 100644
> --- a/gcc/testsuite/gfortran.dg/nan_1.f90
> +++ b/gcc/testsuite/gfortran.dg/nan_1.f90
> @@ -66,35 +66,12 @@ program test
>    if (isinf(-nan) .or. isinf(-large) .or. .not. isinf(-inf)) STOP 4
>
>    ! Check that MIN and MAX behave correctly
> -  if (max(2.0, nan) /= 2.0) STOP 5
> -  if (min(2.0, nan) /= 2.0) STOP 6
> -  if (max(nan, 2.0) /= 2.0) STOP 7
> -  if (min(nan, 2.0) /= 2.0) STOP 8
> -
> -  if (max(2.d0, nan) /= 2.d0) STOP 9! { dg-warning "Extension: Different
> type kinds" }
> -  if (min(2.d0, nan) /= 2.d0) STOP 10! { dg-warning "Extension: Different
> type kinds" }
> -  if (max(nan, 2.d0) /= 2.d0) STOP 11! { dg-warning "Extension: Different
> type kinds" }
> -  if (min(nan, 2.d0) /= 2.d0) STOP 12! { dg-warning "Extension: Different
> type kinds" }
>
>    if (.not. isnan(min(nan,nan))) STOP 13
>    if (.not. isnan(max(nan,nan))) STOP 14
>
>    ! Same thing, with more arguments
>
> -  if (max(3.0, 2.0, nan) /= 3.0) STOP 15
> -  if (min(3.0, 2.0, nan) /= 2.0) STOP 16
> -  if (max(3.0, nan, 2.0) /= 3.0) STOP 17
> -  if (min(3.0, nan, 2.0) /= 2.0) STOP 18
> -  if (max(nan, 3.0, 2.0) /= 3.0) STOP 19
> -  if (min(nan, 3.0, 2.0) /= 2.0) STOP 20
> -
> -  if (max(3.d0, 2.d0, nan) /= 3.d0) STOP 21! { dg-warning "Extension:
> Different type kinds" }
> -  if (min(3.d0, 2.d0, nan) /= 2.d0) STOP 22! { dg-warning "Extension:
> Different type kinds" }
> -  if (max(3.d0, nan, 2.d0) /= 3.d0) STOP 23! { dg-warning "Extension:
> Different type kinds" }
> -  if (min(3.d0, nan, 2.d0) /= 2.d0) STOP 24! { dg-warning "Extension:
> Different type kinds" }
> -  if (max(nan, 3.d0, 2.d0) /= 3.d0) STOP 25! { dg-warning "Extension:
> Different type kinds" }
> -  if (min(nan, 3.d0, 2.d0) /= 2.d0) STOP 26! { dg-warning "Extension:
> Different type kinds" }
> -
>    if (.not. isnan(min(nan,nan,nan))) STOP 27
>    if (.not. isnan(max(nan,nan,nan))) STOP 28
>    if (.not. isnan(min(nan,nan,nan,nan))) STOP 29
> @@ -105,20 +82,8 @@ program test
>    ! Large values, INF and NaNs
>    if (.not. isinf(max(large, inf))) STOP 33
>    if (isinf(min(large, inf))) STOP 34
> -  if (.not. isinf(max(nan, large, inf))) STOP 35
> -  if (isinf(min(nan, large, inf))) STOP 36
> -  if (.not. isinf(max(large, nan, inf))) STOP 37
> -  if (isinf(min(large, nan, inf))) STOP 38
> -  if (.not. isinf(max(large, inf, nan))) STOP 39
> -  if (isinf(min(large, inf, nan))) STOP 40
>
>    if (.not. isinf(min(-large, -inf))) STOP 41
>    if (isinf(max(-large, -inf))) STOP 42
> -  if (.not. isinf(min(nan, -large, -inf))) STOP 43
> -  if (isinf(max(nan, -large, -inf))) STOP 44
> -  if (.not. isinf(min(-large, nan, -inf))) STOP 45
> -  if (isinf(max(-large, nan, -inf))) STOP 46
> -  if (.not. isinf(min(-large, -inf, nan))) STOP 47
> -  if (isinf(max(-large, -inf, nan))) STOP 48
>
>  end program test
> --
> 2.17.1
>
>


-- 
Janne Blomqvist


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]