Re: [Fortran] Yet more array dependence improvements

[Roger Sayle <roger at eyesopen dot com>]

Hi Steve,

On Thu, 9 Mar 2006, Steve Kargl wrote:
> I have a few questions, and will read through this on Saturday.
>
> > +       if (gfc_dep_compare_expr (e1->value.op.op1, e2->value.op.op1) == 0
> > + 	  && gfc_dep_compare_expr (e1->value.op.op2, e2->value.op.op2) == 0)
> > + 	return 0;
>
> This deals with "n + 1" == "n + 1"
>
> > +       /* TODO Handle commutative binary operators here?  */
> > +       return -2;
>
> Is this comment for "n + 1" == "1 + n"?  Won't the following work?
>
>    if (gfc_dep_compare_expr (e1->value.op.op1, e2->value.op.op2) == 0
>  	&& gfc_dep_compare_expr (e1->value.op.op2, e2->value.op.op1) == 0)
>  	return 0;

Yes, this is exactly the test to handle commutative operators.
The only reason I didn't add support for this in this patch, is
the need to audit and explicitly list the commutative operators.
i.e. your test above works fine for INTRINSIC_PLUS and INTRINSIC_EQ,
but isn't applicable for INTRINSIC_DIVIDE and INTRINSIC_MINUS.
I suspect the number of places where we benefit from identifying
that "n+1" is the same as "1+n" is fairly limited.  However, it
might be nice if someone added this as a follow-up patch.

Of course, in the general case, this routine would end up
looking very similar to fold-const.c's operand_equal_p.
We'd be able to strip INTRINSIC_PARENTHESIS from either argument
and notice that "x INTRINSIC_LT y" is the same as the gfc_expr
"y INTRSINSIC_GE x", etc...  One might argue that a reasonable
implementation is to expand to a middle-end tree, and then
call operand_equal_p!  However, as a first cut, and given that
this is currently predominantly used in array index comparison,
testing for "lexical" equality seems good enough.



> > +       /* We should list the "constant" intrinsic functions.  Those
> > + 	 without side-effects that provide equal results given equal
> > + 	 argument lists.  */
>
> I don't understand this comment.  Intrinsic functions in Fortran 95
> are pure, so by definition none of the intrinsic functions have
> side-effects.  Is this comment meant for the intrinsics that gfortran
> inlines/turns-into __convert_* fucntions?

Hmm, this wasn't clear from the gfc_generic_isym_id enumeration.
For example, the symbols GFC_ISYM_CHDIR, GFC_ISYM_CTIME,
GFC_ISYM_FGETC, GFC_ISYM_MALLOC, GFC_ISYM_SYSTEM, etc... give
the impression that gfortran's intrinsic functions can have
quite significant side-effects.

Like the INTRINSIC_* audit above, it would be great if a gfortran
maintainer could extend the list of "const" intrinsic functions
given in this patch.  For the time being, and to address the
immediate need of gfc_dep_compare_expr, it seems sufficient
to handle just the conversions, which are clearly "const".

It might be worthwhile analysing the array index/bounds expressions
encountered in SPEC and polyhedron, but I suspect that idioms such
as "a(floor(x),:) = ..." aren't frequent enough to deserve special
handling.  The implicit conversions in "a(i,:) = a(i,:) + 1", on the
other hand, potentially kill performance on 64-bit platforms when
compared to 32-bit platforms.


I hope this answers your questions.  Thanks for taking a look
at this patch.

Roger
--