Message ID | 20180810204728.20191-1-blomqvist.janne@gmail.com |
---|---|
State | New |
Headers | show |
Series | Unconditionally use MAX_EXPR/MIN_EXPR for MAX/MIN intrinsics | expand |
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 > >
Hi Janne, > 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? OK. Could you also document this behavior in the "Compiler Characteristics" section, and make mention of the change in gcc-9/changes.html ? Regards Thomas
On Sun, Aug 19, 2018 at 10:47 PM Thomas Koenig <tkoenig@netcologne.de> wrote: > Hi Janne, > > > 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? > > OK. > Thanks, committed. > > Could you also document this behavior in the "Compiler Characteristics" > section, and make mention of the change in gcc-9/changes.html ? > Yes, done. I also updated the News page in the wiki.
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