[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