On Fri, 2 Sep 2011, Eric Botcazou wrote:
> > Well, the comment for that folding is totally odd - of _course_
> > unsigned sizetype things can overflow (we hid that issue merely
> > by pretending all unsigned sizetype constants (yes, only constants)
> > are signed. Huh.)
>
> It's again the special semantics of sizetypes whereby we pretend that they
> don't overflow. I know your opinion about this, but this is documented:
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).
So you might argue I should have fixed the "bug" here instead
of removing a TYPE_IS_SIZETYPE check (which I very much like to do,
as you know ;)).
> /* In an INTEGER_TYPE, it means the type represents a size. We use
> this both for validity checking and to permit optimizations that
> are unsafe for other types. Note that the C `size_t' type should
> *not* have this flag set. The `size_t' type is simply a typedef
> for an ordinary integer type that happens to be the type of an
> expression returned by `sizeof'; `size_t' has no special
> properties. Expressions whose type have TYPE_IS_SIZETYPE set are
> always actual sizes. */
> #define TYPE_IS_SIZETYPE(NODE) \
> (INTEGER_TYPE_CHECK (NODE)->type_common.no_force_blk_flag)
Well, this only says "and to permit optimizations that are unsafe for
other types." but it doesn't say what constraints apply to sizetypes.
We can't at the same time possibly introduce overflow and rely
on no overflow at the same time.
> and we rely on these optimizations to simplify size computations in Ada.
Please please add some _testcases_ then that would fail when I test
this kind of patches.
> > 2011-08-31 Richard Guenther <rguent...@suse.de>
> >
> > * fold-const.c (extract_muldiv_1): Remove bogus TYPE_IS_SIZETYPE
> > special-casing.
>
> IMO you shouldn't commit this kind of patchlets without the final big patch.
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.
Richard.
