Dear All,
Dominique has just flagged up a slight technical problem with the patch...
it's not for this PR :-( Please find the correct patch attached.
Paul
On 8 February 2015 at 12:42, Paul Richard Thomas
<[email protected]> wrote:
> Dear All,
>
> This came up at
> https://groups.google.com/forum/#!topic/comp.lang.fortran/TvVY5j3GPmg
>
> gfortran produces wrong result from:
>
> PROGRAM Main
> INTEGER :: i, index(5) = (/ (i, i = 1,5) /)
> REAL :: array(5) = (/ (i+0.0, i = 1,5) /)
> array = Fred(index,array)
> PRINT *, array
> CONTAINS
> ELEMENTAL FUNCTION Fred (n, x)
> REAL :: Fred
> INTEGER, INTENT(IN) :: n
> REAL, INTENT(IN) :: x
> ! In general, this would be in an external procedure
> Fred = x+SUM(array(:n-1))+SUM(array(n+1:))
> END FUNCTION Fred
> END PROGRAM Main
>
> outputs
> 15.0000000 29.0000000 56.0000000 109.000000
> 214.000000
> when result should be
> 5*15.0
>
> A temporary should be produced for array = Fred(index, array). See the
> clf thread for the reasoning.
>
> In a nutshell, the reason is:
> The execution of the assignment shall have the same effect as
> if the evaluation of expr and the evaluation of all expressions
> in variable occurred before any portion of the variable is
> defined by the assignment. The evaluation of expressions within
> variable shall neither affect nor be affected by the evaluation
> of expr.
>
> Clearly, the above code violates this requirement because of the
> references to 'array' in 'Fred'. I think that we will have to provide
> an attribute that marks up array valued elemental functions that have
> any external array references and provide a temporary for assignment
> from one of these. Clearly something less brutal could be done, such
> as attaching a list of external arrays (to the elemental function,
> that is) to the symbol of the elemental function and comparing them
> with the lhs of an assignment. However, this works and has no
> perceivable effect on Polyhedron timings.
>
> I will change the name of the flags to potentially_aliasing.
>
> Bootstrapped and regtested on FC21/x86_64 - OK for trunk?
>
> Paul
>
> 2015-02-08 Paul Thomas <[email protected]>
>
> PR fortran/64952
> * gfortran.h : Add 'potentially_aliased' field to symbol_attr.
> * trans.h : Add 'potentially_aliased' field to gfc_ss_info.
> * resolve.c (resolve_variable): Mark elemental function symbol
> as 'potentially_aliased' if it has an array reference from
> outside its own namespace.
> * trans-array.c (gfc_conv_resolve_dependencies): If any ss is
> marked as 'potentially_aliased' generate a temporary.
> (gfc_walk_function_expr): If the function is marked as
> 'potentially_aliased', likewise mark the head gfc_ss.
>
> 2015-02-08 Paul Thomas <[email protected]>
>
> PR fortran/64952
> * gfortran.dg/finalize_28.f90: New test
--
Outside of a dog, a book is a man's best friend. Inside of a dog it's
too dark to read.
Groucho Marx
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;
+
/* 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;
+
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;
+ }
/* Scalar functions are OK as these are evaluated outside the scalarization
loop. Pass back and let the caller deal with it. */
Index: gcc/testsuite/gfortran.dg/elemental_dependency_4.f90
===================================================================
*** gcc/testsuite/gfortran.dg/elemental_dependency_4.f90 (revision 0)
--- gcc/testsuite/gfortran.dg/elemental_dependency_4.f90 (working copy)
***************
*** 0 ****
--- 1,23 ----
+ ! { dg-do run }
+ !
+ ! Tests the fix for PR64952, in which the assignment to 'array' should
+ ! have generated a temporary because of the references to the lhs in
+ ! the function 'Fred'.
+ !
+ ! Contributed by Nick Maclaren <[email protected]> on clf
+ ! https://groups.google.com/forum/#!topic/comp.lang.fortran/TvVY5j3GPmg
+ !
+ PROGRAM Main
+ INTEGER :: i, index(5) = (/ (i, i = 1,5) /)
+ REAL :: array(5) = (/ (i+0.0, i = 1,5) /)
+ array = Fred(index,array)
+ If (any (array .ne. array(1))) call abort
+ CONTAINS
+ ELEMENTAL FUNCTION Fred (n, x)
+ REAL :: Fred
+ INTEGER, INTENT(IN) :: n
+ REAL, INTENT(IN) :: x
+ ! In general, this would be in an external procedure
+ Fred = x+SUM(array(:n-1))+SUM(array(n+1:))
+ END FUNCTION Fred
+ END PROGRAM Main
