[Patch, fortran] PR48955 [4.6/4.7 Regression] Wrong result for array assignment due to missing temporary

[Patch, fortran] PR48955 [4.6/4.7 Regression] Wrong result for array assignment due to missing temporary

[Paul Richard Thomas]

[-- Attachment #1: Type: text/plain, Size: 760 bytes --]

This patch fixes the problem in two steps:
(i) It reverts r162289; and
(ii) It adds the correct initialization for loop.reverse[] in
gfc_trans_assignment_1.  This was implemented incorrectly in the fix
for PR24524 (in spite of the correct comment in dependency.c!) and
removed at sometime, I do not know why.

Bootstraps and regtests on x86_64/FC9.  OK for trunk and 4.6?

Paul

2011-05-12  Paul Thomas  <pault@gcc.gnu.org>

	PR fortran/48955
	* dependency.c (gfc_dep_resolver): Revert r162829 which changed
	the condition for setting this_dep to GFC_DEP_OVERLAP.
	* trans-expr.c (gfc_trans_assignment_1): Enable loop reversal.

2011-05-12  Paul Thomas  <pault@gcc.gnu.org>

	PR fortran/48955
	* gfortran.dg/dependency_40.f90 : New test.

[-- Attachment #2: submit.diff --]
[-- Type: text/x-patch, Size: 2328 bytes --]

Index: gcc/fortran/trans-expr.c
===================================================================
*** gcc/fortran/trans-expr.c	(revision 173649)
--- gcc/fortran/trans-expr.c	(working copy)
*************** gfc_trans_assignment_1 (gfc_expr * expr1
*** 6052,6057 ****
--- 6052,6061 ----
        /* Initialize the scalarizer.  */
        gfc_init_loopinfo (&loop);
  
+       /* Enable loop reversal.  */
+       for (n = 0; n < GFC_MAX_DIMENSIONS; n++)
+ 	loop.reverse[n] = GFC_CAN_REVERSE;
+ 
        /* Walk the rhs.  */
        rss = gfc_walk_expr (expr2);
        if (rss == gfc_ss_terminator)
Index: gcc/fortran/dependency.c
===================================================================
*** gcc/fortran/dependency.c	(revision 173649)
--- gcc/fortran/dependency.c	(working copy)
*************** gfc_dep_resolver (gfc_ref *lref, gfc_ref
*** 1832,1839 ****
  
  		  /* If no intention of reversing or reversing is explicitly
  		     inhibited, convert backward dependence to overlap.  */
! 		  if (this_dep == GFC_DEP_BACKWARD
! 		      && (reverse == NULL || reverse[n] == GFC_CANNOT_REVERSE))
  		    this_dep = GFC_DEP_OVERLAP;
  		}
  
--- 1832,1839 ----
  
  		  /* If no intention of reversing or reversing is explicitly
  		     inhibited, convert backward dependence to overlap.  */
! 		  if ((reverse == NULL && this_dep == GFC_DEP_BACKWARD)
! 		      || (reverse != NULL && reverse[n] == GFC_CANNOT_REVERSE))
  		    this_dep = GFC_DEP_OVERLAP;
  		}
  
Index: gcc/testsuite/gfortran.dg/dependency_40.f90
===================================================================
*** gcc/testsuite/gfortran.dg/dependency_40.f90	(revision 0)
--- gcc/testsuite/gfortran.dg/dependency_40.f90	(revision 0)
***************
*** 0 ****
--- 1,17 ----
+ ! { dg-do run }
+ !Test the fix for PR48955, in which a temporary was not being generated for
+ ! the dependent assignment to 'V1'.
+ !
+ ! Reported by Tobias Burnus  <burnus@gcc.gnu.org>
+ !
+ program ala
+    implicit none
+    integer, parameter  :: n = 8
+    real, dimension(n) :: v0, v1, v2
+    v0 = [-10.0, -10., -10., -10., 10., 10., 10., 10.]
+    v1 = v0
+    v2 = v0
+    v1(2:n-1) = 0.5*(v1(1:n-2) + v1(3:n) + 2.0*v1(2:n-1))  ! Needs temporary
+    v2(2:n-1) = 0.5*(v0(1:n-2) + v0(3:n) + 2.0*v0(2:n-1))
+    if (any (v1 .ne. v2)) call abort
+ end program ala

Re: [Patch, fortran] PR48955 [4.6/4.7 Regression] Wrong result for array assignment due to missing temporary

[Tobias Burnus]

On 05/12/2011 05:46 PM, Paul Richard Thomas wrote:
> This patch fixes the problem in two steps:
> (i) It reverts r162289; and
> (ii) It adds the correct initialization for loop.reverse[] in
> gfc_trans_assignment_1.  This was implemented incorrectly in the fix
> for PR24524 (in spite of the correct comment in dependency.c!) and
> removed at sometime, I do not know why.
> Bootstraps and regtests on x86_64/FC9.  OK for trunk and 4.6?

Looks OK - unless Thomas has objections.

I would prefer if you could credit Kacper Kowalik for the example - he 
reported it on IRC (and CCed himself to the PR after I reported it.) I 
only relayed the example and did some minor analysis.

Tobias

PS: Regarding regressions, I think gfortran is doing fine - ignoring the 
restricted pointer PR 45586. What are your next plans for gfortran? I 
wouldn't mind if you could fix PR 47674 - it always pops up as only 
testsuite failure when I regtest. Though, as the test case never worked, 
I am not sure whether one can classify it as regression or not. (You may 
need valgrind to reproduce the failure.)

> 2011-05-12  Paul Thomas<pault@gcc.gnu.org>
>
> 	PR fortran/48955
> 	* dependency.c (gfc_dep_resolver): Revert r162829 which changed
> 	the condition for setting this_dep to GFC_DEP_OVERLAP.
> 	* trans-expr.c (gfc_trans_assignment_1): Enable loop reversal.
>
> 2011-05-12  Paul Thomas<pault@gcc.gnu.org>
>
> 	PR fortran/48955
> 	* gfortran.dg/dependency_40.f90 : New test.