This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [patch] Fix tree-optimization/91169
> So isn't this an issue with the code before when we have a RANGE_EXPR
> that covers the wrapping point?
No, see the reduced testcase attached in the PR.
> Then index > max_index and may not catch the found element with
>
> /* Do we have match? */
> if (wi::cmpu (access_index, index) >= 0
> && wi::cmpu (access_index, max_index) <= 0)
> return cval;
>
> ? We seem to be careful to convert the indices to the index type but
> above use unsigned compares - isn't that the underlying issue?
Not really, everything is properly unsigned at this point, so the traversal
wraps around in an unsigned type.
> I'm not sure how "doing from scratch" would fix things - we simply rely
> on reliably detecting an element match.
>
> I wonder how much we can rely on the array domain type to be in sync
> with the constructor field entries.
At least we need to take into account the wraparound.
> Also we're using offset_int here - doesn't that break when with
> Ada we have an array type with domain [uint128_t_max - 1, uint128_t_max + 2]
> in a type larger than int128_t? Or, since we're interpreting it
> inconsistently signed/unsigned, a 128bit integer typed domain wraps
> around the sign?
Integer types are limited to 64 bits in GNAT.
> If that's not an issue then using signed compare unconditionally is
> a valid fix as well I guess.
But all the types are unsigned here so this doesn't seem appealing.
> But in reality 'index' should never really wrap, that is, if the domain
> is of 'int' type and starts with INT_MAX then the array shall not have
> more than 1 element. No?
The range is [-1, 1] converted to sizetype, which is unsigned, so the low
bound is UINT32_MAX and the high bound is 1.
> The patch you posted isn't a real fix btw, since iff the break was
> hit we either missed an index that is present or we found a gap
> but then we can break.
No, we cannot break if the index wraps around, that's the bug.
> So I'd appreciate more explanation on how the index "wraps".
See above. I can rewind the entire chain of events if need be, but this is a
long one starting a long time ago with the initial blunder with PLUS_EXPR and
sizetype.
--
Eric Botcazou