diff mbox

[fortran] PR35339 Optimize implied do loops in io statements

Message ID 53722520-52ae-0fa7-d732-f30995c9f93a@student.ethz.ch
State New
Headers show

Commit Message

Nicolas Koenig May 29, 2017, 3:49 p.m. UTC
Hello Dominique,

mea culpa, their was a bit confusion with the file being open in emacs
and vi at the same time. Attached is the new patch with the #define removed.

Nicolas


On 05/29/2017 05:32 PM, Dominique d'Humières wrote:
> Hi Nicolas,
>
> Updating gfortran with your patch fails with
>
> ../../work/gcc/fortran/frontend-passes.c: In function 'bool traverse_io_block(gfc_code*, bool*, gfc_code*)':
> ../../work/gcc/fortran/frontend-passes.c:1067:20: error: expected unqualified-id before '(' token
>   #define swap(x, y) (x) ^= (y), (y) ^= (x), (x) ^= (y);
>                      ^
> ../../work/gcc/fortran/frontend-passes.c:1180:15: note: in expansion of macro 'swap'
>            std::swap(start->value.op.op1, start->value.op.op2);
>                 ^~~~
> ../../work/gcc/fortran/frontend-passes.c:1067:36: error: invalid operands of types 'gfc_expr*' and 'gfc_expr*' to binary 'operator^'
>   #define swap(x, y) (x) ^= (y), (y) ^= (x), (x) ^= (y);
>                                  ~~~~^~~~~~
> ../../work/gcc/fortran/frontend-passes.c:1180:15: note: in expansion of macro 'swap'
>            std::swap(start->value.op.op1, start->value.op.op2);
>                 ^~~~
> ../../work/gcc/fortran/frontend-passes.c:1067:41: error:   in evaluation of 'operator^=(struct gfc_expr*, struct gfc_expr*)'
>   #define swap(x, y) (x) ^= (y), (y) ^= (x), (x) ^= (y);
>                                           ^
> ../../work/gcc/fortran/frontend-passes.c:1180:15: note: in expansion of macro 'swap'
>            std::swap(start->value.op.op1, start->value.op.op2);
>                 ^~~~
> ../../work/gcc/fortran/frontend-passes.c:1067:48: error: invalid operands of types 'gfc_expr*' and 'gfc_expr*' to binary 'operator^'
>   #define swap(x, y) (x) ^= (y), (y) ^= (x), (x) ^= (y);
>                                              ~~~~^~~~~~
> ../../work/gcc/fortran/frontend-passes.c:1180:15: note: in expansion of macro 'swap'
>            std::swap(start->value.op.op1, start->value.op.op2);
>                 ^~~~
> ../../work/gcc/fortran/frontend-passes.c:1067:53: error:   in evaluation of 'operator^=(struct gfc_expr*, struct gfc_expr*)'
>   #define swap(x, y) (x) ^= (y), (y) ^= (x), (x) ^= (y);
>                                                       ^
> ../../work/gcc/fortran/frontend-passes.c:1180:15: note: in expansion of macro 'swap'
>            std::swap(start->value.op.op1, start->value.op.op2);
>                 ^~~~
>
> TIA
>
> Dominique
>

Comments

Dominique d'Humières May 29, 2017, 4:24 p.m. UTC | #1
> Le 29 mai 2017 à 17:49, Nicolas Koenig <koenigni@student.ethz.ch> a écrit :
> 
> Hello Dominique,
> 
> mea culpa, their was a bit confusion with the file being open in emacs
> and vi at the same time. Attached is the new patch with the #define removed.
> 
> Nicolas
> 

Thanks for the quick fix!

Testing in progress

Dominique
Bernhard Reutner-Fischer May 31, 2017, 6:16 a.m. UTC | #2
On 29 May 2017 17:49:30 CEST, Nicolas Koenig <koenigni@student.ethz.ch> wrote:
>Hello Dominique,
>
>mea culpa, their was a bit confusion with the file being open in emacs
>and vi at the same time. Attached is the new patch with the #define
>removed.


+static int
+simplify_io_impl_do (gfc_code **code, int *walk_subtrees,
+                  void *data ATTRIBUTE_UNUSED)
+{
+  gfc_code **curr, *prev = NULL;
+  struct do_stack write, first;
+  bool b = false;
+  *walk_subtrees = 1;
+  if (!(*code)->block || ((*code)->block->op != EXEC_WRITE
+                          && (*code)->block->op != EXEC_READ))
+    return 0;
+  
+  *walk_subtrees = 0;
+  write.prev = NULL;
+  write.iter = NULL;
+  write.code = *code;
+  
+  for (curr = &(*code)->block; *curr; curr = &(*curr)->next)
+    {
+      if ((*curr)->op == EXEC_DO)
+        {
+          first.prev = &write;
+	  first.iter = (*curr)->ext.iterator;
+  	  first.code = *curr;
+	  stack_top = &first;

It seems indentation is off above.
thanks,

+	  traverse_io_block((*curr)->block->next, &b, prev);
+	  stack_top = NULL;
+        }
+      prev = *curr;
+    }
+  return 0;
+}
Dominique d'Humières May 31, 2017, 3:40 p.m. UTC | #3
If I am not mistaken, compiling the following code with the patch applied

program test_ivs
  use iso_varying_string
  implicit none

  type(varying_string),dimension(:,:),allocatable :: array2d
  type(varying_string) :: extra
  integer :: i,j

  allocate(array2d(2,3))

  extra = "four"

  array2d(:,:) = reshape((/ var_str("1"), &
       var_str("2"), var_str("3"), &
       extra, var_str("5"), &
       var_str("six") /), (/ 2, 3 /))


  print *,"array2d second ",ubound(array2d),(("'"//char(array2d(i,j))//"' ",i=1,size(array2d,1)),j=1,size(array2d,2))

end program test_ivs

gives an ICE.

TIA

Dominique

> Le 31 mai 2017 à 08:16, Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> a écrit :
> 
> On 29 May 2017 17:49:30 CEST, Nicolas Koenig <koenigni@student.ethz.ch> wrote:
>> Hello Dominique,
>> 
>> mea culpa, their was a bit confusion with the file being open in emacs
>> and vi at the same time. Attached is the new patch with the #define
>> removed.
Dominique d'Humières May 31, 2017, 3:49 p.m. UTC | #4
> Le 31 mai 2017 à 17:40, Dominique d'Humières <dominiq@lps.ens.fr> a écrit :
> 
> If I am not mistaken, compiling the following code with the patch applied

simpler test

  print *,(huge(0),i=1,6)
!  print*,(i,i=1,6)
!  print*,(i,i=10000,60000,10000)
  end

> 
> gives an ICE.
> 
> TIA
> 
> Dominique
diff mbox

Patch

Index: frontend-passes.c
===================================================================
--- frontend-passes.c	(revision 248539)
+++ frontend-passes.c	(working copy)
@@ -1060,6 +1060,256 @@  convert_elseif (gfc_code **c, int *walk_subtrees A
   return 0;
 }
 
+struct do_stack
+{
+  struct do_stack *prev;
+  gfc_iterator *iter;
+  gfc_code *code;
+} *stack_top;
+
+/* Recursivly traverse the block of a WRITE or READ statement, and, can it be
+   optimized, do so. It optimizes it by replacing do loops with their analog
+   array slices. For example:
+   
+     write (*,*) (a(i), i=1,4)
+     
+   is replaced with
+     
+     write (*,*) a(1:4:1) .  */
+
+static bool 
+traverse_io_block(gfc_code *code, bool *has_reached, gfc_code *prev)
+{
+  gfc_code *curr; 
+  gfc_expr *new_e, *expr, *start;
+  gfc_ref *ref;
+  struct do_stack ds_push;
+  int i, future_rank = 0;
+  gfc_iterator *iters[GFC_MAX_DIMENSIONS];
+
+  /* Find the first transfer/do statement.  */
+  for (curr = code; curr; curr = curr->next)
+    {
+      if (curr->op == EXEC_DO || curr->op == EXEC_TRANSFER)
+        break;
+    }
+
+  /* Ensure it is the only transfer/do statement because cases like
+       
+       write (*,*) (a(i), b(i), i=1,4)
+
+     cannot be optimized.  */
+
+  if (!curr || curr->next)
+    return false;
+
+  if (curr->op == EXEC_DO)
+    {
+      if (curr->ext.iterator->var->ref)
+        return false;
+      ds_push.prev = stack_top;
+      ds_push.iter = curr->ext.iterator;
+      ds_push.code = curr;
+      stack_top = &ds_push;
+      if (traverse_io_block(curr->block->next, has_reached, prev))
+        {
+	  if (curr != stack_top->code && !*has_reached)
+	    {
+              curr->block->next = NULL;
+              gfc_free_statements(curr);
+	    }
+	  else
+	    *has_reached = true;
+	  return true;
+        }
+      return false;
+    }
+
+  gcc_assert(curr->op == EXEC_TRANSFER);
+
+  if (curr->expr1->symtree->n.sym->attr.allocatable)
+    return false;
+
+  ref = curr->expr1->ref;
+  if (!ref || ref->type != REF_ARRAY || ref->u.ar.codimen != 0)
+    return false;
+
+  /* Find the iterators belonging to each variable and check conditions.  */
+  for (i = 0; i < ref->u.ar.dimen; i++)
+    {
+      if (!ref->u.ar.start[i] || ref->u.ar.start[i]->ref
+          || ref->u.ar.dimen_type[i] != DIMEN_ELEMENT)
+        return false;
+      
+      start = ref->u.ar.start[i];
+      gfc_simplify_expr(start, 0);
+      switch (start->expr_type)
+        {
+	case EXPR_VARIABLE:
+
+	  /* write (*,*) (a(i), i=a%b,1) not handled yet.  */
+	  if (start->ref)
+	    return false;
+
+	  /*  Check for (a(k), i=1,4) or ((a(j, i), i=1,4), j=1,4).  */
+	  if (!stack_top || !stack_top->iter 
+	     || stack_top->iter->var->symtree != start->symtree)
+	    iters[i] = NULL; 
+	  else
+	    {
+              iters[i] = stack_top->iter;
+	      stack_top = stack_top->prev;
+	      future_rank++;
+	    }
+	  break;
+        case EXPR_CONSTANT:
+	  iters[i] = NULL;
+	  break;
+	case EXPR_OP:
+          switch (start->value.op.op)
+	    {
+	    case INTRINSIC_PLUS:
+	    case INTRINSIC_TIMES:
+	      if (start->value.op.op1->expr_type != EXPR_VARIABLE)
+	        std::swap(start->value.op.op1, start->value.op.op2);
+	    __attribute__((fallthrough));
+	    case INTRINSIC_MINUS:
+	      if ((start->value.op.op1->expr_type!= EXPR_VARIABLE 
+	            && start->value.op.op2->expr_type != EXPR_CONSTANT)
+	          || start->value.op.op1->ref)
+	        return false;
+              if (!stack_top || !stack_top->iter 
+	         || stack_top->iter->var->symtree 
+		    != start->value.op.op1->symtree)
+	        return false;
+	      iters[i] = stack_top->iter; 
+	      stack_top = stack_top->prev;
+	      break;
+	    default:
+	      return false;
+	    }
+	    future_rank++;
+	  break;
+	default:
+	  return false;
+        }
+    }
+
+  /* Create new expr.  */
+  new_e = gfc_copy_expr(curr->expr1);
+  new_e->expr_type = EXPR_VARIABLE;
+  new_e->rank = future_rank; 
+  if (curr->expr1->shape)
+    {
+      new_e->shape = gfc_get_shape(new_e->rank);
+    }
+
+  /* Assign new starts, ends and strides if necessary.  */
+  for (i = 0; i < ref->u.ar.dimen; i++)
+    {
+      if (!iters[i])
+        continue;
+      start = ref->u.ar.start[i];
+      switch (start->expr_type)
+        {
+	case EXPR_CONSTANT:
+	  gfc_internal_error("bad expression");
+	  break;
+	case EXPR_VARIABLE:
+	  new_e->ref->u.ar.dimen_type[i] = DIMEN_RANGE;
+	  new_e->ref->u.ar.type = AR_SECTION;
+	  gfc_free_expr(new_e->ref->u.ar.start[i]);
+	  new_e->ref->u.ar.start[i] = gfc_copy_expr(iters[i]->start);
+	  new_e->ref->u.ar.end[i] = gfc_copy_expr(iters[i]->end);
+	  new_e->ref->u.ar.stride[i] = gfc_copy_expr(iters[i]->step);
+          break;
+	case EXPR_OP:
+	  new_e->ref->u.ar.dimen_type[i] = DIMEN_RANGE; 
+	  new_e->ref->u.ar.type = AR_SECTION;
+	  gfc_free_expr(new_e->ref->u.ar.start[i]);
+	  expr = gfc_copy_expr(start);
+	  expr->value.op.op1 = gfc_copy_expr(iters[i]->start);
+	  new_e->ref->u.ar.start[i] = expr;
+	  expr = gfc_copy_expr(start);
+	  expr->value.op.op1 = gfc_copy_expr(iters[i]->end);
+	  new_e->ref->u.ar.end[i] = expr;
+	  switch(start->value.op.op)
+	    {
+	    case INTRINSIC_MINUS:
+	    case INTRINSIC_PLUS:
+	      new_e->ref->u.ar.stride[i] = gfc_copy_expr(iters[i]->step);
+	      break;
+	    case INTRINSIC_TIMES:
+	      expr = gfc_copy_expr(start);
+	      expr->value.op.op1 = gfc_copy_expr(iters[i]->step);
+	      new_e->ref->u.ar.stride[i] = expr;
+	      break;
+	    default:
+	      gfc_internal_error("bad op");
+	    }
+	  break;
+	default:
+	  gfc_internal_error("bad expression");
+	}
+    }
+  curr->expr1 = new_e;
+
+  /* Insert modified statement.  Check whether the statement needs to be
+     inserted at the lowest level.  */
+  if (!stack_top->iter)
+    {
+      if (prev)
+        {
+          curr->next = prev->next->next;
+          prev->next = curr;
+	}
+      else 
+        {
+          curr->next = stack_top->code->block->next->next->next;
+	  stack_top->code->block->next = curr;
+	}
+    }
+  else
+    stack_top->code->block->next = curr;
+  return true;
+}
+
+/* Function for the gfc_code_walker.  If code is a READ or WRITE statement, it
+   tries to optimize its block.  */
+
+static int
+simplify_io_impl_do (gfc_code **code, int *walk_subtrees,
+                  void *data ATTRIBUTE_UNUSED)
+{
+  gfc_code **curr, *prev = NULL;
+  struct do_stack write, first;
+  bool b = false;
+  *walk_subtrees = 1;
+  if (!(*code)->block || ((*code)->block->op != EXEC_WRITE
+                          && (*code)->block->op != EXEC_READ))
+    return 0;
+  
+  *walk_subtrees = 0;
+  write.prev = NULL;
+  write.iter = NULL;
+  write.code = *code;
+  
+  for (curr = &(*code)->block; *curr; curr = &(*curr)->next)
+    {
+      if ((*curr)->op == EXEC_DO)
+        {
+          first.prev = &write;
+	  first.iter = (*curr)->ext.iterator;
+  	  first.code = *curr;
+	  stack_top = &first;
+	  traverse_io_block((*curr)->block->next, &b, prev);
+	  stack_top = NULL;
+        }
+      prev = *curr;
+    }
+  return 0;
+}
+
 /* Optimize a namespace, including all contained namespaces.  */
 
 static void
@@ -1073,6 +1323,7 @@  optimize_namespace (gfc_namespace *ns)
   in_assoc_list = false;
   in_omp_workshare = false;
 
+  gfc_code_walker (&ns->code, simplify_io_impl_do, dummy_expr_callback, NULL);
   gfc_code_walker (&ns->code, convert_do_while, dummy_expr_callback, NULL);
   gfc_code_walker (&ns->code, convert_elseif, dummy_expr_callback, NULL);
   gfc_code_walker (&ns->code, cfe_code, cfe_expr_0, NULL);