Re: [patch, fortran] Fix PR 52893

[Thomas Koenig]

Hi Paul,


OK for trunk and for 4.7.


Committed as rev. 186213.


As a matter of curiosity, why did you not inhibit common function
extraction when the function arguments contain an implicit loop
variable, rather than when they are in an implicit loop?  That would
make the optimisation rather less conservative.


The main reason was simplicity; it would have been necessary to keep
track of all the iterator variables (which can be nested) and check
if the function has any of them as arguments.  Not impossible, but
I wanted this bug fix out of the door as soon as possible.

I have added a FIXME in the comment for this.

Thanks for the review!

Thomas

Re: [patch, fortran] Fix PR 52893

[Paul Richard Thomas]

Dear Thomas,


> after some time with a defective computer, I am back online.

It seems to be catching both my linux laptop and my desktop are as
dead as door-nails.

>
> Here is a fix for PR 52893 (which I just submitted, I wanted to
> set a new record between bug posting and patch submissin :-), a
> wrong-code regression for trunk and 4.7. Regression-tested.
>
> OK for both?

OK for trunk and for 4.7.

As a matter of curiosity, why did you not inhibit common function
extraction when the function arguments contain an implicit loop
variable, rather than when they are in an implicit loop?  That would
make the optimisation rather less conservative.

Thanks for the patch

Paul

[patch, fortran] Fix PR 52893

[Thomas Koenig]

Hello world,

after some time with a defective computer, I am back online.

Here is a fix for PR 52893 (which I just submitted, I wanted to
set a new record between bug posting and patch submissin :-), a
wrong-code regression for trunk and 4.7. Regression-tested.

OK for both?

Thomas

2012-04-06  Thomas Koenig  

PR fortran/52893
* frontend-passes.c:  Keep track of wether we are in an implicit
DO loop; do not do function elimination if we are.

2012-04-06  Thomas Koenig  

PR fortran/52893
* gfortran.dg/function_optimize_11.f90:  New test.
Index: frontend-passes.c
===
--- frontend-passes.c	(Revision 186190)
+++ frontend-passes.c	(Arbeitskopie)
@@ -70,6 +70,10 @@ static int forall_level;
 
 static bool in_omp_workshare;
 
+/* Keep track of iterators for array constructors.  */
+
+static int iterator_level;
+
 /* Entry point - run all passes for a namespace.  So far, only an
optimization pass is run.  */
 
@@ -179,6 +183,12 @@ cfe_register_funcs (gfc_expr **e, int *walk_subtre
   if (forall_level > 0)
 return 0;
 
+  /* Function elimination inside an iterator could lead to functions
+ which depend on iterator variables being moved outside.  */
+
+  if (iterator_level > 0)
+return 0;
+
   /* If we don't know the shape at compile time, we create an allocatable
  temporary variable to hold the intermediate result, but only if
  allocation on assignment is active.  */
@@ -581,6 +591,7 @@ optimize_namespace (gfc_namespace *ns)
 
   current_ns = ns;
   forall_level = 0;
+  iterator_level = 0;
   in_omp_workshare = false;
 
   gfc_code_walker (&ns->code, convert_do_while, dummy_expr_callback, NULL);
@@ -1140,9 +1151,13 @@ gfc_expr_walker (gfc_expr **e, walk_expr_fn_t expr
 	for (c = gfc_constructor_first ((*e)->value.constructor); c;
 		 c = gfc_constructor_next (c))
 	  {
-		WALK_SUBEXPR (c->expr);
-		if (c->iterator != NULL)
+		if (c->iterator == NULL)
+		  WALK_SUBEXPR (c->expr);
+		else
 		  {
+		iterator_level ++;
+		WALK_SUBEXPR (c->expr);
+		iterator_level --;
 		WALK_SUBEXPR (c->iterator->var);
 		WALK_SUBEXPR (c->iterator->start);
 		WALK_SUBEXPR (c->iterator->end);
! { dg-do run }
! { dg-options "-ffrontend-optimize" }
! Do not move common functions out of implicit DO loop constructors.
program test
  integer, parameter :: N = 4
  integer, parameter :: dp=kind(1.d0)
  real(kind=dp), parameter :: pi=4*atan(1._dp)
  real(kind=dp), parameter :: eps = 1.e-14_dp
  real(kind=dp) :: h1(0:N-1), h2(0:N-1)
  integer i

  i = 1
  h1 = [(cos(2*pi*mod(i*k,N)/N),k=0,N/2), &
   & (sin(2*pi*mod(i*k,N)/N),k=1,N/2-1)]
  h2 = (/ 1._dp, 0._dp, -1._dp, 1._dp /)
  if (any(abs(h1 - h2) > eps)) call abort
end program test