Perhaps I'm still missing how some cases are handled or not handled,
sorry for the noise.

> I'm not sure there is anything to "interpret" -- the operation is unsigned
> and overflow is when the operation may wrap around zero.  There might
> be clever ways of re-writing the expression to
> (uint64_t)((uint32_t)((int32_t)uint32 + -1) + 1)
> avoiding the overflow and thus allowing the transform but I'm not sure that's
> good.

The extra work I introduced was to discern between

 (uint64_t)(a + UINT_MAX) + 1  -> (uint64_t)(a),
 (uint64_t)(a + UINT_MAX) + 1  -> (uint64_t)(a) + (uint64_t)(UINT_MAX + 1),

For a's range of [1,1] there is an overflow in both cases.
We still want to simplify the first case by combining UINT_MAX + 1 -> 0.
If "interpreting" UINT_MAX as -1 is not the correct thing to do, perhaps
(uint64_t)((uint32_t)(UINT_MAX + 1)) is? This fails, however, if the
outer constant is larger than UINT_MAX. What else can we do here?
Do we see cases like the second one at all? If it's not needed, the
extra work is likely not needed.

> A related thing would be canonicalizing unsigned X plus CST to
> unsigned X minus CST'
> if CST' has a smaller absolute value than CST.  I think currently we
> simply canonicalize
> to 'plus CST' but also only in fold-const.c, not in match.pd (ah we
> do, but only in a simplified manner).

I can imagine this could simplify the treatment of some cases, yet I'm
already a bit lost with the current cases :)

> That said, can we leave that "trick" out of the patch?  I think your
> more complicated
> "overflows" result from extract_range_from_binary_expr_1 doesn't apply to all
> ops (like MULT_EXPR where more complicated cases can arise).

There is certainly additional work to be done for MULT_EXPR, I
disregarded it so far. For this patch, I'd rather conservatively assume
overflow.

Regards
 Robin

Reply via email to