diff mbox

[Fortran] Simplify lbound

Message ID 553E83B4.8020005@netcologne.de
State New
Headers show

Commit Message

Thomas Koenig April 27, 2015, 6:45 p.m. UTC
Am 25.04.2015 um 20:12 schrieb Mikael Morin:

> I've double-checked in the standard, and it seems it is not possible to
> simplify after all:
> 
> 	If ARRAY is a whole array and either ARRAY is an assumed-size
> 	array of rank DIM or dimension DIM of ARRAY has nonzero extent,
> 	LBOUND (ARRAY, DIM) has a value equal to the lower bound for
> 	subscript DIM of ARRAY. Otherwise the result value is 1.
> 
> We can't tell whether the array is zero-sized, so we can't tell the
> lbound value.

So it is only possible to simplify LBOUND if the lower bound is
equal to one, both for assumed-shape and explicit-shape arrays...
OK.

The attached patch does that, including a test case which catches
that particular case.

> As you may want to simplify in the limited scope of the matmul inlining,
> I'm giving comments about the patch (otherwise you can ignore them):
>  - No need to check for allocatable or pointer, it should be excluded by
> as->type == AS_ASSUMED_SHAPE (but does no harm either).

Actually, no.  You can have assumed-shape allocatable or pointer
dummy arguments which keep their original lbound; see the subroutine
'bar' in the test case.

>  - Please modify the early return condition:
>      if (as && (as->type == AS_DEFERRED || as->type == AS_ASSUMED_SHAPE
> 	        || as->type == AS_ASSUMED_RANK))
>        return NULL;
>    and let the existing code do the simplification work.

That is not part of my patch.

> Or drop the lbound simplification idea, and fetch the lbound "by hand"
> at matmul inline time.

I will probably do so as a future optimization, but I think that most
people will see no reason for using different lower bounds, so it is
OK for the time being to (slightly) pessimize this case.

So... here is the new patch.  OK for trunk?

	Thomas

2015-04-25  Thomas Koenig  <tkoenig@gcc.gnu.org>

        PR fortran/37131
        * simplify.c (simplify_bound): Get constant lower bounds of one
        from array spec for assumed and explicit shape shape arrays if
        the lower bounds are indeed one.

2015-04-25  Thomas Koenig  <tkoenig@gcc.gnu.org>

        PR fortran/37131
        * gfortran.dg/coarray_lib_this_image_2.f90:  Adjust
        scan pattern.
        * gfortran.dg/bound_9.f90:  New test case.

P.S:

In an earlier version, I also added


but that caused the condition to always return true.  I haven't figured
out why, but either I am misunderstanding something here, or gfc_likely
is buggy, or both.

Comments

Mikael Morin April 30, 2015, 6:19 p.m. UTC | #1
Le 27/04/2015 20:45, Thomas Koenig a écrit :
> Am 25.04.2015 um 20:12 schrieb Mikael Morin:
> 
>> I've double-checked in the standard, and it seems it is not possible to
>> simplify after all:
>>
>> 	If ARRAY is a whole array and either ARRAY is an assumed-size
>> 	array of rank DIM or dimension DIM of ARRAY has nonzero extent,
>> 	LBOUND (ARRAY, DIM) has a value equal to the lower bound for
>> 	subscript DIM of ARRAY. Otherwise the result value is 1.
>>
>> We can't tell whether the array is zero-sized, so we can't tell the
>> lbound value.
> 
> So it is only possible to simplify LBOUND if the lower bound is
> equal to one, both for assumed-shape and explicit-shape arrays...
> 
Indeed.

>> As you may want to simplify in the limited scope of the matmul inlining,
>> I'm giving comments about the patch (otherwise you can ignore them):
>>  - No need to check for allocatable or pointer, it should be excluded by
>> as->type == AS_ASSUMED_SHAPE (but does no harm either).
> 
> Actually, no.  You can have assumed-shape allocatable or pointer
> dummy arguments which keep their original lbound; see the subroutine
> 'bar' in the test case.
> 
>>  - Please modify the early return condition:
>>      if (as && (as->type == AS_DEFERRED || as->type == AS_ASSUMED_SHAPE
>> 	        || as->type == AS_ASSUMED_RANK))
>>        return NULL;
>>    and let the existing code do the simplification work.
> 
> That is not part of my patch.
> 
I'm not sure I expressed what I was asking for clearly enough.
Anyway, I may as well submit the requested changes myself.

> So... here is the new patch.  OK for trunk?
> 
Yes, thanks.

Mikael
H.J. Lu May 2, 2015, 10:50 a.m. UTC | #2
On Mon, Apr 27, 2015 at 11:45 AM, Thomas Koenig <tkoenig@netcologne.de> wrote:
> Am 25.04.2015 um 20:12 schrieb Mikael Morin:
>
>> I've double-checked in the standard, and it seems it is not possible to
>> simplify after all:
>>
>>       If ARRAY is a whole array and either ARRAY is an assumed-size
>>       array of rank DIM or dimension DIM of ARRAY has nonzero extent,
>>       LBOUND (ARRAY, DIM) has a value equal to the lower bound for
>>       subscript DIM of ARRAY. Otherwise the result value is 1.
>>
>> We can't tell whether the array is zero-sized, so we can't tell the
>> lbound value.
>
> So it is only possible to simplify LBOUND if the lower bound is
> equal to one, both for assumed-shape and explicit-shape arrays...
> OK.
>
> The attached patch does that, including a test case which catches
> that particular case.
>
>> As you may want to simplify in the limited scope of the matmul inlining,
>> I'm giving comments about the patch (otherwise you can ignore them):
>>  - No need to check for allocatable or pointer, it should be excluded by
>> as->type == AS_ASSUMED_SHAPE (but does no harm either).
>
> Actually, no.  You can have assumed-shape allocatable or pointer
> dummy arguments which keep their original lbound; see the subroutine
> 'bar' in the test case.
>
>>  - Please modify the early return condition:
>>      if (as && (as->type == AS_DEFERRED || as->type == AS_ASSUMED_SHAPE
>>               || as->type == AS_ASSUMED_RANK))
>>        return NULL;
>>    and let the existing code do the simplification work.
>
> That is not part of my patch.
>
>> Or drop the lbound simplification idea, and fetch the lbound "by hand"
>> at matmul inline time.
>
> I will probably do so as a future optimization, but I think that most
> people will see no reason for using different lower bounds, so it is
> OK for the time being to (slightly) pessimize this case.
>
> So... here is the new patch.  OK for trunk?
>
>         Thomas
>
> 2015-04-25  Thomas Koenig  <tkoenig@gcc.gnu.org>
>
>         PR fortran/37131
>         * simplify.c (simplify_bound): Get constant lower bounds of one
>         from array spec for assumed and explicit shape shape arrays if
>         the lower bounds are indeed one.
>
> 2015-04-25  Thomas Koenig  <tkoenig@gcc.gnu.org>
>
>         PR fortran/37131
>         * gfortran.dg/coarray_lib_this_image_2.f90:  Adjust
>         scan pattern.
>         * gfortran.dg/bound_9.f90:  New test case.
>

This caused:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65981
diff mbox

Patch

Index: simplify.c
===================================================================
--- simplify.c	(Revision 222431)
+++ simplify.c	(Arbeitskopie)
@@ -3445,6 +3445,39 @@  simplify_bound (gfc_expr *array, gfc_expr *dim, gf
 
  done:
 
+  /* If the array shape is assumed shape or explicit, we can simplify lbound
+     to 1 if the given lower bound is one because this matches what lbound
+     should return for an empty array.  */
+
+  if (!upper && as && dim && dim->expr_type == EXPR_CONSTANT
+      && (as->type == AS_ASSUMED_SHAPE || as->type == AS_EXPLICIT) 
+      && ref->u.ar.type != AR_SECTION)
+    {
+      /* Watch out for allocatable or pointer dummy arrays, they can have
+	 lower bounds that are not equal to one.  */
+      if (!(array->symtree && array->symtree->n.sym
+	    && (array->symtree->n.sym->attr.allocatable
+		|| array->symtree->n.sym->attr.pointer)))
+	{
+	  unsigned long int ndim;
+	  gfc_expr *lower, *res;
+
+	  ndim = mpz_get_si (dim->value.integer) - 1;
+	  lower = as->lower[ndim];
+	  if (lower->expr_type == EXPR_CONSTANT
+	      && mpz_cmp_si (lower->value.integer, 1) == 0)
+	    {
+	      res = gfc_copy_expr (lower);
+	      if (kind)
+		{
+		  int nkind = mpz_get_si (kind->value.integer);
+		  res->ts.kind = nkind;
+		}
+	      return res;
+	    }
+	}
+    }
+
   if (as && (as->type == AS_DEFERRED || as->type == AS_ASSUMED_SHAPE
 	     || as->type == AS_ASSUMED_RANK))
     return NULL;