diff mbox

[fortran,4.6/4.7,Regression] Fix PR 48955

Message ID BANLkTi=oKkyxhz=hM+zH-LhhhYOOTnPaOg@mail.gmail.com
State New
Headers show

Commit Message

Paul Richard Thomas May 24, 2011, 9:18 p.m. UTC 
Dear All,
>
> I have posted a simpler alternative on the PR that uses your
> suggestion that forward and backward dependences need to to be
> recorded to get this right.
>
> I believe that it's OK but have only now had the opportunity to put it
> on to regtest.
>

Following some comments from Thomas, the attached is the version that
I wound up with.  Tell us which one you prefer and Thomas and I will
do the honours.

Bootstrapped and regtested on FC9/x86_64  - OK for trunk and 4.6?

Paul

2011-05-24  Paul Thomas  <pault@gcc.gnu.org>
	    Thomas Koenig  <tkoenig@gcc.gnu.org>

	PR fortran/48955
	* trans-expr.c (gfc_trans_assignment_1): GFC_REVERSE_NOT_SET
	changed to GFC_ENABLE_REVERSE.
	* trans-array.c (gfc_init_loopinfo): GFC_CANNOT_REVERSE changed
	to GFC_INHIBIT_REVERSE.
	* gfortran.h : Enum gfc_reverse is now GFC_ENABLE_REVERSE,
	GFC_FORWARD_SET, GFC_REVERSE_SET and GFC_INHIBIT_REVERSE.
	* dependency.c (gfc_dep_resolver): Change names for elements of
	gfc_reverse as necessary. Change the logic so that forward
	dependences are remembered as well as backward ones. When both
	have appeared, force a temporary.

2011-05-24  Thomas Koenig  <tkoenig@gcc.gnu.org>

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

Comments

Thomas Koenig May 26, 2011, 6:36 a.m. UTC | #1 
Dear Paul,


> Following some comments from Thomas, the attached is the version that
> I wound up with.  Tell us which one you prefer and Thomas and I will
> do the honours.
>
> Bootstrapped and regtested on FC9/x86_64  - OK for trunk and 4.6?

Your patch is does the job and is less intrusive than mine - therefore 
OK for trunk and 4.6.

Thanks a lot!  I'm glad we could get this resolved long before 4.6.1.

	Thomas
diff mbox

Patch

Index: gcc/fortran/trans-expr.c
===================================================================
*** gcc/fortran/trans-expr.c	(revision 173213)
--- gcc/fortran/trans-expr.c	(working copy)
*************** gfc_trans_assignment_1 (gfc_expr * expr1
*** 6069,6076 ****
        /* Calculate the bounds of the scalarization.  */
        gfc_conv_ss_startstride (&loop);
        /* Enable loop reversal.  */
!       for (n = 0; n < loop.dimen; n++)
! 	loop.reverse[n] = GFC_REVERSE_NOT_SET;
        /* Resolve any data dependencies in the statement.  */
        gfc_conv_resolve_dependencies (&loop, lss, rss);
        /* Setup the scalarizing loops.  */
--- 6069,6076 ----
        /* Calculate the bounds of the scalarization.  */
        gfc_conv_ss_startstride (&loop);
        /* Enable loop reversal.  */
!       for (n = 0; n < GFC_MAX_DIMENSIONS; n++)
! 	loop.reverse[n] = GFC_ENABLE_REVERSE;
        /* Resolve any data dependencies in the statement.  */
        gfc_conv_resolve_dependencies (&loop, lss, rss);
        /* Setup the scalarizing loops.  */
Index: gcc/fortran/trans-array.c
===================================================================
*** gcc/fortran/trans-array.c	(revision 173212)
--- gcc/fortran/trans-array.c	(working copy)
*************** gfc_init_loopinfo (gfc_loopinfo * loop)
*** 2244,2250 ****
    for (n = 0; n < GFC_MAX_DIMENSIONS; n++)
      {
        loop->order[n] = n;
!       loop->reverse[n] = GFC_CANNOT_REVERSE;
      }
  
    loop->ss = gfc_ss_terminator;
--- 2244,2250 ----
    for (n = 0; n < GFC_MAX_DIMENSIONS; n++)
      {
        loop->order[n] = n;
!       loop->reverse[n] = GFC_INHIBIT_REVERSE;
      }
  
    loop->ss = gfc_ss_terminator;
Index: gcc/fortran/gfortran.h
===================================================================
*** gcc/fortran/gfortran.h	(revision 173212)
--- gcc/fortran/gfortran.h	(working copy)
*************** gfc_fcoarray;
*** 578,587 ****
  
  typedef enum
  {
!   GFC_REVERSE_NOT_SET,
    GFC_REVERSE_SET,
!   GFC_CAN_REVERSE,
!   GFC_CANNOT_REVERSE
  }
  gfc_reverse;
  
--- 578,587 ----
  
  typedef enum
  {
!   GFC_ENABLE_REVERSE,
!   GFC_FORWARD_SET,
    GFC_REVERSE_SET,
!   GFC_INHIBIT_REVERSE
  }
  gfc_reverse;
  
Index: gcc/fortran/dependency.c
===================================================================
*** gcc/fortran/dependency.c	(revision 173212)
--- gcc/fortran/dependency.c	(working copy)
*************** gfc_dep_resolver (gfc_ref *lref, gfc_ref
*** 1807,1813 ****
  
  	      /* Now deal with the loop reversal logic:  This only works on
  		 ranges and is activated by setting
! 				reverse[n] == GFC_CAN_REVERSE
  		 The ability to reverse or not is set by previous conditions
  		 in this dimension.  If reversal is not activated, the
  		 value GFC_DEP_BACKWARD is reset to GFC_DEP_OVERLAP.  */
--- 1807,1813 ----
  
  	      /* Now deal with the loop reversal logic:  This only works on
  		 ranges and is activated by setting
! 				reverse[n] == GFC_ENABLE_REVERSE
  		 The ability to reverse or not is set by previous conditions
  		 in this dimension.  If reversal is not activated, the
  		 value GFC_DEP_BACKWARD is reset to GFC_DEP_OVERLAP.  */
*************** gfc_dep_resolver (gfc_ref *lref, gfc_ref
*** 1815,1839 ****
  		    && lref->u.ar.dimen_type[n] == DIMEN_RANGE)
  		{
  		  /* Set reverse if backward dependence and not inhibited.  */
! 		  if (reverse && reverse[n] != GFC_CANNOT_REVERSE)
  		    reverse[n] = (this_dep == GFC_DEP_BACKWARD) ?
  			         GFC_REVERSE_SET : reverse[n];
  
! 		  /* Inhibit loop reversal if dependence not compatible.  */
! 		  if (reverse && reverse[n] != GFC_REVERSE_NOT_SET
! 		        && this_dep != GFC_DEP_EQUAL
! 		        && this_dep != GFC_DEP_BACKWARD
! 		        && this_dep != GFC_DEP_NODEP)
  		    {
! 	              reverse[n] = GFC_CANNOT_REVERSE;
! 		      if (this_dep != GFC_DEP_FORWARD)
! 			this_dep = GFC_DEP_OVERLAP;
  		    }
  
  		  /* 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;
  		}
  
--- 1815,1848 ----
  		    && lref->u.ar.dimen_type[n] == DIMEN_RANGE)
  		{
  		  /* Set reverse if backward dependence and not inhibited.  */
! 		  if (reverse && reverse[n] == GFC_ENABLE_REVERSE)
  		    reverse[n] = (this_dep == GFC_DEP_BACKWARD) ?
  			         GFC_REVERSE_SET : reverse[n];
  
! 		  /* Set forward if forward dependence and not inhibited.  */
! 		  if (reverse && reverse[n] == GFC_ENABLE_REVERSE)
! 		    reverse[n] = (this_dep == GFC_DEP_FORWARD) ?
! 			         GFC_FORWARD_SET : reverse[n];
! 
! 		  /* Flag up overlap if dependence not compatible with
! 		     the overall state of the expression.  */
! 		  if (reverse && reverse[n] == GFC_REVERSE_SET
! 		        && this_dep == GFC_DEP_FORWARD)
  		    {
! 	              reverse[n] = GFC_INHIBIT_REVERSE;
! 		      this_dep = GFC_DEP_OVERLAP;
! 		    }
! 		  else if (reverse && reverse[n] == GFC_FORWARD_SET
! 		        && this_dep == GFC_DEP_BACKWARD)
! 		    {
! 	              reverse[n] = GFC_INHIBIT_REVERSE;
! 		      this_dep = GFC_DEP_OVERLAP;
  		    }
  
  		  /* 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_INHIBIT_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,29 ----
+ ! { dg-do run }
+ ! PR 48955 - missing array temporary when there was both a forward
+ ! and a backward dependency.
+ ! Test case slightly modified from the original one by Kacper Kowalik.
+ program ala
+    implicit none
+ 
+    integer, parameter  :: n = 6
+    real, dimension(n), parameter :: result = [1.,10.,30.,90.,270., 243.];
+    real, dimension(n) :: v0, v1
+    character(len=80) :: line1, line2
+ 
+    v0 = [1.0, 3.0, 9.0, 27.0, 81.0, 243.0]
+    v1 = v0
+ 
+    v1(2:n-1) = v1(1:n-2) + v1(3:n)
+    if (any(v1 /= result)) call abort
+    v1 = v0
+    v1(2:n-1) = v0(1:n-2) + v0(3:n)
+    if (any(v1 /= result)) call abort
+ 
+    v1 = v0
+    v1(2:n-1) = v1(3:n) + v1(1:n-2)
+    if (any(v1 /= result)) call abort
+    v1 = v0
+    v1(2:n-1) = v0(3:n) + v0(1:n-2)
+    if (any(v1 /= result)) call abort
+ 
+ end program ala