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
>
