On Fri, 10 Nov 2017, Jakub Jelinek wrote:
> On Fri, Nov 10, 2017 at 08:52:16AM +0100, Richard Biener wrote:
> > > @@ -958,8 +959,10 @@ pass_store_merging::terminate_all_aliasi
> > > unsigned int i;
> > > FOR_EACH_VEC_ELT (cur->m_store_info, i, info)
> > > {
> > > - if (ref_maybe_used_by_stmt_p (stmt, gimple_assign_lhs (info->stmt))
> > > - || stmt_may_clobber_ref_p (stmt, gimple_assign_lhs (info->stmt)))
> > > + tree lhs = gimple_assign_lhs (info->stmt);
> > > + if (ref_maybe_used_by_stmt_p (stmt, lhs)
> > > + || stmt_may_clobber_ref_p (stmt, lhs)
> > > + || (store_lhs && refs_output_dependent_p (store_lhs, lhs)))
> >
> > Looks good but may do redundant work for store_lhs? So rather
> >
> > || (! store_lhs && stmt_may_clobber_ref_p (stmt, lhs)
> > || (store_lhs && refs_output_dependent_p (store_lhs, lhs)
> >
> > ? Fails to handle storing calls (in case those can appear in the chains).
>
> info->stmt is known to be a store, but stmt is not, it can be any other
> stmt, including calls, so the above would miss the calls handling.
>
> > Looks like we miss some convenient stmt_output/anti_dependent_p (you can
> > follow stmt_may_clobbers_ref_p[_1] for cut&pasting and/or add a
> > bool tbaa flag we can pass down to stmt_may_clobber_ref_p_1).
>
> So perhaps bool tbaa = true argument to both stmt_may_clobber_ref_p_1
> and stmt_may_clobber_ref_p, or just stmt_may_clobber_ref_p_1 and
> add some differently named alternative to stmt_may_clobber_ref_p
> (in that case, any suggestions on a good name?)?
Internally (aka static fn) I'd just add a bool param w/o default.
The external API should be stmt_output/anti_dependent_p and I think
these days with C++ we could make stmt_may_clobber_ref_p_1 taking ao_ref
and stmt_may_clobber_ref_p taking a tree overloads of
stmt_may_clobber_ref_p. We'd then have _1 being static and having the
extra arg.
Richard.
> > That said - the patch is ok, any improvements can be done as followup.
>
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
