Re: 'walk_stmt_load_store_addr_ops' for non-'gimple_assign_single_p (stmt)'

[Richard Biener <richard.guenther@gmail.com>]

On Wed, Mar 17, 2021 at 12:25 AM Thomas Schwinge
<thomas@codesourcery.com> wrote:
>
> Hi!
>
> Thanks, Michael, and again Richard for your quick responses.
>
> On 2021-03-16T15:25:10+0000, Michael Matz <matz@suse.de> wrote:
> > On Tue, 16 Mar 2021, Thomas Schwinge wrote:
> >
> >> >>Indeed, given (Fortran) 'zzz = 1', we produce GIMPLE:
> >> >>
> >> >>    gimple_assign <real_cst, zzz, 1.0e+0, NULL, NULL>
> >> >>
> >> >>..., and calling 'walk_stmt_load_store_addr_ops' on that, I see, as
> >> >>expected, the 'visit_store' callback invoked, with 'rhs' and 'arg':
> >> >>'<var_decl zzz>'.
> >> >>
> >> >>However, given (Fortran) 'zzz = r + r2', we produce GIMPLE:
> >> >>
> >> >>    gimple_assign <plus_expr, zzz, r, r2, NULL>
> >
> > But that's pre-ssa form.  After writing into SSA 'zzz' will be replaced by
> > an SSA name, and the actual store into 'zzz' will happen in a store
> > instruction.
>
> Oh, I see, and...
>
> >> >>..., and calling 'walk_stmt_load_store_addr_ops' on that, I see,
> >> >>unexpectedly, no callback at all invoked: neither 'visit_load', nor
> >> >>'visit_store' (nor 'visit_address', obviously).
> >> >
> >> > The variables involved are registers. You only get called on memory operands.
> >>
> >> How would I have told that from the 'walk_stmt_load_store_addr_ops'
> >> function description?  (How to improve that one "to reflect relatity"?)
> >>
> >> But 'zzz' surely is the same in 'zzz = 1' vs. 'zzz = r + r2' -- for the
> >> former I *do* see the 'visit_store' callback invoked, for the latter I
> >> don't?
> >
> > The walk_gimple functions are intended to be used on the SSA form of
> > gimple (i.e. the one that it is in most of the time).
>
> Yes, "most of the time", but actually not in my case: I'm doing stuff
> right after gimplification (before OMP lowering)...  So that's the
> "detail" I was missing -- sorry for not mentioning that right away.  :-|
>
> As 'walk_gimple_[...]' are used during gimplification, OMP lowering,
> supposedly they're fine to use in non-SSA form -- but evidently some of
> the helper functions are not.  (Might there be a way to add
> 'gcc_checking_assert (gimple_in_ssa_p (cfun))' or similar for that?
> Putting that into 'walk_stmt_load_store_addr_ops' triggers a lot, as
> called from 'gcc/cgraphbuild.c:cgraph_node::record_stmt_references', for
> example.)
>
> > And in that it's
> > not the case that 'zzz = 1' and 'zzz = r + r2' are similar.  The former
> > can have memory as the lhs (that includes static variables, or indirection
> > through pointers), the latter can not.  The lhs of a binary statement is
> > always an SSA name.  A write to an SSA name is not a store, which is why
> > it's not walked for walk_stmt_load_store_addr_ops.
> >
> > Maybe it helps to look at simple C examples: [...]
>
> I see, many thanks for reminding me about these items!
>
> > If you want
> > to capture writes into SSA names as well ([...])
> > you need the per-operand callback indeed.
>
> What I actually need is loads from/uses of actual variables (and I didn't
> see 'walk_stmt_load_store_addr_ops' give me these).
>
> > But that depends on
> > what you actually want to do.
>
> This is a prototype/"initial hack" for a very simple implementation of
> <https://gcc.gnu.org/PR90591> "Avoid unnecessary data transfer out of OMP
> construct".  (... to be completed and posted later.)  So simple that it
> will easily fail (gracefully, of course), but yet is effective for a lot
> of real-world code:
>
>     subroutine [...]
>     [...]
>     real([...])   xx        !temporary variable, for distance calculation
>     [...]
>     !$acc kernels pcopyin(x, zii) reduction(+:eva) ! implicit 'copy(xx)' for scalar used inside region; established during gimplification
>           do 100 i=0,n-1
>             evx=0.0d0
>             do 90 j=0,n-1
>               xx=abs(x(1,i)-x(1,j))
>     [...]
>     !$acc end kernels
>     [...]
>     ['xx' never used here]
>     end subroutine [...]
>
> Inside 'kernels', we'd like to automatically parallelize loops (we've
> basically got that working; analysis by Graphite etc.), but the problem
> is that given "implicit 'copy(xx)' for scalar used inside region", when
> Graphite later looks at the outlined 'kernels' region's function, it must
> assume that 'xx' is still live after the OpenACC 'kernels' construct --
> and thus cannot treat it as a thread-private temporary, cannot
> parallelize.
>
> Now, walking each function backwards (!), I'm taking note of any
> variables' uses, and if I reach an 'kernels' construct, but have not seen
> a use of 'xx', I may then optimize 'copy(xx)' -> 'firstprivate(xx)',
> enabling Graphite to do its thing.  (Other such optimizations may be
> added later.)  (This is inspired by Jakub's commit
> 1a80d6b87d81c3f336ab199a901cf80ae349c335 "re PR tree-optimization/68128
> (A huge regression in Parboil v2.5 OpenMP CUTCP test (2.5 times lower
> performance))".)
>
> I've now got a simple 'callback_op', which for '!is_lhs' looks at
> 'get_base_address ([op])', and if that 'var' is contained in the set of
> current candidates (initialized per containg 'bind's, which we enter
> first, even if walking a 'gimple_seq' backwards), removes that 'var' as a
> candidate for such optimization.  (Plus some "details", of couse.)  This
> seems to work fine, as far as I can tell.  :-)

It might then still fail for x = a[var] when you are interested in 'var'.

I think you want to use walk_gimple_stmt and provide walk_tree_fn which
will recurse into the complex tree operands (also making get_base_address
unnecessary).

>
> Of course, the eventual IPA-based solution (see PR90591, PR94693, etc.)
> will be much better -- but we need something now.
>
>
> Grüße
>  Thomas
> -----------------
> Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank Thürauf