Bug 46842

It does not seem to affect all SPEC testers as Rev. 167317 is already quite some old (= 2010-11-30) and I do not have seen any other report. (Cf. for instance http://gcc.gnu.org/ml/gcc-testresults/2010-12/msg00579.html)

I assume it worked before - can you find out which version caused the regression? Or do some other narrowing down of the problem?

Note: To my knowledge none of the gfortran developers has access to SPEC CPU 2006.
Tonto itself is accessible (GPL) from http://www.theochem.uwa.edu.au/tonto/download

(In reply to comment #1)
> It does not seem to affect all SPEC testers as Rev. 167317 is already quite
> some old (= 2010-11-30) and I do not have seen any other report. (Cf. for
> instance http://gcc.gnu.org/ml/gcc-testresults/2010-12/msg00579.html)

This bug is filed against test data set. train and ref data sets are fine.

> I assume it worked before - can you find out which version caused the
> regression? Or do some other narrowing down of the problem?
The last working re I observed is 162788 (very very old).
The first bad rev I observed   is 166096 ( very old)

Could it be a duplicate of PR46506?

Yes, a duplicate. A fortran frontend bug?

*** This bug has been marked as a duplicate of bug 46506 ***

(In reply to comment #4)
> Yes, a duplicate. A fortran frontend bug?

Maybe - however, in order that we could investigate, it would help if you could narrow down the failing commit - without SPEC CPU 2006 access that's impossible. For the revisions you have given one finds
 58 files changed, 13026 insertions(+), 4788 deletions(-)

Reducing.

I can confirm that rev165732 miscompiles tonto and rev162788 is working.
This is at -O0.

*** Bug 46506 has been marked as a duplicate of this bug. ***

This is caused by revision 164494:

http://gcc.gnu.org/ml/gcc-cvs/2010-09/msg00791.html

This patch:

http://gcc.gnu.org/ml/gcc-patches/2010-09/msg01621.html

is the cause, which says

---
With this, the transpose optimization is back.
The two previous patches permitted to call gfc_conv_expr_descriptor on transpose's arg, so now we just have to bypass the temporary generation in the transpose case. 
I don't add the testcase from the original commit as there is a wrong code regression introduced by this patch (uncaught by the testsuite) to be fixed in patch 5/5. 
---

But the patch 5/5 doesn't fix tonto.

I fear that debugging this will be not easy - especially as to my knowledge none of the gfortran developers has access to SPEC CPU 2006.

 * * *

I tried http://sourceforge.net/projects/tonto-chem/files/tonto/2.3/tonto-2.3.1.tar.bz2/download

If I compile it with a version before the regression, namely
  4.6.0 20100909 (experimental) [trunk revision 164046]
I get only failures with "make tests" - I have to recompile with "-fno-sign-zero" to reduce the failures. However, I still get failures (= differences) to the result of the Intel compiler.

I think one could still try to find differences between such a version and a version after the TRANSPOSE committal.

Compiling notice: You need to remove the "-std=f95" and - to speed up the very slow compile a tiny bit - remove also "-Wall -O". Additionally, as mentioned above, consider using "-fno-sign-zero". (In my case, the configuration is in platforms/GNU-gfortran-on-LINUX.) Unfortunately, almost all changes trigger a complete recompilation - and "make -j5" does not work.

 * * *

I could find TRANSPOSEs in the files mo_localiser.F90, molecule.F90, vec{atom}.F90, marchingcube.F90, isosurface.F90, mat{cpx}.F90, mat3{real}.F90, spacegroup.F90, periodic_fmm_level.F90, cluster.F90, mat{real}.F90, run_mat{real}.F90, molecule.main.F90, unit_cell.F90 and crystal.F90. I tried to create some failing test myself, but they were handled properly.

The following is wrongly compiled - however, it is not a regression as it occurs since GCC 4.1. The solution is the same as for gfc_trans_arrayfunc_assign, where the same issue can occur. Cf. PR 44582.

Due to the missing temporary, the array contains 88 88 88 88 instead of 88 1 88 3; the test case works with ifort, NAG and g95.

(There is a slim chance that this is also the problem for Tonto as there was a time window in 4.6 where this optimization was never applied; it should have then also failed with 4.1 to 4.6.)


integer :: a(2,2)
a = reshape([1,2,3,4], [2,2])
call sub(transpose(a))
contains
 subroutine sub(x)
   integer :: x(:,:)
   a(1,:) = 88
   a(2,:) = x(:,1)
   if (    any (a(:,1) /= [88, 1])  &
       .or.any (a(:,2) /= [88, 3])) then
     print *, a
     call abort()
   end if
 end subroutine sub
end

Another reason for the Tonto failure could be PR 45777.

I have now looked through the TRANSPOSEs of Tonto (cf. comment 11) - and I did not see anything which looks as if it could go wrong. I think it would help if someone with access to SPEC CPU 2006 could debug this further - and tell which version of Tonto is included in CPU 2006. I have looked the the source of the latest (2.3.1) - maybe the crucial code is an an earlier version.

The issue of comment 12 does not seem to apply to Tonto - but it should be fixed nevertheless.

crystal.fppized.f90 is micompiled.

(In reply to comment #14)
> crystal.fppized.f90 is micompiled.

If I compare 4.6.0 20100909 with today's GCC build for Tonto 2.3.1's crystal.F90, one sees in the original dump that the only TRANSPOSE relevant change is in the line:

       seitz = transpose(self%spacegroup%seitz(:,:,s))

(Older version: Call to _gfortran_transpose_r8, newer one not.) However, when I try to create an example out of it, it works as it should.

Thanks to Sebastian for some debugging help.

Reduced test case below.

Older gfortrans and other compilers produce (correct):
   4.0000000       4.0000000       6.0000000       6.0000000
The current 4.6 gfortran produces (wrong):
   3.0000000       3.0000000       7.0000000       7.0000000


implicit none
call sub()
contains
   subroutine sub()
      real, dimension(2,2) :: b
      b = 1.0
      b = matmul(b,transpose(func()))
      print *, b
   end subroutine

   function func() result(res)
      real, dimension(2,2) :: res
      res = reshape([1,2,3,4], [2,2])
   end function
end

With the function call - there is no transpose at all:

    // Temporary descriptor
    atmp.10.dim[0].stride = 1;  // Normal bounds: OK
    atmp.10.dim[0].lbound = 0;
    atmp.10.dim[0].ubound = 1;
    atmp.10.dim[1].stride = 2;
    atmp.10.dim[1].lbound = 0;
    atmp.10.dim[1].ubound = 1;
    atmp.10.data = (void * restrict) &A.11;
    atmp.10.offset = 0;
    func (&atmp.10); // Function call - so far OK.
    D.1585 = &atmp.10; // Before this line a TRANSPOSE is missing
    [...]
    _gfortran_matmul_r4 (&atmp.12, D.1580, D.1585, 0, 0, 0B);

As this affects SPEC CPU it should be P1, even though Fortran isn't release
critical (something we might want to change).

(In reply to comment #18)
> Fortran isn't release
> critical (something we might want to change).

yes ... please :-)

Thanks for reducing Tobias. 
I'm looking right now.

Does this fix tonto ?

====================
diff --git a/trans-array.c b/trans-array.c
index 52ba831..e312777 100644
--- a/trans-array.c
+++ b/trans-array.c
@@ -5442,6 +5442,10 @@ gfc_conv_expr_descriptor (gfc_se * se, gfc_expr * expr, gfc_ss * ss)
     }
   else if (expr->expr_type == EXPR_FUNCTION)
     {
+      for (n = 0; n < info->dimen; n++)
+	if (info->dim[n] != n)
+	  goto descriptor;
+      
       desc = info->descriptor;
       se->string_length = ss->string_length;
     }
@@ -5460,6 +5464,7 @@ gfc_conv_expr_descriptor (gfc_se * se, gfc_expr * expr, gfc_ss * ss)
       tree to;
       tree base;
 
+descriptor:
       /* Set the string_length for a character array.  */
       if (expr->ts.type == BT_CHARACTER)
 	se->string_length =  gfc_get_expr_charlen (expr);
====================

PS: patch is against ~ 2 months old trunk

(In reply to comment #21)
> Does this fix tonto ?

The patch fixed the 465.tonto test miscompare when applied to
the current gcc 4.6 trunk!

Thanks,

The patch in comment #21 also fixes the test in comment #16, but not the one in comment #12.

The case in 12 is a different bug and not a regression. How about this variation for this PR and get this one closed?

Index: trans-array.c
===================================================================
--- trans-array.c	(revision 167624)
+++ trans-array.c	(working copy)
@@ -5293,6 +5293,16 @@ get_array_charlen (gfc_expr *expr, gfc_se *se)
     }
 }
 
+/* Helper function to check dimensions.  */
+static bool
+dim_ok (gfc_ss_info *info)
+{
+  int n;
+  for (n = 0; n < info->dimen; n++)
+    if (info->dim[n] != n)
+      return false;
+  return true;
+}
 
 /* Convert an array for passing as an actual argument.  Expressions and
    vector subscripts are evaluated and stored in a temporary, which is then
@@ -5588,7 +5598,7 @@ gfc_conv_expr_descriptor (gfc_se * se, gfc_expr *
 
       desc = loop.temp_ss->data.info.descriptor;
     }
-  else if (expr->expr_type == EXPR_FUNCTION)
+  else if (expr->expr_type == EXPR_FUNCTION && dim_ok (info))
     {
       desc = info->descriptor;
       se->string_length = ss->string_length;

Author: jvdelisle
Date: Sat Dec 11 20:05:20 2010
New Revision: 167713

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=167713
Log:
2010-12-11  Mikael Morin   <mikael@gcc.gnu.org>
	    Jerry DeLisle  <jvdelisle@gcc.gnu.org>

	PR fortran/46842
	* trans-array.c (dim_ok): New helper function.
	(gfc_conv_expr_descriptor): Use new helper function to check
	function array is full.

Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/trans-array.c

Author: jvdelisle
Date: Sat Dec 11 20:09:59 2010
New Revision: 167714

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=167714
Log:
2010-12-11  Jerry DeLisle  <jvdelisle@gcc.gnu.org>

	PR fortran/46842
	* gfortran.dg/array_function_6.f90: New test.

Added:
    trunk/gcc/testsuite/gfortran.dg/array_function_6.f90
Modified:
    trunk/gcc/testsuite/ChangeLog

Fixed, thanks to Mikael concept.

Closing, see PR46896 for the problem identified in comment 12 here.