Patchwork [fortran] Fix PR 52893

login
register
mail settings
Submitter Thomas Koenig
Date April 6, 2012, 7:24 p.m.
Message ID <4F7F42E2.8070008@netcologne.de>
Download mbox | patch
Permalink /patch/151255/
State New
Headers show

Comments

Thomas Koenig - April 6, 2012, 7:24 p.m.
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  <tkoenig@gcc.gnu.org>

         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  <tkoenig@gcc.gnu.org>

         PR fortran/52893
         * gfortran.dg/function_optimize_11.f90:  New test.
! { 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
Paul Richard Thomas - April 7, 2012, 8 a.m.
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
Thomas Koenig - April 7, 2012, 4:41 p.m.
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

Patch

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);