https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106187
--- Comment #46 from rguenther at suse dot de <rguenther at suse dot de> --- On Mon, 25 Jul 2022, rearnsha at gcc dot gnu.org wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106187 > > --- Comment #45 from Richard Earnshaw <rearnsha at gcc dot gnu.org> --- > The problem with changing rtx_equal_for_cselib_1 is that it is essentially > commutative in its operands - it doesn't disambiguate with x substituting for > y > or vice-versa, so we cannot tell if an operation is a load or a store. True, but the new special mode could require the first to be a load or store and the second a store taking place after the first arg (so we have either WAW or WAR). > A minimal fix, which just suppresses stores would be: > > @@ -81,6 +81,10 @@ reload_cse_noop_set_p (rtx set) > if (cselib_reg_set_mode (SET_DEST (set)) != GET_MODE (SET_DEST (set))) > return 0; > > + /* Fixme: we need to check that removing a store doesn't change > + the alias computations. */ > + if (flag_strict_aliasing && MEM_P (SET_DEST (set))) > + return 0; > return rtx_equal_for_cselib_p (SET_DEST (set), SET_SRC (set)); > } Yeah, that works (does that catch all stores? or at least all stores that are simple enough for cselib to handle?). > But we could no-doubt improve on that. The issue here is that SET_SRC (set) is usually a REG, but we need the corresponding earlier MEM SET_DEST is requal to which the REG was derived from to make the decision on whether the store is redundant from a TBAA perspective as well.
