On Tue, Jul 18, 2017 at 09:31:11AM -0600, Martin Sebor wrote:
> > --- gcc/match.pd.jj 2017-07-17 16:25:20.000000000 +0200
> > +++ gcc/match.pd 2017-07-18 12:32:52.896924558 +0200
> > @@ -1125,6 +1125,19 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> > && wi::neg_p (@1, TYPE_SIGN (TREE_TYPE (@1))))
> > (cmp @2 @0))))))
> >
> > +/* (X - 1U) <= INT_MAX-1U into (int) X > 0. */
>
> Since the transformation applies to other types besides int I
> suggest to make it clear in the comment. E.g., something like:
>
> /* (X - 1U) <= TYPE_MAX - 1U into (TYPE) X > 0 for any integer
> TYPE. */
>
> (with spaces around all the operators as per GCC coding style).
I think many of the match.pd comments are also not fully generic
to describe what it does, just to give an idea what it does.
The above isn't correct either, because it isn't for any integer TYPE,
there needs to be a signed and corresponding unsigned type involved,
X is of the unsigned type, so is the 1. And TYPE_MAX is actually
the signed type's maximum cast to unsigned type. And the reason for not putting
spaces around - in the second case was an attempt to give a hint that
it is comparison against a INT_MAX-1U constant, not another subtraction.
After all, the pattern doesn't handle subtraction, because that isn't
what is in the IL, but addition, i.e. X + -1U.
And, the <= -> > is just one possibility, the pattern also handles
> -> <=.
Examples of other comments that "suffer" from similar lack of sufficient
genericity, but perhaps are good enough to let somebody understand it
quickly:
/* Avoid this transformation if C is INT_MIN, i.e. C == -C. */
/* Avoid this transformation if X might be INT_MIN or
Y might be -1, because we would then change valid
INT_MIN % -(-1) into invalid INT_MIN % -1. */
/* If the constant operation overflows we cannot do the transform
directly as we would introduce undefined overflow, for example
with (a - 1) + INT_MIN. */
/* X+INT_MAX+1 is X-INT_MIN. */
Jakub
