Consider the following program: debian-gfortran:~/test> cat maxval.f90 integer(1) :: i(3) logical :: msk(3) i = -huge(i) i = i - 1 write(*,*) i write(*,*) maxval(i) msk = .false. i = 1 write(*,*) i write(*,*) -huge(i), maxval(i, msk) end In both of these cases, the result of the MAXVAL call should be -huge(i)-1. (For the latter case, see the standard on the definition of the intrinsic, which says that the result is the negative number with the largest magnitude possible within the representation). However, the actual result is this: debian-gfortran:~/test> ../bin-trunk/bin/gfortran maxval.f90 -o maxval debian-gfortran:~/test> ./maxval -128 -128 -128 -127 1 1 1 -127 -127 This error holds for larger integer kinds, as well. The error seems to stem from the libgfortran implementation, in which the initial search value is initialized as -GFC_INTEGER_<n>_HUGE, where it should be one less than that.
Vaguely related is: trans-intrinsic.c's gfc_conv_intrinsic_minmaxval, which contains currently: /* Most negative(-HUGE) for maxval, most positive (-HUGE) for minval. */ if (op == GT_EXPR) tmp = fold_build1 (NEGATE_EXPR, TREE_TYPE (tmp), tmp); Which has a broken comment and also the -HUGE vs -HUGE-1 problem (for BT_INTEGER only). Regarding the library, m4/iparm.m4 contains: define(atype_max, atype_name`_HUGE')dnl define(atype_min, `-'atype_max)dnl Subtracting one from atype_min is the correct thing to do for BT_INTEGER, but not for BT_REAL.
Non-library part of this fix. This fixes the reported bug, but libgfortran/m4/* should be fixed as well. m4/iparm.m4 contains: define(atype_max, atype_name`_HUGE')dnl define(atype_min, `-'atype_max)dnl One would need to put an "ifelse" for atype_min, but I'm rather illiterate in m4. Index: trans-intrinsic.c =================================================================== --- trans-intrinsic.c (Revision 120999) +++ trans-intrinsic.c (Arbeitskopie) @@ -1976,11 +1976,16 @@ gcc_unreachable (); } - /* Most negative(+HUGE) for maxval, most negative (-HUGE) for minval. */ + /* Most negative(-HUGE) for maxloc, most positiv (+HUGE) for minloc. */ if (op == GT_EXPR) tmp = fold_build1 (NEGATE_EXPR, TREE_TYPE (tmp), tmp); gfc_add_modify_expr (&se->pre, limit, tmp); + /* Most negative for BT_INTEGER is -HUGE-1. */ + if (op == GT_EXPR && expr->ts.type == BT_INTEGER) + tmp = build2 (MINUS_EXPR, TREE_TYPE (tmp), tmp, + build_int_cst (type, 1)); + /* Initialize the scalarizer. */ gfc_init_loopinfo (&loop); gfc_add_ss_to_loop (&loop, arrayss); @@ -2135,9 +2140,15 @@ gcc_unreachable (); } - /* Most negative(-HUGE) for maxval, most positive (-HUGE) for minval. */ + /* Most negative(-HUGE) for maxval, most positive (+HUGE) for minval. */ if (op == GT_EXPR) tmp = fold_build1 (NEGATE_EXPR, TREE_TYPE (tmp), tmp); + + /* Most negative for BT_INTEGER is -HUGE-1. */ + if (op == GT_EXPR && expr->ts.type == BT_INTEGER) + tmp = build2 (MINUS_EXPR, TREE_TYPE (tmp), tmp, + build_int_cst (type, 1)); + gfc_add_modify_expr (&se->pre, limit, tmp); /* Walk the arguments. */
I don't think it's a bug, since "the negative number with the largest magnitude possible within the representation" is not -huge()-1, but -huge(). If I understand the standard correctly, -huge()-1, although being representible by the hardware you have, is not "within the representation" of this integer kind, because the range of the representation of an integer kind is supposed to be symmetric. For what it's worth, the Intel and Sun compilers have the behaviour you expect, but the Portland compiler and g95 both have the same behaviour as gfortran. Steve might have an idea on that, IIRC he's the one who implemented the -frange-check option. Otherwise, a question to comp.lang.fortran would be a good thing.
> For what it's worth, the Intel and Sun compilers have the behaviour you > expect, but the Portland compiler and g95 both have the same behaviour as > gfortran. NAG f95 and pathscale 2.4 have: -128. > If I understand the standard correctly, -huge()-1, although being > representible by the hardware you have, is not "within the representation" > of this integer kind, because the range of the representation of an integer > kind is supposed to > be symmetric. Might be, but on the otherhand, if I have the following program: integer(1) :: a(2) a = -128 print *, maxval(a) I would expect that it prints "-128" and not "-127".
Subject: Re: MAXVAL() incorrect for zero-size int arrays, and for -HUGE-1 maximum values. > >- Comment #3 from fxcoudert at gcc dot gnu dot org 2007-01-22 07:56 ------- > I don't think it's a bug, since "the negative number with the largest > magnitude possible within the representation" is not -huge()-1, but -huge(). > > If I understand the standard correctly, -huge()-1, although being > representible by the hardware you have, is not "within the representation" > of this integer kind, because the range of the representation of an > integer kind is supposed to be symmetric. > > For what it's worth, the Intel and Sun compilers have the behaviour > you expect, but the Portland compiler and g95 both have the same > behaviour as gfortran. > > Steve might have an idea on that, IIRC he's the one who implemented the > -frange-check option. Otherwise, a question to comp.lang.fortran would be a > good thing. > I fixed the range checking from [-huge()-1: 2*huge()] to [-huge()-1: huge()]. The old upper limited would allow people to write integer(1) i i = -128_1 ! This should be -huge(i) - 1_1 end because the 128_1 was not flagged as overflowing a integer(1). Note, -128_1 is a unary minus operating on the 128_1. When I changed the range checking or shortly after, jerryd introduced the -f[no]-range-check option. For the case at hand, the integers do not need to be symmetric, and so minval and maxval should properly handle -huge()-1. As pointed out by Tobias, integer(1) a(2) a = -huge(a) - 1 print *, minval(a), maxval(a) end should print "-128 -128" Now, where the symmetry of the integers becomes a problem is integer(1) i i = - huge() - 1 i = abs(i) ! This is invalid. end The above is prohibited in 13.14: The types and type parameters of intrinsic procedure arguments and function results are determined by these specifications. 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.
Here's a patch for the m4 part: Index: m4/iparm.m4 =================================================================== --- m4/iparm.m4 (revision 121230) +++ m4/iparm.m4 (working copy) @@ -28,6 +28,6 @@ define_type(rtype, rtype_tmp)dnl define(rtype_qual,`_'rtype_kind)dnl ')dnl define(atype_max, atype_name`_HUGE')dnl -define(atype_min, `-'atype_max)dnl +define(atype_min,ifelse(rtype_letter,`i',`(-'atype_max`-1)',`-'atype_max))dnl define(name, regexp(regexp(file, `[^/]*$', `\&'), `^\([^_]*\)_', `\1'))dnl define(rtype_ccode,ifelse(rtype_letter,`i',rtype_kind,rtype_code))dnl
Should we commit the combined fix? I do think this is a bug. Thomas
> Should we commit the combined fix? I do think this > is a bug. So do I, but we also need a test case, I think.
Let's then fix this bug.
Subject: Bug number PR30512 A patch for this bug has been added to the patch tracker. The mailing list url for the patch is http://gcc.gnu.org/ml/gcc-patches/2007-02/msg00023.html
Subject: Bug 30512 Author: burnus Date: Fri Feb 9 21:56:06 2007 New Revision: 121777 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=121777 Log: fortran/ 2007-02-09 Tobias Burnus <burnus@net-b.de> PR fortran/30512 * trans-intrinsic.c (gfc_conv_intrinsic_minmaxloc, gfc_conv_intrinsic_minmaxval): Use HUGE-1 for most negative integer. testsuite/ 2007-02-09 Tobias Burnus <burnus@net-b.de> PR fortran/30512 * gfortran.dg/maxlocval_1.f90: New test. libgfortran/ 2007-02-09 Thomas Koenig <Thomas.Koenig@online.de> Tobias Burnus <burnus@net-b.de> PR fortran/30512 * m4/iparm.m4: Use HUGE-1 for most negative integer. * generated/maxloc1_8_i4.c: Regenerate. * generated/maxloc0_8_i8.c: Regenerate. * generated/maxloc1_16_i4.c: Regenerate. * generated/maxloc0_16_i8.c: Regenerate. * generated/maxval_i4.c: Regenerate. * generated/maxloc1_4_i8.c: Regenerate. * generated/maxloc0_16_i16.c: Regenerate. * generated/maxloc1_4_i16.c: Regenerate. * generated/maxloc0_8_i16.c: Regenerate. * generated/maxloc0_4_i4.c: Regenerate. * generated/maxloc1_8_i8.c: Regenerate. * generated/maxloc0_8_i4.c: Regenerate. * generated/maxloc0_16_i4.c: Regenerate. * generated/maxloc1_16_i8.c: Regenerate. * generated/maxloc1_4_i4.c: Regenerate. * generated/maxval_i8.c: Regenerate. * generated/maxloc0_4_i16.c: Regenerate. * generated/maxloc1_8_i16.c: Regenerate. * generated/maxloc0_4_i8.c: Regenerate. * generated/maxloc1_16_i16.c: Regenerate. * generated/maxval_i16.c: Regenerate. Added: trunk/gcc/testsuite/gfortran.dg/maxlocval_1.f90 Modified: trunk/gcc/fortran/ChangeLog trunk/gcc/fortran/trans-intrinsic.c trunk/gcc/testsuite/ChangeLog trunk/libgfortran/ChangeLog trunk/libgfortran/generated/maxloc0_16_i16.c trunk/libgfortran/generated/maxloc0_16_i4.c trunk/libgfortran/generated/maxloc0_16_i8.c trunk/libgfortran/generated/maxloc0_4_i16.c trunk/libgfortran/generated/maxloc0_4_i4.c trunk/libgfortran/generated/maxloc0_4_i8.c trunk/libgfortran/generated/maxloc0_8_i16.c trunk/libgfortran/generated/maxloc0_8_i4.c trunk/libgfortran/generated/maxloc0_8_i8.c trunk/libgfortran/generated/maxloc1_16_i16.c trunk/libgfortran/generated/maxloc1_16_i4.c trunk/libgfortran/generated/maxloc1_16_i8.c trunk/libgfortran/generated/maxloc1_4_i16.c trunk/libgfortran/generated/maxloc1_4_i4.c trunk/libgfortran/generated/maxloc1_4_i8.c trunk/libgfortran/generated/maxloc1_8_i16.c trunk/libgfortran/generated/maxloc1_8_i4.c trunk/libgfortran/generated/maxloc1_8_i8.c trunk/libgfortran/generated/maxval_i16.c trunk/libgfortran/generated/maxval_i4.c trunk/libgfortran/generated/maxval_i8.c trunk/libgfortran/m4/iparm.m4
Subject: Bug 30512 Author: burnus Date: Fri Feb 16 14:15:36 2007 New Revision: 122043 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=122043 Log: fortran/ 2007-02-16 Tobias Burnus <burnus@net-b.de> PR fortran/30512 * trans-intrinsic.c (gfc_conv_intrinsic_minmaxloc, gfc_conv_intrinsic_minmaxval): Use HUGE-1 for most negative integer. testsuite/ 2007-02-16 Tobias Burnus <burnus@net-b.de> PR fortran/30512 * gfortran.dg/maxlocval_1.f90: New test. libgfortran/ 2007-02-16 Thomas Koenig <Thomas.Koenig@online.de> Tobias Burnus <burnus@net-b.de> PR fortran/30512 * m4/iparm.m4: Use HUGE-1 for most negative integer. * generated/maxloc1_8_i4.c: Regenerate. * generated/maxloc0_8_i8.c: Regenerate. * generated/maxloc1_16_i4.c: Regenerate. * generated/maxloc0_16_i8.c: Regenerate. * generated/maxval_i4.c: Regenerate. * generated/maxloc1_4_i8.c: Regenerate. * generated/maxloc0_16_i16.c: Regenerate. * generated/maxloc1_4_i16.c: Regenerate. * generated/maxloc0_8_i16.c: Regenerate. * generated/maxloc0_4_i4.c: Regenerate. * generated/maxloc1_8_i8.c: Regenerate. * generated/maxloc0_8_i4.c: Regenerate. * generated/maxloc0_16_i4.c: Regenerate. * generated/maxloc1_16_i8.c: Regenerate. * generated/maxloc1_4_i4.c: Regenerate. * generated/maxval_i8.c: Regenerate. * generated/maxloc0_4_i16.c: Regenerate. * generated/maxloc1_8_i16.c: Regenerate. * generated/maxloc0_4_i8.c: Regenerate. * generated/maxloc1_16_i16.c: Regenerate. * generated/maxval_i16.c: Regenerate. Added: branches/gcc-4_2-branch/gcc/testsuite/gfortran.dg/maxlocval.f90 Modified: branches/gcc-4_2-branch/gcc/fortran/ChangeLog branches/gcc-4_2-branch/gcc/fortran/trans-intrinsic.c branches/gcc-4_2-branch/gcc/testsuite/ChangeLog branches/gcc-4_2-branch/libgfortran/ChangeLog branches/gcc-4_2-branch/libgfortran/generated/maxloc0_16_i16.c branches/gcc-4_2-branch/libgfortran/generated/maxloc0_16_i4.c branches/gcc-4_2-branch/libgfortran/generated/maxloc0_16_i8.c branches/gcc-4_2-branch/libgfortran/generated/maxloc0_4_i16.c branches/gcc-4_2-branch/libgfortran/generated/maxloc0_4_i4.c branches/gcc-4_2-branch/libgfortran/generated/maxloc0_4_i8.c branches/gcc-4_2-branch/libgfortran/generated/maxloc0_8_i16.c branches/gcc-4_2-branch/libgfortran/generated/maxloc0_8_i4.c branches/gcc-4_2-branch/libgfortran/generated/maxloc0_8_i8.c branches/gcc-4_2-branch/libgfortran/generated/maxloc1_16_i16.c branches/gcc-4_2-branch/libgfortran/generated/maxloc1_16_i4.c branches/gcc-4_2-branch/libgfortran/generated/maxloc1_16_i8.c branches/gcc-4_2-branch/libgfortran/generated/maxloc1_4_i16.c branches/gcc-4_2-branch/libgfortran/generated/maxloc1_4_i4.c branches/gcc-4_2-branch/libgfortran/generated/maxloc1_4_i8.c branches/gcc-4_2-branch/libgfortran/generated/maxloc1_8_i16.c branches/gcc-4_2-branch/libgfortran/generated/maxloc1_8_i4.c branches/gcc-4_2-branch/libgfortran/generated/maxloc1_8_i8.c branches/gcc-4_2-branch/libgfortran/generated/maxval_i16.c branches/gcc-4_2-branch/libgfortran/generated/maxval_i4.c branches/gcc-4_2-branch/libgfortran/generated/maxval_i8.c branches/gcc-4_2-branch/libgfortran/m4/iparm.m4
Hi Tobias, should we close this?
(In reply to comment #13) > should we close this? We can close it as I think it is not worth to backport it to 4.1.