[Bug fortran/64952] New: Missing temporary in assignment from elemental function

[Bug fortran/64952] New: Missing temporary in assignment from elemental function

[pault at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64952

            Bug ID: 64952
           Summary: Missing temporary in assignment from elemental
                    function
           Product: gcc
           Version: 5.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: fortran
          Assignee: unassigned at gcc dot gnu.org
          Reporter: pault at gcc dot gnu.org

See 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 that won the day (I had another point of view) 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. Fortunately, it only
affects contained functions so that we do not have to carry it across when
using modules.

Paul

[Bug fortran/64952] Missing temporary in assignment from elemental function

[burnus at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64952

--- Comment #1 from Tobias Burnus <burnus at gcc dot gnu.org> ---
Technical, it is a bit similar to:

module m
  integer :: i
contains
  pure subroutine f(x)
    integer, intent(inout) :: x
    x = 2*x + i
  end subroutine
end module m

which doesn't modify "i" but still feels a bit odd as it accesses an
nonexplicitly passed variable. Thus, we may need to also take care of:

   array = elemental(array) * pure_function()

where the pure_function accesses the array, e.g. as "sum(array)". Thus, not
only "elemental" has to be checked for such an access, but also nonelemental
pure functions in the surrounding. On the other hand, it would be probably
faster to do:
  tmp = pure_function()
  array = elemental(array) * tmp
which also avoids this problem. (The FE optimization might do this, but it
should also correctly work with -O0.)

[Bug fortran/64952] Missing temporary in assignment from elemental function

[mikael at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64952

Mikael Morin <mikael at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mikael at gcc dot gnu.org

--- Comment #2 from Mikael Morin <mikael at gcc dot gnu.org> ---
(In reply to Paul Thomas from comment #0)
> Fortunately, it only affects contained functions so that we do not have to
> carry it across when using modules.
> 
Don't we have to?
Wouldn't it be the same if ARRAY and FRED were use-associated symbols, or
if FRED was accessing ARRAY through use-association instead of
host-association?

(In reply to Tobias Burnus from comment #1)
> On the other hand, it would be probably
> faster to do:
>   tmp = pure_function()
>   array = elemental(array) * tmp
> which also avoids this problem.

As far as I know, the scalarizer does this already (in both cases where
pure_function is scalar and non-scalar).


By the way, a temporary is also necessary for
    array = Fred(index, other_array)
that is, array need not be as argument to Fred.

[Bug fortran/64952] Missing temporary in assignment from elemental function

[pault at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64952

Paul Thomas <pault at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2015-02-07
                 CC|                            |pault at gcc dot gnu.org
           Assignee|unassigned at gcc dot gnu.org      |pault at gcc dot gnu.org
     Ever confirmed|0                           |1

--- Comment #3 from Paul Thomas <pault at gcc dot gnu.org> ---
Created attachment 34694
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=34694&action=edit
Patch for the PR

This is a somewhat conservative fix - doubtless somebody feeling strong enough
could add a list of violating symbols to be compared with the lvalue
expression.

However, come hell or high water, this generates correct code!

Bootstraps and regtests on FC21/x86_64.

Paul

[Bug fortran/64952] Missing temporary in assignment from elemental function

[mikael at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64952

--- Comment #4 from Mikael Morin <mikael at gcc dot gnu.org> ---
Hello Paul,

setting potentially_aliased should be done inside
gfc_walk_elemental_function_args, as the ss argument may be returned
unmodified.
In fact, I think it's better to do all the trans-array.c code inside
gfc_conv_resolve_dependencies without adding the gfc_ss_info flag.

There is also the case of typebound procedures and procedure pointer
components,
for which we should generate a temporary in any case.

I think this case is something that was overlooked by the standard commitee
when they introduced the PURE attribute.  Maybe they can provide some kind of
"REALLY_PURE" attribute (or PURE ELEMENTAL, different from regular ELEMENTAL)
that avoids generating temporaries everywhere?
Or maybe the function Fred should bee IMPURE ELEMENTAL?

Anyway, I think we should not rush to fix this before we are sure that the
standard committee really expects temporaries (almost) everywhere array
elemental functions are involved.

[Bug fortran/64952] Missing temporary in assignment from elemental function

[mikael at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64952

--- Comment #5 from Mikael Morin <mikael at gcc dot gnu.org> ---
Author: mikael
Date: Mon Mar 23 07:53:31 2015
New Revision: 221586

URL: https://gcc.gnu.org/viewcvs?rev=221586&root=gcc&view=rev
Log:
2015-03-23  Paul Thomas  <pault@gcc.gnu.org>
        Mikael Morin  <mikael@gcc.gnu.org>

    PR fortran/64952
fortran/
    * gfortran.h (struct symbol_attribute) : New field
    'array_outer_dependency'.
    * trans.h (struct gfc_ss_info): New field 'array_outer_dependency'.
    * module.c (enum ab_attribute): New value AB_ARRAY_OUTER_DEPENDENCY.
    (attr_bits): Append same value to initializer.
    (mio_symbol_attribute): Handle 'array_outer_dependency' attr
    in module read and write.
    * resolve.c (update_current_proc_outer_array_dependency): New function.
    (resolve_function, resolve_call): Add code to update current procedure's
    'array_outer_dependency' attribute.
    (resolve_variable): Mark current procedure with attribute
    array_outer_dependency if the variable is an array coming from outside
    the current namespace.
    (resolve_fl_procedure): Mark a procedure without body with attribute
    'array_outer_dependency'.
    * trans-array.c (gfc_conv_resolve_dependencies): If any ss is
    marked as 'array_outer_dependency' generate a temporary.
    (gfc_walk_function_expr): If the function may reference external arrays,
    mark the head gfc_ss with flag 'array_outer_dependency'.
testsuite/
    * gfortran.dg/elemental_dependency_4.f90: New.
    * gfortran.dg/elemental_dependency_5.f90: New.


Added:
    trunk/gcc/testsuite/gfortran.dg/elemental_dependency_4.f90
    trunk/gcc/testsuite/gfortran.dg/elemental_dependency_5.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/gfortran.h
    trunk/gcc/fortran/module.c
    trunk/gcc/fortran/resolve.c
    trunk/gcc/fortran/trans-array.c
    trunk/gcc/fortran/trans.h
    trunk/gcc/testsuite/ChangeLog

[Bug fortran/64952] Missing temporary in assignment from elemental function

[paul.richard.thomas at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64952

--- Comment #6 from paul.richard.thomas at gmail dot com <paul.richard.thomas at gmail dot com> ---
Dear Mikael,

The pureness is also confused by the C pure, which is whiter than
white pure. I agree with your last remark about the standards
committee needing to reflect on this.

Thanks for finishing the job. Will you post a message on the clf
thread, or would you like me to do it?

Paul

On 8 February 2015 at 14:00, mikael at gcc dot gnu.org
<gcc-bugzilla@gcc.gnu.org> wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64952
>
> --- Comment #4 from Mikael Morin <mikael at gcc dot gnu.org> ---
> Hello Paul,
>
> setting potentially_aliased should be done inside
> gfc_walk_elemental_function_args, as the ss argument may be returned
> unmodified.
> In fact, I think it's better to do all the trans-array.c code inside
> gfc_conv_resolve_dependencies without adding the gfc_ss_info flag.
>
> There is also the case of typebound procedures and procedure pointer
> components,
> for which we should generate a temporary in any case.
>
> I think this case is something that was overlooked by the standard commitee
> when they introduced the PURE attribute.  Maybe they can provide some kind of
> "REALLY_PURE" attribute (or PURE ELEMENTAL, different from regular ELEMENTAL)
> that avoids generating temporaries everywhere?
> Or maybe the function Fred should bee IMPURE ELEMENTAL?
>
> Anyway, I think we should not rush to fix this before we are sure that the
> standard committee really expects temporaries (almost) everywhere array
> elemental functions are involved.
>
> --
> You are receiving this mail because:
> You are on the CC list for the bug.
> You are the assignee for the bug.
> You reported the bug.

[Bug fortran/64952] Missing temporary in assignment from elemental function

[mikael at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64952

--- Comment #7 from Mikael Morin <mikael at gcc dot gnu.org> ---
(In reply to paul.richard.thomas@gmail.com from comment #6)
> Thanks for finishing the job.
I have yet to fix 4.9 as well, as you suggested. In a week or so.

> Will you post a message on the clf
> thread, or would you like me to do it?
> 
It's better if you do it, I don't follow clf discussions.

[Bug fortran/64952] Missing temporary in assignment from elemental function

[ktkachov at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64952

ktkachov at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ktkachov at gcc dot gnu.org

--- Comment #8 from ktkachov at gcc dot gnu.org ---
(In reply to Mikael Morin from comment #5)
> Author: mikael
> Date: Mon Mar 23 07:53:31 2015
> New Revision: 221586
> 
> URL: https://gcc.gnu.org/viewcvs?rev=221586&root=gcc&view=rev
> Log:
> 2015-03-23  Paul Thomas  <pault@gcc.gnu.org>
> 	    Mikael Morin  <mikael@gcc.gnu.org>
> 
> 	PR fortran/64952
> fortran/
> 	* gfortran.h (struct symbol_attribute) : New field
> 	'array_outer_dependency'.
> 	* trans.h (struct gfc_ss_info): New field 'array_outer_dependency'.
> 	* module.c (enum ab_attribute): New value AB_ARRAY_OUTER_DEPENDENCY.
> 	(attr_bits): Append same value to initializer.
> 	(mio_symbol_attribute): Handle 'array_outer_dependency' attr
> 	in module read and write.
> 	* resolve.c (update_current_proc_outer_array_dependency): New function.
> 	(resolve_function, resolve_call): Add code to update current procedure's
> 	'array_outer_dependency' attribute.
> 	(resolve_variable): Mark current procedure with attribute
> 	array_outer_dependency if the variable is an array coming from outside
> 	the current namespace.
> 	(resolve_fl_procedure): Mark a procedure without body with attribute
> 	'array_outer_dependency'.
> 	* trans-array.c (gfc_conv_resolve_dependencies): If any ss is
> 	marked as 'array_outer_dependency' generate a temporary.
> 	(gfc_walk_function_expr): If the function may reference external arrays,
> 	mark the head gfc_ss with flag 'array_outer_dependency'.
> testsuite/
> 	* gfortran.dg/elemental_dependency_4.f90: New.
> 	* gfortran.dg/elemental_dependency_5.f90: New.
> 
> 
> Added:
>     trunk/gcc/testsuite/gfortran.dg/elemental_dependency_4.f90
>     trunk/gcc/testsuite/gfortran.dg/elemental_dependency_5.f90
> Modified:
>     trunk/gcc/fortran/ChangeLog
>     trunk/gcc/fortran/gfortran.h
>     trunk/gcc/fortran/module.c
>     trunk/gcc/fortran/resolve.c
>     trunk/gcc/fortran/trans-array.c
>     trunk/gcc/fortran/trans.h
>     trunk/gcc/testsuite/ChangeLog

With this commit I can no longer build 481.wrf for aarch64.
I get:
module_ra_rrtm.fppized.f90:4438:33:

       DIMENSION H2OREF(59),CO2REF(59), ETAREF(10)                              
                                 1
Error: Different shape for array assignment at (1) on dimension 1 (59 and 7)

Anything that can be done?
>From gcc-bugs-return-481357-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org Tue Mar 24 09:49:59 2015
Return-Path: <gcc-bugs-return-481357-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org>
Delivered-To: listarch-gcc-bugs@gcc.gnu.org
Received: (qmail 65484 invoked by alias); 24 Mar 2015 09:49:58 -0000
Mailing-List: contact gcc-bugs-help@gcc.gnu.org; run by ezmlm
Precedence: bulk
List-Id: <gcc-bugs.gcc.gnu.org>
List-Archive: <http://gcc.gnu.org/ml/gcc-bugs/>
List-Post: <mailto:gcc-bugs@gcc.gnu.org>
List-Help: <mailto:gcc-bugs-help@gcc.gnu.org>
Sender: gcc-bugs-owner@gcc.gnu.org
Delivered-To: mailing list gcc-bugs@gcc.gnu.org
Received: (qmail 65424 invoked by uid 48); 24 Mar 2015 09:49:54 -0000
From: "ebotcazou at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug tree-optimization/65519] [5 regression] unable to coalesce ssa_names 2 and 87 which are marked as MUST COALESCE
Date: Tue, 24 Mar 2015 10:06:00 -0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: changed
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gcc
X-Bugzilla-Component: tree-optimization
X-Bugzilla-Version: 5.0
X-Bugzilla-Keywords:
X-Bugzilla-Severity: normal
X-Bugzilla-Who: ebotcazou at gcc dot gnu.org
X-Bugzilla-Status: ASSIGNED
X-Bugzilla-Priority: P3
X-Bugzilla-Assigned-To: ebotcazou at gcc dot gnu.org
X-Bugzilla-Target-Milestone: 5.0
X-Bugzilla-Flags:
X-Bugzilla-Changed-Fields:
Message-ID: <bug-65519-4-A1y23V8nLj@http.gcc.gnu.org/bugzilla/>
In-Reply-To: <bug-65519-4@http.gcc.gnu.org/bugzilla/>
References: <bug-65519-4@http.gcc.gnu.org/bugzilla/>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/
Auto-Submitted: auto-generated
MIME-Version: 1.0
X-SW-Source: 2015-03/txt/msg02501.txt.bz2
Content-length: 1139

https://gcc.gnu.org/bugzilla/show_bug.cgi?ide519

--- Comment #9 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
> gimple-fold works like fold - it tries to re-simplify what you feed it, but
> only the outermost level (so it doesn't really recurse).  Still I think this
> bug needs changing code-generation like with the attached patch which I am
> testing right now.

I see, thanks.

> I always forget how to force SJLJ EH on x86_64-linux for ada so I didn't
> manage to reproduce the issue or check if the patch fixes it ...

In a build tree:
eric@polaris:~/build/gcc/native> cp gcc/ada/rts/system.ads .
Edit system.ads and change ZCX_By_Default to False.
Then compile the reduced testcase:
eric@polaris:~/build/gcc/native> gcc/xgcc -Bgcc -S p.ads -O2

Unable to coalesce ssa_names 2 and 87 which are marked as MUST COALESCE.
I.3_2(ab) and  I.3_87(ab)
+===========================GNAT BUG DETECTED==============================+
| 5.0.0 20150316 (experimental) [trunk revision 221457] (x86_64-suse-linux) GCC
error:|
| SSA corruption                                                           |
| Error detected around p.ads:6:9

[Bug fortran/64952] Missing temporary in assignment from elemental function

[dominiq at lps dot ens.fr]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64952

--- Comment #9 from Dominique d'Humieres <dominiq at lps dot ens.fr> ---
Duplicate of pr65532?

[Bug fortran/64952] Missing temporary in assignment from elemental function

[mikael at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64952

--- Comment #10 from Mikael Morin <mikael at gcc dot gnu.org> ---
(In reply to Dominique d'Humieres from comment #9)
> Duplicate of pr65532?

Rather cause of pr65532.  Only comment #8 is a duplicate.


(In reply to Mikael Morin from comment #7)
> (In reply to paul.richard.thomas@gmail.com from comment #6)
> > Thanks for finishing the job.
> I have yet to fix 4.9 as well, as you suggested. In a week or so.
>  
The patch isn't as safe as I thought, so I think I won't backport it after all.

[Bug fortran/64952] Missing temporary in assignment from elemental function

[mikael at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64952

--- Comment #11 from Mikael Morin <mikael at gcc dot gnu.org> ---
Author: mikael
Date: Wed Mar 25 10:15:46 2015
New Revision: 221657

URL: https://gcc.gnu.org/viewcvs?rev=221657&root=gcc&view=rev
Log:
Fix regression introduced at revision 221586.

    PR fortran/64952
    PR fortran/65532
fortran/
    * gfortran.h (struct gfc_namespace): New field 'types_resolved'.
    * resolve.c (resolve_types): Return early if field 'types_resolved'
    is set.  Set 'types_resolved' at the end.
testsuite/
    * gfortran.dg/data_initialized_3.f90: New.


Added:
    trunk/gcc/testsuite/gfortran.dg/data_initialized_3.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/gfortran.h
    trunk/gcc/fortran/resolve.c
    trunk/gcc/testsuite/ChangeLog

[Bug fortran/64952] Missing temporary in assignment from elemental function

[mikael at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64952

Mikael Morin <mikael at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #12 from Mikael Morin <mikael at gcc dot gnu.org> ---
Fixed for 5.0.  Closing.