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 <paul.richard.tho...@gmail.com> 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 <pa...@gcc.gnu.org> > > 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 <pa...@gcc.gnu.org> > > 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 <n...@cam.ac.uk> 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