diff mbox

[RFC] Fix PR49957 - build array index differently

Message ID 201108032312.46775.mikael.morin@sfr.fr
State New
Headers show

Commit Message

Mikael Morin Aug. 3, 2011, 9:12 p.m. UTC 
Hello,

On Wednesday 03 August 2011 15:47:37 Richard Guenther wrote:
> Comments?  Any idea why reversing the loop would break?

Yes, the list of scalarized expressions has to be created in the same order it 
is consumed. Here the scalarized expressions are array indexes to be 
precomputed out of the loop.
The attached patch seems to work (the interesting part is in 
gfc_walk_variable_expr).

Mikael

Comments

Richard Biener Aug. 4, 2011, 9:22 a.m. UTC | #1 
On Wed, 3 Aug 2011, Mikael Morin wrote:

> Hello,
> 
> On Wednesday 03 August 2011 15:47:37 Richard Guenther wrote:
> > Comments?  Any idea why reversing the loop would break?
> 
> Yes, the list of scalarized expressions has to be created in the same order it 
> is consumed. Here the scalarized expressions are array indexes to be 
> precomputed out of the loop.
> The attached patch seems to work (the interesting part is in 
> gfc_walk_variable_expr).

Ah, thanks.  I'll work from there to revise the patch.

Richard.
diff mbox

Patch

diff --git a/trans-array.c b/trans-array.c
index 85acf0c..ebab43f 100644
--- a/trans-array.c
+++ b/trans-array.c
@@ -2623,6 +2623,21 @@  gfc_conv_tmp_array_ref (gfc_se * se)
 }
 
 
+static void
+add_to_offset (tree *cst_offset, tree *offset, tree value)
+{
+  if (integer_zerop (value))
+    return;
+
+  if (TREE_CODE (value) == INTEGER_CST)
+    *cst_offset = fold_build2_loc (input_location, PLUS_EXPR,
+				   gfc_array_index_type, *cst_offset, value);
+  else
+    *offset = fold_build2_loc (input_location, PLUS_EXPR,
+			       gfc_array_index_type, *offset, value);
+}
+
+
 /* Build an array reference.  se->expr already holds the array descriptor.
    This should be either a variable, indirect variable reference or component
    reference.  For arrays which do not have a descriptor, se->expr will be
@@ -2634,9 +2649,9 @@  gfc_conv_array_ref (gfc_se * se, gfc_array_ref * ar, gfc_symbol * sym,
 		    locus * where)
 {
   int n;
-  tree index;
   tree tmp;
   tree stride;
+  tree offset, cst_offset;
   gfc_se indexse;
   gfc_se tmpse;
 
@@ -2669,10 +2684,12 @@  gfc_conv_array_ref (gfc_se * se, gfc_array_ref * ar, gfc_symbol * sym,
       return;
     }
 
-  index = gfc_index_zero_node;
+  cst_offset = offset = gfc_index_zero_node;
+
+  add_to_offset (&cst_offset, &offset, gfc_conv_array_offset (se->expr));
 
   /* Calculate the offsets from all the dimensions.  */
-  for (n = 0; n < ar->dimen; n++)
+  for (n = ar->dimen - 1; n >= 0; n--)
     {
       /* Calculate the index for this dimension.  */
       gfc_init_se (&indexse, se);
@@ -2741,19 +2758,17 @@  gfc_conv_array_ref (gfc_se * se, gfc_array_ref * ar, gfc_symbol * sym,
 			     indexse.expr, stride);
 
       /* And add it to the total.  */
-      index = fold_build2_loc (input_location, PLUS_EXPR,
-			       gfc_array_index_type, index, tmp);
+      add_to_offset (&cst_offset, &offset, tmp);
     }
 
-  tmp = gfc_conv_array_offset (se->expr);
-  if (!integer_zerop (tmp))
-    index = fold_build2_loc (input_location, PLUS_EXPR,
-			     gfc_array_index_type, index, tmp);
+  if (!integer_zerop (cst_offset))
+    offset = fold_build2_loc (input_location, PLUS_EXPR,
+			      gfc_array_index_type, offset, cst_offset);
 
   /* Access the calculated element.  */
   tmp = gfc_conv_array_data (se->expr);
   tmp = build_fold_indirect_ref (tmp);
-  se->expr = gfc_build_array_ref (tmp, index, sym->backend_decl);
+  se->expr = gfc_build_array_ref (tmp, offset, sym->backend_decl);
 }
 
 
@@ -7575,7 +7590,7 @@  gfc_walk_variable_expr (gfc_ss * ss, gfc_expr * expr)
       switch (ar->type)
 	{
 	case AR_ELEMENT:
-	  for (n = 0; n < ar->dimen + ar->codimen; n++)
+	  for (n = ar->dimen + ar->codimen - 1; n >= 0; n--)
 	    {
 	      newss = gfc_get_ss ();
 	      newss->type = GFC_SS_SCALAR;