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. Thanks, Kugan > > for this? > > Thanks, > Richard. > >> Thanks, >> Kugan >> >> >> >>
0001-Bug-123067-V2-LICM-wrong-code.patch
Description: 0001-Bug-123067-V2-LICM-wrong-code.patch
