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