I apologize for the delayed response; I haven't been able to check my emails promptly due to work commitments. Thank you for your review. I will send the second version of the patch.
Tnanks, Xin Wang Andrew Pinski <[email protected]> 于2026年1月16日周五 10:25写道： > On Thu, Jan 15, 2026 at 4:11 AM Xin Wang <[email protected]> wrote: > > > > From: Xin Wang <[email protected]> > > > > This patch centralizes the delicate handling of MEM_REF offsets and base > > pointer equality into a new helper function > im_compare_access_position_and > > _size, which is now used by both mem_ref_hasher::equal and is_self_write. > > > > gcc/ChangeLog: > > * tree-ssa-loop-im.cc (im_compare_access_position_and_size): New helper > > function to compare ao_ref position and size. (mem_ref_hasher::equal): > > Use the new helper for position and size comparison, keeping additional > > hash table specific checks. (is_self_write): Likewise, using the > > centralized helper after checking ref_decomposed. > > > > Signed-off-by: Xin Wang <[email protected]> > > --- > > gcc/tree-ssa-loop-im.cc | 117 ++++++++++++++++++++++++++-------------- > > 1 file changed, 76 insertions(+), 41 deletions(-) > > > > diff --git a/gcc/tree-ssa-loop-im.cc b/gcc/tree-ssa-loop-im.cc > > index b9b1d92b518..bf9ae4a5fd9 100644 > > --- a/gcc/tree-ssa-loop-im.cc > > +++ b/gcc/tree-ssa-loop-im.cc > > @@ -180,6 +180,9 @@ query_loop_dependence (class loop *loop, im_mem_ref > *ref, dep_kind kind) > > return dep_unknown; > > } > > > > +/* Forward declaration. */ > > +static bool im_compare_access_position_and_size (const ao_ref *, const > ao_ref *); > > Why not move this function up and not add the forward declaration? It > does not depend on anything defined in this file. > That is the only thing I have to add here as the TODO was added for > Richard to review it later. Plus we are in stage 4 so this > might have to wait until stage 1 of GCC 17. > > Thanks, > Andrew > > > + > > /* Mem_ref hashtable helpers. */ > > > > struct mem_ref_hasher : nofree_ptr_hash <im_mem_ref> > > @@ -204,31 +207,30 @@ inline bool > > mem_ref_hasher::equal (const im_mem_ref *mem1, const ao_ref *obj2) > > { > > if (obj2->max_size_known_p ()) > > - return (mem1->ref_decomposed > > - && ((TREE_CODE (mem1->mem.base) == MEM_REF > > - && TREE_CODE (obj2->base) == MEM_REF > > - && operand_equal_p (TREE_OPERAND (mem1->mem.base, 0), > > - TREE_OPERAND (obj2->base, 0), 0) > > - && known_eq (mem_ref_offset (mem1->mem.base) * > BITS_PER_UNIT + mem1->mem.offset, > > - mem_ref_offset (obj2->base) * > BITS_PER_UNIT + obj2->offset)) > > - || (operand_equal_p (mem1->mem.base, obj2->base, 0) > > - && known_eq (mem1->mem.offset, obj2->offset))) > > - && known_eq (mem1->mem.size, obj2->size) > > - && known_eq (mem1->mem.max_size, obj2->max_size) > > - && mem1->mem.volatile_p == obj2->volatile_p > > - && (mem1->mem.ref_alias_set == obj2->ref_alias_set > > - /* We are not canonicalizing alias-sets but for the > > - special-case we didn't canonicalize yet and the > > - incoming ref is a alias-set zero MEM we pick > > - the correct one already. */ > > - || (!mem1->ref_canonical > > - && (TREE_CODE (obj2->ref) == MEM_REF > > - || TREE_CODE (obj2->ref) == TARGET_MEM_REF) > > - && obj2->ref_alias_set == 0) > > - /* Likewise if there's a canonical ref with alias-set > zero. */ > > - || (mem1->ref_canonical && mem1->mem.ref_alias_set == 0)) > > - && types_compatible_p (TREE_TYPE (mem1->mem.ref), > > - TREE_TYPE (obj2->ref))); > > + { > > + if (!mem1->ref_decomposed) > > + return false; > > + > > + /* Use the centralized helper for position and size comparison. > */ > > + if (!im_compare_access_position_and_size (&mem1->mem, obj2)) > > + return false; > > + > > + /* Additional checks specific to hash table lookup. */ > > + return (mem1->mem.volatile_p == obj2->volatile_p > > + && (mem1->mem.ref_alias_set == obj2->ref_alias_set > > + /* We are not canonicalizing alias-sets but for the > > + special-case we didn't canonicalize yet and the > > + incoming ref is a alias-set zero MEM we pick > > + the correct one already. */ > > + || (!mem1->ref_canonical > > + && (TREE_CODE (obj2->ref) == MEM_REF > > + || TREE_CODE (obj2->ref) == TARGET_MEM_REF) > > + && obj2->ref_alias_set == 0) > > + /* Likewise if there's a canonical ref with alias-set > zero. */ > > + || (mem1->ref_canonical && mem1->mem.ref_alias_set == > 0)) > > + && types_compatible_p (TREE_TYPE (mem1->mem.ref), > > + TREE_TYPE (obj2->ref))); > > + } > > else > > return operand_equal_p (mem1->mem.ref, obj2->ref, 0); > > } > > @@ -3148,6 +3150,50 @@ ref_always_accessed_p (class loop *loop, > im_mem_ref *ref, bool stored_p) > > ref_always_accessed (loop, stored_p)); > > } > > > > +/* Returns true if the positions and sizes of two ao_ref structures are > > + equal. This handles MEM_REF offsets specially by merging the offset > > + from the MEM_REF base with the ao_ref offset. > > + > > + This helper centralizes the delicate handling of memory reference > > + comparison, used by both mem_ref_hasher::equal and is_self_write. */ > > + > > +static bool > > +im_compare_access_position_and_size (const ao_ref *ref1, const ao_ref > *ref2) > > +{ > > + /* Check base pointer equality. For MEM_REF types, we need to > > + compare the underlying pointer operands and merge the offsets. */ > > + if (TREE_CODE (ref1->base) == MEM_REF && TREE_CODE (ref2->base) == > MEM_REF) > > + { > > + if (!operand_equal_p (TREE_OPERAND (ref1->base, 0), > > + TREE_OPERAND (ref2->base, 0), 0)) > > + return false; > > + > > + /* Both are MEM_REF - compare merged offsets from base and > offset. */ > > + if (!known_eq (mem_ref_offset (ref1->base) * BITS_PER_UNIT > > + + ref1->offset, > > + mem_ref_offset (ref2->base) * BITS_PER_UNIT > > + + ref2->offset)) > > + return false; > > + } > > + else if (!operand_equal_p (ref1->base, ref2->base, 0) > > + || !known_eq (ref1->offset, ref2->offset)) > > + return false; > > + > > + /* Compare sizes. */ > > + if (!known_eq (ref1->size, ref2->size)) > > + return false; > > + > > + /* If both max_sizes are known, they must agree to ensure > > + no partial overlaps are possible. */ > > + if (ref1->max_size_known_p () && ref2->max_size_known_p ()) > > + { > > + if (!known_eq (ref1->max_size, ref2->max_size)) > > + return false; > > + } > > + > > + return true; > > +} > > + > > /* Returns true if LOAD_REF and STORE_REF form a "self write" pattern > > where the stored value comes from the loaded value via SSA. > > Example: a[i] = a[0] is safe to hoist a[0] even when i==0. */ > > @@ -3177,23 +3223,12 @@ is_self_write (im_mem_ref *load_ref, im_mem_ref > *store_ref) > > if (stored_val != loaded_val) > > return false; > > > > + /* They may alias. Verify exact same location. > > + Use the centralized helper to handle MEM_REF offsets properly. */ > > + if (!load_ref->ref_decomposed || !store_ref->ref_decomposed) > > + return operand_equal_p (load_ref->mem.ref, store_ref->mem.ref, 0); > > > > - /* TODO: Try to factor this out with mem_ref_hasher::equal > > - into im_compare_access_position_and_size > > - or a similar helper to centralize this delicate handling > > - complete for MEM_REF offsets and base pointer equality. > > - > > - TODO: Also ensure max_size_known_p agrees or resort to > > - alignment considerations to rule out partial overlaps. > > - > > - See: > > - > https://gcc.gnu.org/pipermail/gcc-patches/2025-December/704155.html > > - For more context on TODOs above. */ > > - > > - /* They may alias. Verify exact same location. */ > > - return (operand_equal_p (load_ref->mem.base, store_ref->mem.base, 0) > > - && known_eq (load_ref->mem.size, store_ref->mem.size) > > - && known_eq (load_ref->mem.offset, store_ref->mem.offset)); > > + return im_compare_access_position_and_size (&load_ref->mem, > &store_ref->mem); > > > > } > > > > -- > > 2.34.1 > > >
