On Wed, Nov 25, 2015 at 12:19:04PM +0100, Marc Glisse wrote:
> On Wed, 25 Nov 2015, Jakub Jelinek wrote:
>
> >On Wed, Nov 25, 2015 at 09:58:15AM +0100, Marc Glisse wrote:
> >>I guess it got lost in my text, but if a user writes:
> >>
> >>unsigned diff = a - b;
> >>if (b > a) { /* overflow */ ... }
> >>else { ... }
> >>
> >>Your patch will not detect it. It seems that replacing x-y>x with y>x could
> >
> >Sorry, already committed the patch (without incremental that hasn't been
> >tested anyway).
>
> Sorry, I never meant to imply that your patch was wrong in any way or should
> be blocked, I like it. I was only discussing possible future improvements...
BTW, the primary reason for the patch has been a code quality regression,
and I bet that is only for the case of if (diff > a), otherwise combiner
with the problematic subtraction with overflow patterns wouldn't be able to
find anything. rth fixed it only for the case where users explicitly use
the new __builtin_*_overflow builtins, and the patch has been trying to fix
the regression even when not written that way.
> The same is true whether we write it b > a or (a - b) > a (I don't think PRE
> + SCCVN avoid increasing register pressure).
>
> >So, I'd really prefer doing x-y>x to y>x only for single use.
>
> Ok (for now).
Do you plan to work on that (my match.pd experience is smaller than yours),
or should I add to my todo list?
Jakub
