On Wed, 25 Nov 2015, Jakub Jelinek wrote:
On Wed, Nov 25, 2015 at 08:56:45AM +0100, Marc Glisse wrote:
This is the GIMPLE side of Richard's i?86 uadd/usub overflow
testing improvements. If unsigned addition or subtraction
result is used both normally and in a GIMPLE_COND/COND_EXPR/tcc_comparison
that tests if unsigned overflow happened, the patch replaces it shortly
before expansion with {ADD,SUB}_OVERFLOW, so that RTL expansion can generate
better code on it.
If I test a+b<a and don't use a+b anywhere else, don't we also want to use
the OVERFLOW things so we can expand to test the carry flag? That is, I am
not convinced we want to punt on has_single_use for add_overflow. For
sub_overflow with a single use of y-z, I guess y-z>y should become z>y, and
going through a rewrite with sub_overflow neither helps nor hinders that.
Actually, writing z>y is something the user is not unlikely to have done
himself, and walking through the uses of y or z should not be hard, so I
guess it could make sense to rewrite y-z>y to z>y always in match.pd and
only look for the second form in math-opts.
Incremental diff for also handling the single use case if it is overflow
check is below. But we already generate good code without it for the
x+y<x or x+y<y cases (and they aren't really problematic, as they are single
use), and while it is true that for x-y>x case the incremental patch below
improves the generated code right now, as you said it is better to rewrite
those as y>x and as it is a single use, it is easier to do it in match.pd.
So, I'd prefer to add that transformation and not use {ADD,SUB}_OVERFLOW
for those cases, because we get good enough code without increasing the IL
size, eating more memory etc.
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 be done whether it is single use or not, and we could look for the
pattern above instead of the one you currently have for sub_overflow. The
main change would be that now it isn't obvious where to insert the
sub_overflow, since one operation doesn't obviously dominate the other :-(
--
Marc Glisse
