On Wed, 20 May 2026, Xin Wang wrote:
> Factor out the base-and-offset comparison shared by mem_ref_hasher::equal
> and is_self_write. The helper handles MEM_REF bases by comparing their
> underlying pointer operands and by folding the MEM_REF offset into the
> ao_ref offset before comparing the final access position.
> Keep the callers' higher-level semantics separate. mem_ref_hasher::equal
> still performs its hashtable-specific size, max_size, volatile, alias-set
> and type checks after the shared base/offset comparison.
> For is_self_write, require both references to describe the same exact access
> before treating them as the same write: size must match, max_size must be
> known and match, and max_size must equal size for both references. This
> addresses the partial-overlap concern by avoiding exact self-write
> classification when either access may cover more bytes than its nominal size.
>
> Signed-off-by: Xin Wang <[email protected]>
> ---
> gcc/tree-ssa-loop-im.cc | 69 ++++++++++++++++++++++++-----------------
> 1 file changed, 41 insertions(+), 28 deletions(-)
>
> diff --git a/gcc/tree-ssa-loop-im.cc b/gcc/tree-ssa-loop-im.cc
> index 1e99728218f..63325ab5fbf 100644
> --- a/gcc/tree-ssa-loop-im.cc
> +++ b/gcc/tree-ssa-loop-im.cc
> @@ -196,6 +196,26 @@ mem_ref_hasher::hash (const im_mem_ref *mem)
> return mem->hash;
> }
>
> +/* Return true if REF1 and REF2 have the same base and offset. This handles
> + MEM_REF bases specially by comparing their underlying pointer operands and
> + merging the MEM_REF offset with the ao_ref offset. */
> +
> +static bool
> +im_refs_same_base_and_offset_p (const ao_ref *ref1, const ao_ref *ref2)
> +{
> + if (TREE_CODE (ref1->base) == MEM_REF
> + && TREE_CODE (ref2->base) == MEM_REF)
> + return (operand_equal_p (TREE_OPERAND (ref1->base, 0),
> + TREE_OPERAND (ref2->base, 0), 0)
> + && known_eq (mem_ref_offset (ref1->base) * BITS_PER_UNIT
> + + ref1->offset,
> + mem_ref_offset (ref2->base) * BITS_PER_UNIT
> + + ref2->offset));
> +
> + return (operand_equal_p (ref1->base, ref2->base, 0)
> + && known_eq (ref1->offset, ref2->offset));
So this refactoring doesn't fall back to the latter if the
former return condition evaluates false. I'd prefer to
split the refs_equal condition out in full.
> +}
> +
> /* An equality function for class im_mem_ref object MEM1 with
> memory reference OBJ2. */
>
> @@ -210,17 +230,7 @@ mem_ref_hasher::equal (const im_mem_ref *mem1, const
> ao_ref *obj2)
>
> /* obj2 is now known decomposable and the hash is based on offset,
> size and base, matching gather_mem_refs_stmt. */
> - bool refs_equal
> - = ((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)));
> + bool refs_equal = im_refs_same_base_and_offset_p (&mem1->mem, obj2);
>
> return (refs_equal
> && known_eq (mem1->mem.size, obj2->size)
> @@ -3163,6 +3173,25 @@ ref_always_accessed_p (class loop *loop, im_mem_ref
> *ref, bool stored_p)
> ref_always_accessed (loop, stored_p));
> }
>
> +/* Returns true if REF1 and REF2 describe the same exact access. */
> +
> +static bool
> +im_refs_same_exact_location_p (const im_mem_ref *ref1, const im_mem_ref
> *ref2)
> +{
> + if (!ref1->ref_decomposed || !ref2->ref_decomposed)
> + return operand_equal_p (ref1->mem.ref, ref2->mem.ref, 0);
> +
> + if (!im_refs_same_base_and_offset_p (&ref1->mem, &ref2->mem))
> + return false;
> +
> + return (known_eq (ref1->mem.size, ref2->mem.size)
> + && ref1->mem.max_size_known_p ()
> + && ref2->mem.max_size_known_p ()
> + && known_eq (ref1->mem.max_size, ref2->mem.max_size)
> + && known_eq (ref1->mem.max_size, ref1->mem.size)
> + && known_eq (ref2->mem.max_size, ref2->mem.size));
I think this is one equality too much, I'd drop the last. Likewise
one max_size_known_p too much.
Otherwise the patch fails to git am due to some context changes.
Thanks,
Richard.
> +}
> +
> /* 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. */
> @@ -3192,23 +3221,8 @@ is_self_write (im_mem_ref *load_ref, im_mem_ref
> *store_ref)
> if (stored_val != loaded_val)
> return false;
>
> -
> - /* 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_refs_same_exact_location_p (load_ref, store_ref);
>
> }
>
> @@ -3883,4 +3897,3 @@ make_pass_lim (gcc::context *ctxt)
> {
> return new pass_lim (ctxt);
> }
> -
>
--
Richard Biener <[email protected]>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
