diff mbox

[pach,Fortran] Improve dependency checking

Message ID 1281378170.3622.5.camel@linux-fd1f.site
State New
Headers show

Commit Message

Thomas Koenig Aug. 9, 2010, 6:22 p.m. UTC 
Hello world,

here's another installment of dependency checking improvements.

I debated a little with myself if extending gfc_ref_dimen_size really
was the right approach, but I didn't want to duplicate its code.

Regression-tested on trunk.  OK?

[If there isn't time to review this before tomorrow evening, I would
ask somebody to commit this for me].

	Thomas

2010-08-09  Thomas Koenig  <tkoenig@gcc.gnu.org>

	PR fortran/44235
	* array.c (gfc_ref_dimen_size):  Add end argument.
	If end is non-NULL, calculate it.
	(ref_size):  Adjust call to gfc_ref_dimen_size.
	(gfc_array_dimen_size):  Likewise.
	(gfc_array_res_shape):  Likewise.
	* gfortran.h:  Adjust prototype for gfc_ref_dimen_size.
	* resolve.c (resolve_array_ref):  For stride not equal to -1,
	fill in the lowest possible end.

2010-08-09  Thomas Koenig  <tkoenig@gcc.gnu.org>

	PR fortran/44235
	* dependency_32.f90:  New test.

Comments

Daniel Kraft Aug. 9, 2010, 7:09 p.m. UTC | #1 
Thomas Koenig wrote:
> Hello world,
> 
> here's another installment of dependency checking improvements.
> 
> I debated a little with myself if extending gfc_ref_dimen_size really
> was the right approach, but I didn't want to duplicate its code.
> 
> Regression-tested on trunk.  OK?

Ok.  Just two comments:

+      /* Fill in the upper bound, which may be lower than the
+	 specified one for something like a(2:10:5), which is
+	 identical to a(2:7:5).  Only relevant for non-zero strides.  */

I don't really like "non-zero" stride; I guess you mean that stride is 
present and not one (or did I misunderstand this?) -- in this case maybe 
"stride not one" is better.  Although it seems still clear what you mean.

And secondly, I do not oppose your extension of gfc_ref_dimen_size; 
however, it seems to me that you could have implemented a 
gfc_ref_dimen_end function easily by calling gfc_ref_dimen_size from it, 
instead of duplicating the code.  So if you want, you could do this 
instead?  For me, either way is fine, though.

Thanks for your work on the dependencies!

Daniel
Thomas Koenig Aug. 9, 2010, 7:38 p.m. UTC | #2 
Hi Daniel,

> Ok.  Just two comments:
> 
> +      /* Fill in the upper bound, which may be lower than the
> +        specified one for something like a(2:10:5), which is
> +        identical to a(2:7:5).  Only relevant for non-zero strides.
> */


Now replaced with "strides not equal to one."  Thanks for pointing
this out!


> And secondly, I do not oppose your extension of gfc_ref_dimen_size; 
> however, it seems to me that you could have implemented a 
> gfc_ref_dimen_end function easily by calling gfc_ref_dimen_size from
> it, 
> instead of duplicating the code.  So if you want, you could do this 
> instead?

I would also have had to write the code to access the start, or factor
that out of gfc_ref_dimen_size as well.

Übertrage Daten .......
Revision 163041 übertragen.

Thanks for the quick review!

	Thomas
diff mbox

Patch

Index: array.c
===================================================================
--- array.c	(Revision 162824)
+++ array.c	(Arbeitskopie)
@@ -1940,10 +1940,11 @@  spec_size (gfc_array_spec *as, mpz_t *result)
 }
 
 
-/* Get the number of elements in an array section.  */
+/* Get the number of elements in an array section. Optionally, also supply
+   the end value.  */
 
 gfc_try
-gfc_ref_dimen_size (gfc_array_ref *ar, int dimen, mpz_t *result)
+gfc_ref_dimen_size (gfc_array_ref *ar, int dimen, mpz_t *result, mpz_t *end)
 {
   mpz_t upper, lower, stride;
   gfc_try t;
@@ -2016,6 +2017,15 @@  gfc_try
 	mpz_set_ui (*result, 0);
       t = SUCCESS;
 
+      if (end)
+	{
+	  mpz_init (*end);
+
+	  mpz_sub_ui (*end, *result, 1UL);
+	  mpz_mul (*end, *end, stride);
+	  mpz_add (*end, *end, lower);
+	}
+
     cleanup:
       mpz_clear (upper);
       mpz_clear (lower);
@@ -2040,7 +2050,7 @@  ref_size (gfc_array_ref *ar, mpz_t *result)
 
   for (d = 0; d < ar->dimen; d++)
     {
-      if (gfc_ref_dimen_size (ar, d, &size) == FAILURE)
+      if (gfc_ref_dimen_size (ar, d, &size, NULL) == FAILURE)
 	{
 	  mpz_clear (*result);
 	  return FAILURE;
@@ -2086,7 +2096,7 @@  gfc_array_dimen_size (gfc_expr *array, int dimen,
 		if (ref->u.ar.dimen_type[i] != DIMEN_ELEMENT)
 		  dimen--;
 
-	      return gfc_ref_dimen_size (&ref->u.ar, i - 1, result);
+	      return gfc_ref_dimen_size (&ref->u.ar, i - 1, result, NULL);
 	    }
 	}
 
@@ -2222,7 +2232,7 @@  gfc_array_ref_shape (gfc_array_ref *ar, mpz_t *sha
 	{
 	  if (ar->dimen_type[i] != DIMEN_ELEMENT)
 	    {
-	      if (gfc_ref_dimen_size (ar, i, &shape[d]) == FAILURE)
+	      if (gfc_ref_dimen_size (ar, i, &shape[d], NULL) == FAILURE)
 		goto cleanup;
 	      d++;
 	    }
Index: gfortran.h
===================================================================
--- gfortran.h	(Revision 162824)
+++ gfortran.h	(Arbeitskopie)
@@ -2753,7 +2753,7 @@  gfc_try spec_size (gfc_array_spec *, mpz_t *);
 gfc_try spec_dimen_size (gfc_array_spec *, int, mpz_t *);
 int gfc_is_compile_time_shape (gfc_array_spec *);
 
-gfc_try gfc_ref_dimen_size (gfc_array_ref *, int dimen, mpz_t *);
+gfc_try gfc_ref_dimen_size (gfc_array_ref *, int dimen, mpz_t *, mpz_t *);
 
 
 /* interface.c -- FIXME: some of these should be in symbol.c */
Index: resolve.c
===================================================================
--- resolve.c	(Revision 162824)
+++ resolve.c	(Arbeitskopie)
@@ -4316,6 +4316,37 @@  resolve_array_ref (gfc_array_ref *ar)
 		       &ar->c_where[i], e->rank);
 	    return FAILURE;
 	  }
+
+      /* Fill in the upper bound, which may be lower than the
+	 specified one for something like a(2:10:5), which is
+	 identical to a(2:7:5).  Only relevant for non-zero strides.  */
+      if (ar->dimen_type[i] == DIMEN_RANGE
+	  && ar->stride[i] != NULL && ar->stride[i]->expr_type == EXPR_CONSTANT
+	  && mpz_cmp_si (ar->stride[i]->value.integer, 1L) != 0)
+	{
+	  mpz_t size, end;
+
+	  if (gfc_ref_dimen_size (ar, i, &size, &end) == SUCCESS)
+	    {
+	      if (ar->end[i] == NULL)
+		{
+		  ar->end[i] =
+		    gfc_get_constant_expr (BT_INTEGER, gfc_index_integer_kind,
+					   &ar->where);
+		  mpz_set (ar->end[i]->value.integer, end);
+		}
+	      else if (ar->end[i]->ts.type == BT_INTEGER
+		       && ar->end[i]->expr_type == EXPR_CONSTANT)
+		{
+		  mpz_set (ar->end[i]->value.integer, end);
+		}
+	      else
+		gcc_unreachable ();
+
+	      mpz_clear (size);
+	      mpz_clear (end);
+	    }
+	}
     }
 
   if (ar->type == AR_FULL && ar->as->rank == 0)
Index: simplify.c
===================================================================
--- simplify.c	(Revision 162824)
+++ simplify.c	(Arbeitskopie)
@@ -2807,7 +2807,7 @@  simplify_bound_dim (gfc_expr *array, gfc_expr *kin
     {
       if (upper)
 	{
-	  if (gfc_ref_dimen_size (&ref->u.ar, d-1, &result->value.integer)
+	  if (gfc_ref_dimen_size (&ref->u.ar, d-1, &result->value.integer, NULL)
 	      != SUCCESS)
 	    goto returnNull;
 	}