[PATCH] Fix profiledbootstrap - store-merging aliasing issue (PR bootstrap/82916)

Jakub Jelinek jakub@redhat.com
Fri Nov 10 08:29:00 GMT 2017


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?)?

> That said - the patch is ok, any improvements can be done as followup.

	Jakub



More information about the Gcc-patches mailing list