https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69352

--- Comment #9 from alalaw01 at gcc dot gnu.org ---
(In reply to Jakub Jelinek from comment #7)
> There are various bugs in the r232508 change.
> The
>   gcc_assert (sz0 == sz1);
>   gcc_assert (max0 == max1);
>   gcc_assert (rev0 == rev1);
> asserts are clearly bogus, while for compatible type I bet size will be
> always the same, maximum size can be arbitrary (it will be either equal to
> size, then it is a fixed access, or it will be larger, then it is a variable
> access), and the reverse stuff looks weird (e.g. I think the lack of
> REF_REVERSE_STORAGE_ORDER testing in operand_equal_p is a bug).  For a
> variable access, even if you remove the above max{0,1} assert, I think you
> would happily equate say a[i] and a[j] ARRAY_REFs, because they have the
> same off (likely 0) and max (likely size of array in bits).  Another problem
> I see in the
>       return equal_mem_array_ref_p (expr0->ops.single.rhs,
>                                     expr1->ops.single.rhs)
>              || operand_equal_p (expr0->ops.single.rhs,
>                                  expr1->ops.single.rhs, 0);
> case; under some conditions you decide to hash the MEM_REF/ARRAY_REFs as
> MEM_REF , hash of base, offset and size, so you should use the same
> conditions to decide if you use equal_mem_array_ref_p or operand_equal_p,

Agreed, yes. That would fix the bogus asserts, right - we would then only use
equal_mem_array_ref_p if size==max_size.

> Plus, I'm not sure in what places this hashing
> is used, I'm worried you might hash MEM_REFs with different alias types for
> the accesses as equal, which for some uses might be fine, if you are not
> trying to replace one with another etc., but for other cases it might lead
> to wrong-code.

I think it should be OK to ignore differences in alias type for DOM
optimizations etc., indeed, which is where this was intended.

Reply via email to