Re: [PATCH] Convert manual unsigned +/- overflow checking into {ADD,SUB}_OVERFLOW (PR target/67089)

[Jakub Jelinek <jakub at redhat dot com>]

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 was thinking more match.pd to transform a+b<a and sccvn to somehow CSE a+b
> with add_overflow(a,b), but your patch seems to work well with simpler code,
> that's cool :-)
> 
> And it shouldn't be too hard to add a few more later, to detect widening
> operations that are only used for overflow testing, although the form of
> such tests is much less universal among users.

--- gcc/tree-ssa-math-opts.c.jj	2015-11-24 17:00:10.000000000 +0100
+++ gcc/tree-ssa-math-opts.c	2015-11-25 09:25:31.781087597 +0100
@@ -3586,7 +3586,6 @@ match_uaddsub_overflow (gimple_stmt_iter
   tree type = TREE_TYPE (lhs);
   use_operand_p use_p;
   imm_use_iterator iter;
-  bool use_seen = false;
   bool ovf_use_seen = false;
   gimple *use_stmt;
 
@@ -3594,7 +3593,6 @@ match_uaddsub_overflow (gimple_stmt_iter
   if (!INTEGRAL_TYPE_P (type)
       || !TYPE_UNSIGNED (type)
       || has_zero_uses (lhs)
-      || has_single_use (lhs)
       || optab_handler (code == PLUS_EXPR ? uaddv4_optab : usubv4_optab,
 			TYPE_MODE (type)) == CODE_FOR_nothing)
     return false;
@@ -3606,14 +3604,13 @@ match_uaddsub_overflow (gimple_stmt_iter
 	continue;
 
       if (uaddsub_overflow_check_p (stmt, use_stmt))
-	ovf_use_seen = true;
-      else
-	use_seen = true;
-      if (ovf_use_seen && use_seen)
-	break;
+	{
+	  ovf_use_seen = true;
+	  break;
+	}
     }
 
-  if (!ovf_use_seen || !use_seen)
+  if (!ovf_use_seen)
     return false;
 
   tree ctype = build_complex_type (type);


	Jakub