[Bug fortran/67170] New: PRE can't hoist out a readonly argument

[Bug fortran/67170] New: PRE can't hoist out a readonly argument

[mliska at suse dot cz]

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

            Bug ID: 67170
           Summary: PRE can't hoist out a readonly argument
           Product: gcc
           Version: 6.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: fortran
          Assignee: unassigned at gcc dot gnu.org
          Reporter: mliska at suse dot cz
  Target Milestone: ---

In attached Fortran source file, we are unable to hoist out the single function
argument. The argument is reloaded at the end of loop.

Thanks,
Martin

[Bug fortran/67170] PRE can't hoist out a readonly argument

[mliska at suse dot cz]

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

--- Comment #1 from Martin Liška <mliska at suse dot cz> ---
Created attachment 36160
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=36160&action=edit
Test case
>From gcc-bugs-return-494489-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org Mon Aug 10 11:31:00 2015
Return-Path: <gcc-bugs-return-494489-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org>
Delivered-To: listarch-gcc-bugs@gcc.gnu.org
Received: (qmail 28417 invoked by alias); 10 Aug 2015 11:31:00 -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 28393 invoked by uid 48); 10 Aug 2015 11:30:56 -0000
From: "ncm at cantrip dot org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug tree-optimization/67153] [5/6 Regression] integer optimizations 53% slower than std::bitset<>
Date: Mon, 10 Aug 2015 11:31: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.1.1
X-Bugzilla-Keywords: missed-optimization
X-Bugzilla-Severity: normal
X-Bugzilla-Who: ncm at cantrip dot org
X-Bugzilla-Status: UNCONFIRMED
X-Bugzilla-Resolution:
X-Bugzilla-Priority: P3
X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org
X-Bugzilla-Target-Milestone: 5.3
X-Bugzilla-Flags:
X-Bugzilla-Changed-Fields:
Message-ID: <bug-67153-4-4Hb61EpI7J@http.gcc.gnu.org/bugzilla/>
In-Reply-To: <bug-67153-4@http.gcc.gnu.org/bugzilla/>
References: <bug-67153-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-08/txt/msg00631.txt.bz2
Content-length: 635

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

--- Comment #4 from ncm at cantrip dot org ---
Also fails 5.2.1 (Debian 5.2.1--15) 5.2.1 20150808
As noted, the third version of the program, using bitset but not using
lambdas, is as slow as the version using unsigned int -- even when built
using gcc-4.9.  (Recall the int version and the first bitset version
run fast when built with gcc-4.9.)

Confirmed that on Westmere, compiled -march=native, all versions
run about the same speed with all versions of the compiler reported,
and this runtime is about the same as the slow Haswell speed despite
the very different clock rate.

[Bug fortran/67170] PRE can't hoist out a readonly argument

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2015-08-11
     Ever confirmed|0                           |1

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
Ah.

  # USE = nonlocal escaped { D.3438 }
  # CLB = nonlocal escaped
  foo (&D.3438);

As it clobbers nonlocal it clobbers the incoming argument :/  Because of course
as the argument is pointing to possibly global memory it can access it via
other means than through the argument - and the function attribute in use
(".r") just means accesses through the first argument do not modify the
contents pointed to.

I think Fortran guarantees that things passed as argument are not modified
in other ways (or it makes that undefined).  Not sure about called functions
though.

Now.  In principle I think C says that with

void foo (int * __restrict p)
{
 ...
 bar ();
 ...
}

bar () can only access *p through the use of p or a derived pointer.  So


int x;
bar()
{
  x = 1;
}
main()
{
  foo (&x); 
}

would invoke undefined behavior.

Of course our restrict implementation isn't able to capture that (calls
don't have a clique/base which they would get from the "memory" passed
as reference arguments).

So - confirmed.  But no easy way out :(

Old idea about special-casing recursive calls also exists but without clear
idea of what would be legal special alias answers here.

[Bug fortran/67170] PRE can't hoist out a readonly argument

[rguenth at gcc dot gnu.org]

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

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
Ok, the argument may go like "foo (&D.3438) may not modify *arg_29(D)
because then the fnspec on foo would be incorrect - *arg_29(D) would be
modified".

Untested patch (works for the testcase):

Index: gcc/tree-ssa-alias.c
===================================================================
--- gcc/tree-ssa-alias.c        (revision 226807)
+++ gcc/tree-ssa-alias.c        (working copy)
@@ -2153,6 +2153,33 @@ call_may_clobber_ref_p_1 (gcall *call, a
        return false;
     }

+  /* If the base of an indirect access is a parameter which by means
+     of the fnspec of ourselves not clobbered by us then it is surely
+     not modified by calls we do.  */
+  tree base_ptr;
+  tree fnspec;
+  if (TREE_CODE (base) == MEM_REF
+      && (base_ptr = TREE_OPERAND (base, 0))
+      && TREE_CODE (base_ptr) == SSA_NAME
+      && SSA_NAME_IS_DEFAULT_DEF (base_ptr)
+      && SSA_NAME_VAR (base_ptr)
+      && TREE_CODE (SSA_NAME_VAR (base_ptr)) == PARM_DECL
+      && (fnspec = lookup_attribute ("fn spec",
+                                    TYPE_ATTRIBUTES (TREE_TYPE
(cfun->decl)))))
+    {
+      unsigned i = 0;
+      tree arg;
+      for (arg = DECL_ARGUMENTS (cfun->decl);
+          arg && arg != SSA_NAME_VAR (base_ptr); arg = DECL_CHAIN (arg), ++i)
+       ;
+      gcc_assert (arg == SSA_NAME_VAR (base_ptr));
+      fnspec = TREE_VALUE (TREE_VALUE (fnspec));
+      if ((unsigned) TREE_STRING_LENGTH (fnspec) > i + 1
+         && (TREE_STRING_POINTER (fnspec)[i + 1] == 'R'
+             || TREE_STRING_POINTER (fnspec)[i + 1] == 'r'))
+       return false;
+    }
+
   /* Check if the base variable is call-clobbered.  */
   if (DECL_P (base))
     return pt_solution_includes (gimple_call_clobber_set (call), base);


of course this asks for the argument being marked somehow to avoid the
linear search for its index.  It also requires some thinking on if and
when derived pointers (from the argument) allow similar handling.

A way "simpler" approach would be to change code generation by the Frontend
for scalar pass-by-reference intent-in arguments to load such parameters
at the beginning of the function and further use that load result for
all references to that parameter.

Martin, can you check if the above solves the RA issue it was blocking?

[Bug fortran/67170] PRE can't hoist out a readonly argument

[rguenth at gcc dot gnu.org]

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

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
Corrected variant, we hit the assert I put in the first one.

Index: gcc/tree-ssa-alias.c
===================================================================
--- gcc/tree-ssa-alias.c        (revision 226852)
+++ gcc/tree-ssa-alias.c        (working copy)
@@ -2153,6 +2153,37 @@ call_may_clobber_ref_p_1 (gcall *call, a
        return false;
     }

+  /* If the base of an indirect access is a parameter which by means
+     of the fnspec of ourselves not clobbered by us then it is surely
+     not modified by calls we do.  */
+  tree base_ptr;
+  tree fnspec;
+  if (TREE_CODE (base) == MEM_REF
+      && (base_ptr = TREE_OPERAND (base, 0))
+      && TREE_CODE (base_ptr) == SSA_NAME
+      && SSA_NAME_IS_DEFAULT_DEF (base_ptr)
+      && SSA_NAME_VAR (base_ptr)
+      && TREE_CODE (SSA_NAME_VAR (base_ptr)) == PARM_DECL
+      && (fnspec = lookup_attribute ("fn spec",
+                                    TYPE_ATTRIBUTES (TREE_TYPE
(cfun->decl)))))
+    {
+      unsigned i = 0;
+      tree arg;
+      for (arg = DECL_ARGUMENTS (cfun->decl);
+          arg && arg != SSA_NAME_VAR (base_ptr); arg = DECL_CHAIN (arg), ++i)
+       ;
+      /* The static chain is a PARM_DECL as well for example so we may
+        not be able to find base_ptr in the list of arguments.  */
+      if (arg == SSA_NAME_VAR (base_ptr))
+       {
+         fnspec = TREE_VALUE (TREE_VALUE (fnspec));
+         if ((unsigned) TREE_STRING_LENGTH (fnspec) > i + 1
+             && (TREE_STRING_POINTER (fnspec)[i + 1] == 'R'
+                 || TREE_STRING_POINTER (fnspec)[i + 1] == 'r'))
+           return false;
+       }
+    }
+
   /* Check if the base variable is call-clobbered.  */
   if (DECL_P (base))
     return pt_solution_includes (gimple_call_clobber_set (call), base);

[Bug fortran/67170] PRE can't hoist out a readonly argument

[rguenth at gcc dot gnu.org]

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

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
Ok to make the alias-oracle approach more "scalable" we'd need to cache the
fn spec parameter/result bits in the functions PARM/RESULT_DECLs
(thus in tree_decl_common where we have conveniently 24bits left to put
in EAF_* and ERF_* flags).

Now fnspec is a type attribute so I'm not sure we can catch all cases
in the attribute handler or if we need to process each function body
(gimplifying parameters sounds like a good place to me for example).

I still think the FE should simplify the MEs life by doing the copy trick
for all intent-in scalars though.

[Bug fortran/67170] PRE can't hoist out a readonly argument

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |rguenth at gcc dot gnu.org

--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
Mine, for the middle-end part.

[Bug fortran/67170] PRE can't hoist out a readonly argument

[rguenth at gcc dot gnu.org]

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

--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> ---
We can do what the FE should do at gimplification time as well I guess.

[Bug fortran/67170] PRE can't hoist out a readonly argument

[rguenth at gcc dot gnu.org]

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

--- Comment #8 from Richard Biener <rguenth at gcc dot gnu.org> ---
I wonder why IPA SRA doesn't turn the scalar parameter from by-reference to
by-value passing btw.  I do see that IPA SRA doesn't handle recursion though,
thus

static int __attribute__((noinline)) foo (int *p)
{
  if (*p)
    return foo (p);
  return *p;
}

int main()
{
  int i = 0;
  return foo (&i);
}

is not transformed.  But in the testcase there isn't a recursive call using
the incoming parameter.

As for the idea to use the alias machinery I'm now leaning towards either
having PTA compute a "points-to-readonly" or SCCVN/LIM pre-compute the
parm-decl
property and have its own disambiguator handle the disambiguation.

[Bug fortran/67170] PRE can't hoist out a readonly argument

[rguenth at gcc dot gnu.org]

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

--- Comment #9 from Richard Biener <rguenth at gcc dot gnu.org> ---
Author: rguenth
Date: Tue Sep 29 13:04:18 2015
New Revision: 228244

URL: https://gcc.gnu.org/viewcvs?rev=228244&root=gcc&view=rev
Log:
2015-09-29  Richard Biener  <rguenther@suse.de>

        PR tree-optimization/67170
        * tree-ssa-alias.h (get_continuation_for_phi): Adjust
        the translate function pointer parameter to get the
        bool whether to disambiguate only by reference.
        (walk_non_aliased_vuses): Likewise.
        * tree-ssa-alias.c (maybe_skip_until): Adjust.
        (get_continuation_for_phi_1): Likewise.
        (get_continuation_for_phi): Likewise.
        (walk_non_aliased_vuses): Likewise.
        * tree-ssa-sccvn.c (const_parms): New bitmap.
        (vn_reference_lookup_3): Adjust for interface change.
        Disambiguate parameters pointing to readonly memory.
        (free_scc_vn): Free const_parms.
        (run_scc_vn): Initialize const_parms from a fn spec attribute.

        * gfortran.dg/pr67170.f90: New testcase.

Added:
    trunk/gcc/testsuite/gfortran.dg/pr67170.f90
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-ssa-alias.c
    trunk/gcc/tree-ssa-alias.h
    trunk/gcc/tree-ssa-sccvn.c

[Bug fortran/67170] PRE can't hoist out a readonly argument

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

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

--- Comment #10 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed.

[Bug fortran/67170] PRE can't hoist out a readonly argument

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |6.0
           Keywords|                            |missed-optimization