Dear Mikael,
Thank you very much for the review. You raise some points that I had
thought about and others that I hadn't. I also realised that such
things as blocks, within the elemental function would through the fix
as well. I'll defer doing anything with it until tomorrow night.
I reason that there is always going to be an 'ss', although I should
check that it is not gfc_ss_terminator, and that it does not matter
which one is flagged. I should add a comment to that effect; it's not
quite as hackish as it looks, methinks.
I will be back!
Paul
On 8 February 2015 at 18:27, Mikael Morin <mikael.mo...@sfr.fr> wrote:
> Hello Paul,
>
> comments below
>
> Le 08/02/2015 16:24, Paul Richard Thomas a écrit :
>>
>> Index: gcc/fortran/gfortran.h
>> ===================================================================
>> *** gcc/fortran/gfortran.h (revision 220482)
>> --- gcc/fortran/gfortran.h (working copy)
>> *************** typedef struct
>> *** 789,794 ****
>> --- 789,798 ----
>> cannot alias. Note that this is zero for PURE procedures. */
>> unsigned implicit_pure:1;
>>
>> + /* This set for an elemental function that contains expressions for
>> + arrays coming from outside its namespace. */
>> + unsigned potentially_aliased:1;
>> +
> aliased is more something about pointers, so how about naming it
> something like array_outer_dependency? Anyway, that's minor.
>
> I wonder whether we should negate the meaning, that is set the flag if
> there is no external dependency.
> If we can get the conditions to set it exhaustively right, both are
> equivalent. Otherwise... maybe not.
>
>> /* This is set if the subroutine doesn't return. Currently, this
>> is only possible for intrinsic subroutines. */
>> unsigned noreturn:1;
>> Index: gcc/fortran/trans.h
>> ===================================================================
>> *** gcc/fortran/trans.h (revision 220481)
>> --- gcc/fortran/trans.h (working copy)
>> *************** typedef struct gfc_ss_info
>> *** 226,231 ****
>> --- 226,235 ----
>> /* Suppresses precalculation of scalars in WHERE assignments. */
>> unsigned where:1;
>>
>> + /* Signals that an array argument of an elemental function might be
>> aliased,
>> + thereby generating a temporary in assignments. */
>> + unsigned potentially_aliased:1;
>> +
>> /* Tells whether the SS is for an actual argument which can be a NULL
>> reference. In other words, the associated dummy argument is OPTIONAL.
>> Used to handle elemental procedures. */
>> Index: gcc/fortran/resolve.c
>> ===================================================================
>> *** gcc/fortran/resolve.c (revision 220481)
>> --- gcc/fortran/resolve.c (working copy)
>> *************** resolve_variable (gfc_expr *e)
>> *** 5054,5059 ****
>> --- 5054,5067 ----
>> && gfc_current_ns->parent->parent == sym->ns)))
>> sym->attr.host_assoc = 1;
>>
>> + if (sym->attr.dimension
>> + && (sym->ns != gfc_current_ns
>> + || sym->attr.use_assoc
>> + || sym->attr.in_common)
>> + && gfc_elemental (NULL)
>> + && gfc_current_ns->proc_name->attr.function)
>> + gfc_current_ns->proc_name->attr.potentially_aliased = 1;
> I would expect the flag to also be copied between procedures in some
> cases; namely if A calls B, and B has the flag, then A has the flag.
> There is also the case of external procedures (for which the flag is not
> known -> assume the worst)
>
>> +
>> resolve_procedure:
>> if (t && !resolve_procedure_expression (e))
>> t = false;
>> Index: gcc/fortran/trans-array.c
>> ===================================================================
>> *** gcc/fortran/trans-array.c (revision 220482)
>> --- gcc/fortran/trans-array.c (working copy)
>> *************** gfc_conv_resolve_dependencies (gfc_loopi
>> *** 4391,4396 ****
>> --- 4391,4402 ----
>> {
>> ss_expr = ss->info->expr;
>>
>> + if (ss->info->potentially_aliased)
>> + {
>> + nDepend = 1;
>> + break;
>> + }
>> +
>> if (ss->info->type != GFC_SS_SECTION)
>> {
>> if (flag_realloc_lhs
>> *************** gfc_walk_function_expr (gfc_ss * ss, gfc
>> *** 9096,9104 ****
>> /* Walk the parameters of an elemental function. For now we always pass
>> by reference. */
>> if (sym->attr.elemental || (comp && comp->attr.elemental))
>> ! return gfc_walk_elemental_function_args (ss,
>> expr->value.function.actual,
>> gfc_get_proc_ifc_for_expr (expr),
>> GFC_SS_REFERENCE);
>>
>> /* Scalar functions are OK as these are evaluated outside the
>> scalarization
>> loop. Pass back and let the caller deal with it. */
>> --- 9102,9114 ----
>> /* Walk the parameters of an elemental function. For now we always pass
>> by reference. */
>> if (sym->attr.elemental || (comp && comp->attr.elemental))
>> ! {
>> ! ss = gfc_walk_elemental_function_args (ss,
>> expr->value.function.actual,
>> gfc_get_proc_ifc_for_expr (expr),
>> GFC_SS_REFERENCE);
>> + if (sym->attr.potentially_aliased)
>> + ss->info->potentially_aliased = 1;
>> + }
>
> This is somewhat hackish, potentially_aliased is a global thing, not
> specific to SS, and this may end up marking gfc_ss_terminator as
> potentiallly_aliased for example, but I don't see any other obvious way
> to do it, so it's OK I guess.
>
> Anyway, the comp && comp->attr.elemental part of the if should be
> handled too (always set the flag in that case?). I actually wonder why
> it works without.
>
> I attach a few variants of the testcase, which don't work yet.
>
> Mikael
>
--
Outside of a dog, a book is a man's best friend. Inside of a dog it's
too dark to read.
Groucho Marx
