Bug 32048 - max/min and NaN
Summary: max/min and NaN
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: fortran (show other bugs)
Version: 4.3.0
: P3 enhancement
Target Milestone: 4.3.0
Assignee: Francois-Xavier Coudert
URL:
Keywords: wrong-code
Depends on:
Blocks: 30694
  Show dependency treegraph
 
Reported: 2007-05-22 20:57 UTC by Thomas Koenig
Modified: 2007-07-28 15:23 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2007-07-28 13:50:30


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Thomas Koenig 2007-05-22 20:57:51 UTC
The test program should consistently return
either 2.1 or NaN:

$ cat foo.f90
program main
  real :: nan
  nan = 0.0
  nan = nan / nan
  print *,max(2.1, nan)
  print *,max(nan, 2.1)
end program main
$ gfortran foo.f90
$ ./a.out
   2.100000
            NaN

A lot of compilers seem to get this wrong, as well.
Marking it as an enhancement.
Comment 1 Andrew Pinski 2007-05-22 22:07:13 UTC
NaN is not defined in Fortran's real so I am thinking about closing this as invalid.  Only the IEEE types in Fortran 2003 is where NaN are defined.
Comment 2 Daniel Franke 2007-05-22 22:41:43 UTC
Regardless whether NaN is defined, in an ordered set MAX(a, b) should always equal MAX(b, a).

For the record:
$> ifort pr32048.f90 && ./a.out
   2.100000
   2.100000

$> sunf95 -w4 pr32048.f90 && ./a.out
Floating point exception
Comment 3 Andrew Pinski 2007-05-22 22:50:47 UTC
And NaN is unordered which causes the results to become undefined unless you have a defined value for min/max in the standard (fmin/fmax in C99 have a defined value of having NaN being a missing value).
Comment 4 Tobias Burnus 2007-05-22 23:02:07 UTC
Fortran has no concept of NaN except in Fortran 2003 IEEE module. To say it in the words of Dick Hendrickson (cf. PR 30694): "A program is prohibited from invoking an intrinsic procedure under circumstances where a value to be returned in a subroutine argument or function result is outside the range of values representable by objects of the specified type and type parameters, unless the intrinsic module IEEE_ARITHMETIC (section 14) is accessible and there is support
for an infinite or a NaN result, as appropriate."

If NaN is supported, then the compiler should follow IEEE which defines:
http://754r.ucbtest.org/drafts/754r.pdf

"minNum(x,y) is the canonical floating-point number x if x < y, y if y < x, the
canonicalized floating-point number if one operand is a floating-point number
and the other a NaN. Otherwise it is either x or y.
maxNum(x,y) is the canonical floating-point number y if x < y, x if y < x, the
canonicalized floating-point number if one operand is a floating-point number
and the other a NaN. Otherwise it is either x or y."

Thus the example program should be 2.100000 in both cases.

Other compilers:
- g95: NaN, NaN -> not IEEE conform
- NAG f95: NaN, 2.0999999 -> not consistent, not IEEE conform
- gfortran: 2.100000, NaN -> not consistent, not IEEE conform
- ifort 9.1: NaN, 2.100000 -> not consistent, not IEEE conform
- sunf95: -NaN, 2.1 -> not consistent, not IEEE conform

> And NaN is unordered which causes the results to become undefined unless you
> have a defined value for min/max in the standard
Fortran has - albeit only indirectly via the IEEE support.
Comment 5 Richard Biener 2007-05-23 11:21:15 UTC
Of course people are complaining that min/max as of IEEE does not propagate NaNs
as other operations do.
Comment 6 Thomas Koenig 2007-05-25 20:36:40 UTC
(In reply to comment #5)
> Of course people are complaining that min/max as of IEEE does not propagate
> NaNs
> as other operations do.

We could add flags like -fpropagate-nan, -fno-propagate-nan
and -fdontcare-propagate-nan (no, I'm not serious
about the last option).

Comment 7 Francois-Xavier Coudert 2007-07-28 13:49:41 UTC
Here's a patch that yields results according to IEEE:

Index: f95-lang.c
===================================================================
--- f95-lang.c  (revision 126997)
+++ f95-lang.c  (working copy)
@@ -1004,6 +1004,11 @@ gfc_init_builtin_functions (void)
                      "malloc", false);
   DECL_IS_MALLOC (built_in_decls[BUILT_IN_MALLOC]) = 1;
 
+  tmp = tree_cons (NULL_TREE, void_type_node, void_list_node);
+  ftype = build_function_type (integer_type_node, tmp);
+  gfc_define_builtin ("__builtin_isnan", ftype, BUILT_IN_ISNAN,
+                     "__builtin_isnan", true);
+
 #define DEF_PRIMITIVE_TYPE(ENUM, VALUE) \
   builtin_types[(int) ENUM] = VALUE;
 #define DEF_FUNCTION_TYPE_0(ENUM, RETURN)              \
Index: trans-intrinsic.c
===================================================================
--- trans-intrinsic.c   (revision 126997)
+++ trans-intrinsic.c   (working copy)
@@ -1407,11 +1407,11 @@ gfc_conv_intrinsic_ttynam (gfc_se * se, 
 /* Get the minimum/maximum value of all the parameters.
     minmax (a1, a2, a3, ...)
     {
-      if (a2 .op. a1)
+      if (a2 .op. a1 || isnan(a1))
         mvar = a2;
       else
         mvar = a1;
-      if (a3 .op. mvar)
+      if (a3 .op. mvar || isnan(mvar))
         mvar = a3;
       ...
       return mvar
@@ -1487,7 +1487,7 @@ gfc_conv_intrinsic_minmax (gfc_se * se, 
   elsecase = build2_v (MODIFY_EXPR, mvar, limit);
   for (i = 1, argexpr = argexpr->next; i < nargs; i++)
     {
-      tree cond;
+      tree cond, isnan;
 
       val = args[i]; 
 
@@ -1509,6 +1509,11 @@ gfc_conv_intrinsic_minmax (gfc_se * se, 
       thencase = build2_v (MODIFY_EXPR, mvar, convert (type, val));
 
       tmp = build2 (op, boolean_type_node, convert (type, val), limit);
+      if (FLOAT_TYPE_P (TREE_TYPE (limit)))
+       {
+         isnan = build_call_expr (built_in_decls[BUILT_IN_ISNAN], 1, limit);
+         tmp = fold_build2 (TRUTH_OR_EXPR, boolean_type_node, tmp, isnan);
+       }
       tmp = build3_v (COND_EXPR, tmp, thencase, elsecase);
 
       if (cond != NULL_TREE)


With this, we generate calls to __builtin_isnan, which probably means slower code is generated. Thus, we might want to make this dependent on whether IEEE_ARITHMETIC is loaded or not, when we have this module implemented.
Comment 8 Francois-Xavier Coudert 2007-07-28 15:23:22 UTC
Subject: Bug 32048

Author: fxcoudert
Date: Sat Jul 28 15:23:11 2007
New Revision: 127019

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=127019
Log:
	PR fortran/32048

	* f95-lang.c (gfc_init_builtin_functions): Add declaration for
	__builtin_isnan.
	* trans-intrinsic.c (gfc_conv_intrinsic_minmax): Handled NaNs.

	* gfortran.dg/nan_1.f90: New test.

Added:
    trunk/gcc/testsuite/gfortran.dg/nan_1.f90
Modified:
    trunk/gcc/fortran/f95-lang.c
    trunk/gcc/fortran/trans-intrinsic.c

Comment 9 Francois-Xavier Coudert 2007-07-28 15:23:53 UTC
Fixed.