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

Reply via email to