On Fri, Dec 19, 2025 at 12:52 AM Kugan Vivekanandarajah
<[email protected]> wrote:
>
> Hi,
>
> Thanks for the review.
>
> > On 15 Dec 2025, at 11:56 pm, Richard Biener <[email protected]>
> > wrote:
> >
> > External email: Use caution opening links or attachments
> >
> >
> > On Sun, Dec 14, 2025 at 6:17 AM Kugan Vivekanandarajah
> > <[email protected]> wrote:
> >>
> >> Hi,
> >> This a patch fixes Bug 123067] by checking for partial aliasing in self
> >> write test in LICM.
> >>
> >> Bootstrapped and regression tested with no new regressions.
> >>
> >> gcc/ChangeLog:
> >>
> >> 2025-12-09 Kugan Vivekanandarajah <[email protected]>
> >>
> >> PR middle-end/123067
> >> * tree-ssa-loop-im.cc (is_self_write):
> >>
> >> gcc/testsuite/ChangeLog:
> >>
> >> 2025-12-09 Kugan Vivekanandarajah <[email protected]>
> >>
> >> PR middle-end/123067
> >> * gcc.dg/licm-self-write-partial-alias.c: New test.
> >> Is this OK?
> >
> > + /* Verify there is no partial aliasing. */
> > + if (!mem_refs_may_alias_p (load_ref, store_ref,
> > + &memory_accesses.ttae_cache, true))
> > + return true; /* Disjoint: safe to hoist. */
> >
> > this is redundant? If they are not aliasing then the caller would
> > already say so?
> >
> > + /* 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));
> >
> > this looks incomplete. See mem_ref_hasher::equal.
> > That is, dependent on ->ref_decomposed the compare should look different,
> > merging .offset with the MEM_REF offset in base. Maybe we can factor
> > out a helper like
> >
> > bool im_compare_access_position_and_size (ao_ref *ref1, ao_ref *ref2)
>
> Tried factoring out but this is making it more complicated (due to the
> divergence). Here is the version I tested. Please let me know
> If you want me to post the version with im_compare_access_position_and_size.
I think this will now regress the case where the reference we want to optimize
is variably indexed (with invariant index, of course). I'm not sure we need
the
+ && load_ref->mem.volatile_p == store_ref->mem.volatile_p
+ && (load_ref->mem.ref_alias_set == store_ref->mem.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. */
+ || (!load_ref->ref_canonical
+ && (TREE_CODE (store_ref->mem.ref) == MEM_REF
+ || TREE_CODE (store_ref->mem.ref) == TARGET_MEM_REF)
+ && store_ref->mem.ref_alias_set == 0)
+ /* Likewise if there's a canonical ref with alias-set zero. */
+ || (load_ref->ref_canonical
+ && load_ref->mem.ref_alias_set == 0)));
part here, since the argument is not about TBAA. It's just the
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)
..
else
return operand_equal_p (mem1->mem.ref, obj2->ref, 0);
parts that are relevant. You'll have to ensure max_size_known_p agrees
or resort to alignment considerations to rule out partial overlaps. I mostly
suggested the factoring to have one place with the "delicate" handling.
But I see this is now very complicated so I'd say go with your original
version which should be conservatively correct, just not perfect in
allowing all opportunities. I'll note down a TODO to try to factor this
in a way that suits me which I guess is more effective than trying
back-and-forth via reviews ;)
Thanks,
Richard.
> Thanks,
> Kugan
>
>
>
>
> >
> > for this?
> >
> > Thanks,
> > Richard.
> >
> >> Thanks,
> >> Kugan
> >>
> >>
> >>
> >>
>
