* [PATCH] Unconditionally use MAX_EXPR/MIN_EXPR for MAX/MIN intrinsics
@ 2018-08-10 20:47 Janne Blomqvist
2018-08-19 16:01 ` Janne Blomqvist
0 siblings, 1 reply; 4+ messages in thread
From: Janne Blomqvist @ 2018-08-10 20:47 UTC (permalink / raw)
To: fortran, gcc-patches; +Cc: Janne Blomqvist
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Unconditionally use MAX_EXPR/MIN_EXPR for MAX/MIN intrinsics
2018-08-10 20:47 [PATCH] Unconditionally use MAX_EXPR/MIN_EXPR for MAX/MIN intrinsics Janne Blomqvist
@ 2018-08-19 16:01 ` Janne Blomqvist
2018-08-19 19:47 ` Thomas Koenig
0 siblings, 1 reply; 4+ messages in thread
From: Janne Blomqvist @ 2018-08-19 16:01 UTC (permalink / raw)
To: Fortran List, GCC Patches; +Cc: Janne Blomqvist
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Unconditionally use MAX_EXPR/MIN_EXPR for MAX/MIN intrinsics
2018-08-19 16:01 ` Janne Blomqvist
@ 2018-08-19 19:47 ` Thomas Koenig
2018-08-22 6:59 ` Janne Blomqvist
0 siblings, 1 reply; 4+ messages in thread
From: Thomas Koenig @ 2018-08-19 19:47 UTC (permalink / raw)
To: Janne Blomqvist, Fortran List, GCC Patches
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Unconditionally use MAX_EXPR/MIN_EXPR for MAX/MIN intrinsics
2018-08-19 19:47 ` Thomas Koenig
@ 2018-08-22 6:59 ` Janne Blomqvist
0 siblings, 0 replies; 4+ messages in thread
From: Janne Blomqvist @ 2018-08-22 6:59 UTC (permalink / raw)
To: Thomas Koenig; +Cc: Fortran List, GCC Patches
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.
--
Janne Blomqvist
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-08-22 6:59 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-10 20:47 [PATCH] Unconditionally use MAX_EXPR/MIN_EXPR for MAX/MIN intrinsics Janne Blomqvist
2018-08-19 16:01 ` Janne Blomqvist
2018-08-19 19:47 ` Thomas Koenig
2018-08-22 6:59 ` Janne Blomqvist
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).