This is the mail archive of the mailing list for the GCC project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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 

Eric Botcazou

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]