On 7/29/2022 7:52 AM, Richard Earnshaw via Gcc-patches wrote:
A SET operation that writes memory may have the same value as an earlier store but if the alias sets of the new and earlier store do not conflict then the set is not truly redundant.  This can happen, for example, if objects of different types share a stack slot.

To fix this we define a new function in cselib that first checks for
equality and if that is successful then finds the earlier store in the
value history and checks the alias sets.

The routine is used in two places elsewhere in the compiler. Firstly
in cfgcleanup and secondly in postreload.

gcc/ChangeLog:
    * alias.h (mems_same_for_tbaa_p): Declare.
    * alias.cc (mems_same_for_tbaa_p): New function.
    * dse.cc (record_store): Use it instead of open-coding
    alias check.
    * cselib.h (cselib_redundant_set_p): Declare.
    * cselib.cc: Include alias.h
    (cselib_redundant_set_p): New function.
    * cfgcleanup.cc: (mark_effect): Use cselib_redundant_set_p instead
    of rtx_equal_for_cselib_p.
    * postreload.c (reload_cse_simplify): Use cselib_redundant_set_p.
    (reload_cse_noop_set_p): Delete.
Seems quite reasonable.   The only question I would have would be whether or not you considered including the aliasing info into the hashing used by cselib.  You'd probably still need the bulk of this patch as well since we could presumably still get a hash conflict with two stores of the same value to the same location, but with different alias sets (it's just much less likely), so perhaps it doesn't really buy us anything.

Ideally this would include a testcase.  You might be able to turn that non-executawble reduced case into something useful by scanning the post-reload dumps.

Jeff

Reply via email to