> But they _do_ overflow as my debugging showed, caused by that exact
> same extract_muldiv_1 function here:
>
> case PLUS_EXPR: case MINUS_EXPR:
> /* See if we can eliminate the operation on both sides. If we can,
> we
> can return a new PLUS or MINUS. If we can't, the only remaining
> cases where we can do anything are if the second operand is a
> constant. */
> ...
> /* If this was a subtraction, negate OP1 and set it to be an
> addition.
> This simplifies the logic below. */
> if (tcode == MINUS_EXPR)
> {
> tcode = PLUS_EXPR, op1 = negate_expr (op1);
>
> the unconditional negation of op1 for tcode == MINUS_EXPR overflows
> all sizetype values (well, all unsigned values).
Only if sizetypes are no longer sign-extended, though; otherwise, I don't see
the problem (and we certainly never detected one). Does something really go
wrong with the unpatched compiler?
> Please please add some _testcases_ then that would fail when I test
> this kind of patches.
Performance testcases are hard to extract beforehand. It's only when you have
a regression that you really start to dig.
> The patch fixed a real bug (it's just that it is hard (for me) to
> produce testcases that involve sizetype computations - it always
> requires Ada to expose those bugs). So, I beg to differ.
Well, on the one hand you ask for testcases in difficult cases, and on the
other hand you don't provide one when you already have something at hand.
The elimination of the sign-extension for sizetypes is probably a progress
overall, although this will very likely cause a few problems in Ada. But
removing working code (albeit admittedly hard to grasp) without testcases or
real experiments is a different thing.
--
Eric Botcazou
