Re: [PATCH] Further improvements for the (T)(P+A)-(T)(P+B) folding (PR sanitizer/81281)

[Jakub Jelinek <jakub at redhat dot com>]

On Fri, Dec 15, 2017 at 10:28:52AM +0100, Richard Biener wrote:
> > --- gcc/match.pd.jj	2017-12-07 14:00:51.083048186 +0100
> > +++ gcc/match.pd	2017-12-07 15:17:49.132784931 +0100
> > @@ -1784,8 +1784,8 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> >  
> >    /* (T)(P + A) - (T)P -> (T) A */
> >    (simplify
> > -   (minus (convert (plus:c @0 @1))
> > -    (convert @0))
> > +   (minus (convert (plus:c @@0 @1))
> > +    (convert? @0))
> >     (if (element_precision (type) <= element_precision (TREE_TYPE (@1))
> >  	/* For integer types, if A has a smaller type
> >  	   than T the result depends on the possible
> > @@ -1794,10 +1794,29 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> >  	   However, if an overflow in P + A would cause
> >  	   undefined behavior, we can assume that there
> >  	   is no overflow.  */
> > -	|| (INTEGRAL_TYPE_P (TREE_TYPE (@0))
> > -	    && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@0))))
> > +	|| (INTEGRAL_TYPE_P (TREE_TYPE (@1))
> > +	    && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@1))))
> 
> Given @1 and @@0 are in the same plus this change isn't technically
> necessary but it makes it clearer which type we look at (thus ok).

My understanding is that it is necessary, because @@0 could have different
type from @0 and TREE_TYPE (@0) is the type of where @0 is used rather
than @@0.

> >      (convert @1)))
> >    (simplify
> > +   (plus (convert (plus @1 INTEGER_CST@0)) INTEGER_CST@2)
> > +   (with { bool overflow;
> > +	   wide_int w = wi::neg (wi::to_wide (@2), &overflow); }
> > +    (if (wi::to_widest (@0) == widest_int::from (w, TYPE_SIGN (TREE_TYPE (@2)))
> > +	 && (!overflow
> > +	     || (INTEGRAL_TYPE_P (TREE_TYPE (@2))
> > +		 && TYPE_UNSIGNED (TREE_TYPE (@2))))
> > +	 && (element_precision (type) <= element_precision (TREE_TYPE (@1))
> > +	     /* For integer types, if A has a smaller type
> > +		than T the result depends on the possible
> > +		overflow in P + A.
> > +		E.g. T=size_t, A=(unsigned)429497295, P>0.
> > +		However, if an overflow in P + A would cause
> > +		undefined behavior, we can assume that there
> > +		is no overflow.  */
> > +	     || (INTEGRAL_TYPE_P (TREE_TYPE (@1))
> > +		 && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@1)))))
> 
> I think we don't need to worry about definedness of overflow.  All

We are talking about
(int) (x + 0x80000000U) + INT_MIN
I think you're right that we can still optimize that to (int) x.

> that matters is whether twos complement arithmetic will simplify
> the expression to (convert @1).  Specifically the possible overflow
> of the negation of @2 for the case element_precision (type) <= 
> element_precision (TREE_TYPE (@1)) shouldn't matter, likewise
> for the widening case (we'd never get the equality).
> 
> Don't we want to compare @0 and -@2 in the type of @2?  Like
> for (unsigned int)(unsigned-long-x + 0x100000005) + -5U which
> we should be able to simplify?  For the widening case that would
> work as well as far as I can see?

So, we can have several cases, the narrowing one, e.g.:
(int)(unsigned-long-long-x + 0x100000005ULL) + -5
(unsigned)(long-long-x + 0x100000005LL) + -5U
(int)(unsigned-long-long-x + 0x1fffffffbULL) + 5
(unsigned)(long-long-x + 0x1fffffffbLL) + 5U
same precision:
(int)(unsigned-x + 5U) + -5
(unsigned)(int-x + 5) + -5U
(int)(unsigned-x + -5U) + 5
(unsigned)(int-x + -5) + 5
and widening ones:
(long long)(int-x + 5) + -5LL
(unsigned long long)(int-x + 5) + -5ULL
(long long)(int-x + -5) + 5LL
(unsigned long long)(int-x + -5) + 5ULL
You mean we should effectively (though on wide_int/widest_int)
fold_unary (MINUS_EXPR, TREE_TYPE (@2), fold_convert (TREE_TYPE (@2), @0))
and compare that to @2?

> If you can split out this new pattern the rest is ok with honoring
> the comment below.

Ok (will need to comment out the corresponding testcase, done below).

> > -	 || (INTEGRAL_TYPE_P (TREE_TYPE (@0))
> > -	     && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@0))))
> > +    (if (((element_precision (type) <= element_precision (TREE_TYPE (@1)))
> > +	  == (element_precision (type) <= element_precision (TREE_TYPE (@1))))

Yeah, this @1 above should have been @2.  Thanks for catching this.

So for now like this?

2017-12-15  Jakub Jelinek  <jakub@redhat.com>

	PR sanitizer/81281
	* match.pd ((T)(P + A) - (T)P -> (T) A): Use @@0 instead of @0 and
	convert? on @0 instead of convert.  Check type of @1, not @0.
	((T)P - (T)(P + A) -> -(T) A): Use @@0 instead of @0 and
	convert? on @0 instead of convert.  Check type of @1, not @0.
	((T)(P + A) - (T)(P + B) -> (T)A - (T)B): Use @@0 instead of @0,
	only optimize if either both @1 and @2 types are narrower
	precision, or both are wider or equal precision, and in the former
	case only if both have undefined overflow.

	* gcc.dg/pr81281-3.c: New test.

--- gcc/match.pd.jj	2017-12-07 18:04:54.580750329 +0100
+++ gcc/match.pd	2017-12-15 11:52:22.582118364 +0100
@@ -1784,8 +1784,8 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
 
   /* (T)(P + A) - (T)P -> (T) A */
   (simplify
-   (minus (convert (plus:c @0 @1))
-    (convert @0))
+   (minus (convert (plus:c @@0 @1))
+    (convert? @0))
    (if (element_precision (type) <= element_precision (TREE_TYPE (@1))
 	/* For integer types, if A has a smaller type
 	   than T the result depends on the possible
@@ -1794,8 +1794,8 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
 	   However, if an overflow in P + A would cause
 	   undefined behavior, we can assume that there
 	   is no overflow.  */
-	|| (INTEGRAL_TYPE_P (TREE_TYPE (@0))
-	    && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@0))))
+	|| (INTEGRAL_TYPE_P (TREE_TYPE (@1))
+	    && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@1))))
     (convert @1)))
   (simplify
    (minus (convert (pointer_plus @@0 @1))
@@ -1818,8 +1818,8 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
 
   /* (T)P - (T)(P + A) -> -(T) A */
   (simplify
-   (minus (convert @0)
-    (convert (plus:c @0 @1)))
+   (minus (convert? @0)
+    (convert (plus:c @@0 @1)))
    (if (INTEGRAL_TYPE_P (type)
 	&& TYPE_OVERFLOW_UNDEFINED (type)
 	&& element_precision (type) <= element_precision (TREE_TYPE (@1)))
@@ -1833,8 +1833,8 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
 	    However, if an overflow in P + A would cause
 	    undefined behavior, we can assume that there
 	    is no overflow.  */
-	 || (INTEGRAL_TYPE_P (TREE_TYPE (@0))
-	     && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@0))))
+	 || (INTEGRAL_TYPE_P (TREE_TYPE (@1))
+	     && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@1))))
      (negate (convert @1)))))
   (simplify
    (minus (convert @0)
@@ -1862,23 +1862,28 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
 
   /* (T)(P + A) - (T)(P + B) -> (T)A - (T)B */
   (simplify
-   (minus (convert (plus:c @0 @1))
+   (minus (convert (plus:c @@0 @1))
     (convert (plus:c @0 @2)))
    (if (INTEGRAL_TYPE_P (type)
 	&& TYPE_OVERFLOW_UNDEFINED (type)
-	&& element_precision (type) <= element_precision (TREE_TYPE (@1)))
+	&& element_precision (type) <= element_precision (TREE_TYPE (@1))
+	&& element_precision (type) <= element_precision (TREE_TYPE (@2)))
     (with { tree utype = unsigned_type_for (type); }
      (convert (minus (convert:utype @1) (convert:utype @2))))
-    (if (element_precision (type) <= element_precision (TREE_TYPE (@1))
-	 /* For integer types, if A has a smaller type
-	    than T the result depends on the possible
-	    overflow in P + A.
-	    E.g. T=size_t, A=(unsigned)429497295, P>0.
-	    However, if an overflow in P + A would cause
-	    undefined behavior, we can assume that there
-	    is no overflow.  */
-	 || (INTEGRAL_TYPE_P (TREE_TYPE (@0))
-	     && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@0))))
+    (if (((element_precision (type) <= element_precision (TREE_TYPE (@1)))
+	  == (element_precision (type) <= element_precision (TREE_TYPE (@2))))
+	 && (element_precision (type) <= element_precision (TREE_TYPE (@1))
+	     /* For integer types, if A has a smaller type
+		than T the result depends on the possible
+		overflow in P + A.
+		E.g. T=size_t, A=(unsigned)429497295, P>0.
+		However, if an overflow in P + A would cause
+		undefined behavior, we can assume that there
+		is no overflow.  */
+	     || (INTEGRAL_TYPE_P (TREE_TYPE (@1))
+		 && INTEGRAL_TYPE_P (TREE_TYPE (@2))
+		 && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@1))
+		 && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@2)))))
      (minus (convert @1) (convert @2)))))
   (simplify
    (minus (convert (pointer_plus @@0 @1))
--- gcc/testsuite/gcc.dg/pr81281-3.c.jj	2017-12-15 11:51:38.294654394 +0100
+++ gcc/testsuite/gcc.dg/pr81281-3.c	2017-12-15 11:56:08.097388860 +0100
@@ -0,0 +1,105 @@
+/* PR sanitizer/81281 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-optimized" } */
+/* { dg-final { scan-tree-dump-not "\[+=-] \?123\[ ;]" "optimized" } } */
+
+#ifdef __SIZEOF_INT128__
+__int128
+f1 (int a, long long b)
+{
+  __int128 f = 123 + a;
+  __int128 g = 123 + b;
+  return f - g;
+}
+#endif
+
+signed char
+f2 (int a, long long b)
+{
+  signed char f = 123 + a;
+  signed char g = 123 + b;
+  return f - g;
+}
+
+signed char
+f3 (unsigned int a, unsigned long long b)
+{
+  signed char f = 123 + a;
+  signed char g = 123 + b;
+  return f - g;
+}
+
+unsigned char
+f4 (unsigned int a, unsigned long long b)
+{
+  unsigned char f = 123 + a;
+  unsigned char g = 123 + b;
+  return f - g;
+}
+
+/* This isn't optimized yet.  */
+#if 0
+long long
+f5 (int a)
+{
+  long long f = 123 + a;
+  long long g = 123;
+  return f - g;
+}
+#endif
+
+signed char
+f6 (long long a)
+{
+  signed char f = 123 + a;
+  signed char g = 123;
+  return f - g;
+}
+
+signed char
+f7 (unsigned int a)
+{
+  signed char f = 123 + a;
+  signed char g = 123;
+  return f - g;
+}
+
+unsigned char
+f8 (unsigned long int a)
+{
+  unsigned char f = 123 + a;
+  unsigned char g = 123;
+  return f - g;
+}
+
+long long
+f9 (int a)
+{
+  long long f = 123;
+  long long g = 123 + a;
+  return f - g;
+}
+
+signed char
+f10 (long long a)
+{
+  signed char f = 123;
+  signed char g = 123 + a;
+  return f - g;
+}
+
+signed char
+f11 (unsigned int a)
+{
+  signed char f = 123;
+  signed char g = 123 + a;
+  return f - g;
+}
+
+unsigned char
+f12 (unsigned long int a)
+{
+  unsigned char f = 123;
+  unsigned char g = 123 + a;
+  return f - g;
+}


	Jakub