[Bug libfortran/31726] New: minloc/minval: wrong results with empty array

[Bug libfortran/31726] New: minloc/minval: wrong results with empty array

[dfranke at gcc dot gnu dot org]

$> cat minloc.f90
PRINT *, MINLOC((/ 42, 23 /), DIM=1, MASK=.FALSE.)
PRINT *, MINVAL((/ 42, 23 /), DIM=1, MASK=.FALSE.)
END

$> gfotran-4.2 -g -Wall minloc.f90 && ./a.out
           1
  2147483647

F2003, 13.7.78, MINLOC, Result Value:
"Case (ii) The result of MINLOC (ARRAY, MASK = MASK) is a rank-one array
[...]
If ARRAY has size zero or every element of MASK has the value false, all
elements of the result are zero."

"Case (iii) If ARRAY has rank one, MINLOC (ARRAY, DIM = DIM [, MASK =
MASK]) is a scalar whose value is equal to that of the first element of
MINLOC (ARRAY [, MASK = MASK])."


Thus, the result for MINLOC in above code should be zero. I'm not sure about
MINVAL. The same problem exists with MAXLOC/MAXVAL.


-- 
           Summary: minloc/minval: wrong results with empty array
           Product: gcc
           Version: 4.3.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libfortran
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: dfranke at gcc dot gnu dot org


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31726

[Bug libfortran/31726] minloc/maxloc: wrong results with empty array

[dfranke at gcc dot gnu dot org]

------- Comment #1 from dfranke at gcc dot gnu dot org  2007-04-27 11:22 -------
MINVAL/MAXVAL are ok.

F2003, 13.7.79, MINVAL, Result Value:
Case (i): [...] If ARRAY has size zero and type integer or real, the result 
          has the value of the positive number of the largest magnitude
          supported by the processor for numbers of the type and kind type
          parameter of ARRAY. If ARRAY has size zero and type character,


-- 

dfranke at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to fail|                            |4.1.1 4.2.0 4.3.0
            Summary|minloc/minval: wrong results|minloc/maxloc: wrong results
                   |with empty array            |with empty array


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31726

[Bug libfortran/31726] minloc/maxloc: wrong results with empty array (F2003 only)

[burnus at gcc dot gnu dot org]

------- Comment #2 from burnus at gcc dot gnu dot org  2007-04-27 13:16 -------
The problem is:
  if(0) { /* find index */
  else
        {
          pos.1 = 0;
        }
      D.1356 = (int4) ((<unnamed-unsigned:32>) pos.1 + 1);

As C and the internal tree have array indexes starting at one, the final index
has to be incremented by one. Thus "pos.1 = 0" is ok, but "+1" is only correct
if one has an index. The simplest would be to have "pos.1 = -1". Then one needs
to change trans-intrinsic.c's gfc_conv_intrinsic_minmaxloc:

  /* Initialize the position to zero, following Fortran 2003.  We are free
     to do this because Fortran 95 allows the result of an entirely false
     mask to be processor dependent.  */
  gfc_add_modify_expr (&loop.pre, pos, gfc_index_zero_node);

One should also check the library version of MIN/MAXLOC.


-- 

burnus at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1
           Keywords|                            |wrong-code
   Last reconfirmed|0000-00-00 00:00:00         |2007-04-27 13:16:34
               date|                            |
            Summary|minloc/maxloc: wrong results|minloc/maxloc: wrong results
                   |with empty array            |with empty array (F2003
                   |                            |only)


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31726

[Bug libfortran/31726] minloc/maxloc: wrong results with empty array (F2003 only)

[burnus at gcc dot gnu dot org]

------- Comment #3 from burnus at gcc dot gnu dot org  2007-04-27 15:59 -------
Patch. Dump looks ok. Needs some testcases plus checking of the library
version.

Index: trans-intrinsic.c
===================================================================
--- trans-intrinsic.c   (Revision 124216)
+++ trans-intrinsic.c   (Arbeitskopie)
@@ -2007,10 +2007,10 @@ gfc_conv_intrinsic_minmaxloc (gfc_se * s

   gcc_assert (loop.dimen == 1);

-  /* Initialize the position to zero, following Fortran 2003.  We are free
-     to do this because Fortran 95 allows the result of an entirely false
-     mask to be processor dependent.  */
-  gfc_add_modify_expr (&loop.pre, pos, gfc_index_zero_node);
+  /* For zero-sized arrays or if the mask is .false. for every element,
+     zero needs to be returned according to the Fortran 2003 standard.
+     As this number is always incremented by one, it is initialized to -1.  */
+  gfc_add_modify_expr (&loop.pre, pos, build_int_cst (TREE_TYPE(pos), -1));

   gfc_mark_ss_chain_used (arrayss, 1);
   if (maskss)
@@ -2084,7 +2084,7 @@ gfc_conv_intrinsic_minmaxloc (gfc_se * s
         the pos variable the same way as above.  */

       gfc_init_block (&elseblock);
-      gfc_add_modify_expr (&elseblock, pos, gfc_index_zero_node);
+      gfc_add_modify_expr (&elseblock, pos, build_int_cst (TREE_TYPE(pos),
-1));
       elsetmp = gfc_finish_block (&elseblock);

       tmp = build3_v (COND_EXPR, maskse.expr, tmp, elsetmp);


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31726

[Bug libfortran/31726] minloc/maxloc: wrong results with empty array (F2003 only)

[burnus at gcc dot gnu dot org]

------- Comment #4 from burnus at gcc dot gnu dot org  2007-04-27 17:42 -------
Created an attachment (id=13456)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=13456&action=view)
Test case

Writing test cases first helps!
I need go back to the drawing board as the pos = pos + 1 is not added for empty
arrays.

The attached test case works with NAG f95, g95 and sunf95; it fails 24 times
("1" should be "0") with ifort and 18 times with unmodified gfortran (0 not 1
plus other bugs).
It does not test the library function though.

The test case shows however some additional problems where
 pos = pos.1 - (from - 1)  ! from = starting index of the array
does not work correctly (e.g. "3" instead of "2")


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31726

[Bug libfortran/31726] minloc/maxloc: wrong results with empty array (F2003 only)

[tkoenig at gcc dot gnu dot org]

------- Comment #5 from tkoenig at gcc dot gnu dot org  2007-04-27 20:16 -------

> One should also check the library version of MIN/MAXLOC.

I'll do that.  This is an area that I am working on anyway
(PR 30694).

Thanks for providing the test case, btw.  It is quite
thorough :-)


-- 

tkoenig at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
OtherBugsDependingO|                            |30694
              nThis|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31726

[Bug libfortran/31726] minloc/maxloc: wrong results with empty array (F2003 only)

[tkoenig at gcc dot gnu dot org]

------- Comment #6 from tkoenig at gcc dot gnu dot org  2007-04-27 21:00 -------
Created an attachment (id=13457)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=13457&action=view)
Test case for the library functions

The library function appears to be ok.  Here is the test
case (semi-automatically generated from Tobias' patch :-)


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31726

[Bug libfortran/31726] minloc/maxloc: wrong results with empty array (F2003 only)

[pault at gcc dot gnu dot org]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 3357 bytes --]



------- Comment #7 from pault at gcc dot gnu dot org  2007-06-21 18:14 -------
This fixes Tobias' testcase of comment #4.  Note that it is a diff relative to
the patch for 32298.  I'll resubmit the latter in this new form.

Paul

Index: gcc/fortran/trans-intrinsic.c
===================================================================
*** gcc/fortran/trans-intrinsic.c       (révision 125706)
--- gcc/fortran/trans-intrinsic.c       (copie de travail)
*************** gfc_conv_intrinsic_minmaxloc (gfc_se * s
*** 1928,1933 ****
--- 1928,1934 ----
    tree tmp;
    tree elsetmp;
    tree ifbody;
+   tree offset;
    gfc_loopinfo loop;
    gfc_actual_arglist *actual;
    gfc_ss *arrayss;
*************** gfc_conv_intrinsic_minmaxloc (gfc_se * s
*** 2045,2059 ****
    /* Assign the value to the limit...  */
    gfc_add_modify_expr (&ifblock, limit, arrayse.expr);

!   /* Remember where we are.  */
!   gfc_add_modify_expr (&ifblock, pos, loop.loopvar[0]);

    ifbody = gfc_finish_block (&ifblock);

!   /* If it is a more extreme value or pos is still zero.  */
!   tmp = build2 (TRUTH_OR_EXPR, boolean_type_node,
!                 build2 (op, boolean_type_node, arrayse.expr, limit),
!                 build2 (EQ_EXPR, boolean_type_node, pos,
gfc_index_zero_node));
    tmp = build3_v (COND_EXPR, tmp, ifbody, build_empty_stmt ());
    gfc_add_expr_to_block (&block, tmp);

--- 2046,2068 ----
    /* Assign the value to the limit...  */
    gfc_add_modify_expr (&ifblock, limit, arrayse.expr);

!   /* Remember where we are.  An offset must be added to the loop
!      counter to obtain the required position.  */
!   if (loop.temp_dim)
!     offset = build_int_cst (type, 1);
!   else
!     offset =fold_build2 (MINUS_EXPR, gfc_array_index_type,
!                        gfc_index_one_node, loop.from[0]);
!   offset = gfc_evaluate_now (offset, &block);
! 
!   tmp = build2 (PLUS_EXPR, TREE_TYPE (pos),
!               loop.loopvar[0], offset);
!   gfc_add_modify_expr (&ifblock, pos, tmp);

    ifbody = gfc_finish_block (&ifblock);

!   /* If it is a more extreme value.  */
!   tmp = build2 (op, boolean_type_node, arrayse.expr, limit);
    tmp = build3_v (COND_EXPR, tmp, ifbody, build_empty_stmt ());
    gfc_add_expr_to_block (&block, tmp);

*************** gfc_conv_intrinsic_minmaxloc (gfc_se * s
*** 2098,2109 ****
      }
    gfc_cleanup_loop (&loop);

!   /* Return a value in the range 1..SIZE(array).  */
!   tmp = fold_build2 (MINUS_EXPR, gfc_array_index_type, loop.from[0],
!                    gfc_index_one_node);
!   tmp = fold_build2 (MINUS_EXPR, gfc_array_index_type, pos, tmp);
!   /* And convert to the required type.  */
!   se->expr = convert (type, tmp);
  }

  static void
--- 2107,2113 ----
      }
    gfc_cleanup_loop (&loop);

!   se->expr = convert (type, pos);
  }

  static void


-- 

pault at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |pault at gcc dot gnu dot org
                   |dot org                     |
             Status|NEW                         |ASSIGNED
   Last reconfirmed|2007-04-27 13:16:34         |2007-06-21 18:14:41
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31726

[Bug libfortran/31726] minloc/maxloc: wrong results with empty array (F2003 only)

[patchapp at dberlin dot org]

------- Comment #8 from patchapp at dberlin dot org  2007-06-22 02:05 -------
Subject: Bug number PR31726

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-06/msg01583.html


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31726

[Bug libfortran/31726] minloc/maxloc: wrong results with empty array (F2003 only)

[pault at gcc dot gnu dot org]

------- Comment #9 from pault at gcc dot gnu dot org  2007-06-24 11:04 -------
Subject: Bug 31726

Author: pault
Date: Sun Jun 24 11:04:02 2007
New Revision: 125983

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=125983
Log:
2007-06-24  Paul Thomas  <pault@gcc.gnu.org>

        PR fortran/32298
        PR fortran/31726
        * trans-intrinsic.c (gfc_conv_intrinsic_minmaxloc): Calculate
        the offset between the loop counter and the position as
        defined. Add the offset within the loop so that the mask acts
        correctly.  Do not advance the location on the basis that it
        is zero.

2007-06-24  Paul Thomas  <pault@gcc.gnu.org>

        PR fortran/31726
        * gfortran.dg/minmaxloc_1.f90: New test.

        PR fortran/32298
        * gfortran.dg/minmaxloc_2.f90: New test.


Added:
    trunk/gcc/testsuite/gfortran.dg/minmaxloc_1.f90
    trunk/gcc/testsuite/gfortran.dg/minmaxloc_2.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/trans-intrinsic.c
    trunk/gcc/testsuite/ChangeLog


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31726

[Bug libfortran/31726] minloc/maxloc: wrong results with empty array (F2003 only)

[pault at gcc dot gnu dot org]

------- Comment #10 from pault at gcc dot gnu dot org  2007-06-24 11:05 -------
Fixed on trunk

Paul


-- 

pault at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31726

[Bug libfortran/31726] minloc/maxloc: wrong results with empty array (F2003 only)

[tkoenig at gcc dot gnu dot org]

------- Comment #11 from tkoenig at gcc dot gnu dot org  2007-10-15 18:23 -------
Subject: Bug 31726

Author: tkoenig
Date: Mon Oct 15 18:23:39 2007
New Revision: 129365

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=129365
Log:
2007-10-25  Thomas Koenig  <tkoenig@gcc.gnu.org>
            Paul Thomas  <pault@gcc.gnu.org>

        PR fortran/32298
        PR fortran/31726
        PR fortran/33354
        Backport from trunk
        * trans-intrinsic.c (gfc_conv_intrinsic_minmaxloc): Calculate
        the offset between the loop counter and the position as
        defined. Add the offset within the loop so that the mask acts
        correctly.  Do not advance the location on the basis that it
        is zero.

2007-10-15  Thomas Koenig  <tkoenig@gcc.gnu.org>

        PR fortran/33354
        * gfortran.dg/minmaxloc_4.f90:  New test.



Added:
    branches/gcc-4_2-branch/gcc/testsuite/gfortran.dg/minmaxloc_4.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


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31726