Avoid creating overflows in match.pd (P + A) - (P + B) POINTER_DIFF_EXPR optimization (PR c/61240)

Message ID 20180117201556.GZ2063@tucnak
State New
Headers show
Series
  • Avoid creating overflows in match.pd (P + A) - (P + B) POINTER_DIFF_EXPR optimization (PR c/61240)
Related show

Commit Message

Jakub Jelinek Jan. 17, 2018, 8:15 p.m.
Hi!

POINTER_DIFF_EXPR returns a signed integer, but sadly POINTER_PLUS_EXPR
second arguments are unsigned integers, so if we are adding negative numbers
to pointers, those are very large numbers and we get TREE_OVERFLOW which the
C FE then during c_fully_fold diagnoses.

We want to treat the numbers as signed integers for this purpose, using
VCE seems easiest way to get rid of the unwanted overflows.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Or do you prefer manual fold_converts in the (with { }) block + test
for INTEGER_CST && TREE_OVERFLOW + drop_tree_overflow instead?

2018-01-17  Jakub Jelinek  <jakub@redhat.com>

	PR c/61240
	* match.pd ((P + A) - P, P - (P + A), (P + A) - (P + B)): For
	pointer_diff optimizations use view_convert instead of convert.

	* gcc.dg/pr61240.c: New test.


	Jakub

Comments

Richard Biener Jan. 18, 2018, 8:12 a.m. | #1
On Wed, 17 Jan 2018, Jakub Jelinek wrote:

> Hi!

> 

> POINTER_DIFF_EXPR returns a signed integer, but sadly POINTER_PLUS_EXPR

> second arguments are unsigned integers, so if we are adding negative numbers

> to pointers, those are very large numbers and we get TREE_OVERFLOW which the

> C FE then during c_fully_fold diagnoses.


Bah...

> We want to treat the numbers as signed integers for this purpose, using

> VCE seems easiest way to get rid of the unwanted overflows.

> 

> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

> 

> Or do you prefer manual fold_converts in the (with { }) block + test

> for INTEGER_CST && TREE_OVERFLOW + drop_tree_overflow instead?


I think the patch is ok as-is.  I really hope to make the
POINTER_PLUS_EXPR offset operand signed in next stage1.

Thanks,
Richard.


> 2018-01-17  Jakub Jelinek  <jakub@redhat.com>

> 

> 	PR c/61240

> 	* match.pd ((P + A) - P, P - (P + A), (P + A) - (P + B)): For

> 	pointer_diff optimizations use view_convert instead of convert.

> 

> 	* gcc.dg/pr61240.c: New test.

> 

> --- gcc/match.pd.jj	2018-01-15 10:02:04.000000000 +0100

> +++ gcc/match.pd	2018-01-17 17:10:54.855061485 +0100

> @@ -1832,7 +1832,10 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)

>      /* The second argument of pointer_plus must be interpreted as signed, and

>         thus sign-extended if necessary.  */

>      (with { tree stype = signed_type_for (TREE_TYPE (@1)); }

> -     (convert (convert:stype @1))))

> +     /* Use view_convert instead of convert here, as POINTER_PLUS_EXPR

> +	second arg is unsigned even when we need to consider it as signed,

> +	we don't want to diagnose overflow here.  */

> +     (convert (view_convert:stype @1))))

>  

>    /* (T)P - (T)(P + A) -> -(T) A */

>    (simplify

> @@ -1876,7 +1879,10 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)

>      /* The second argument of pointer_plus must be interpreted as signed, and

>         thus sign-extended if necessary.  */

>      (with { tree stype = signed_type_for (TREE_TYPE (@1)); }

> -     (negate (convert (convert:stype @1)))))

> +     /* Use view_convert instead of convert here, as POINTER_PLUS_EXPR

> +	second arg is unsigned even when we need to consider it as signed,

> +	we don't want to diagnose overflow here.  */

> +     (negate (convert (view_convert:stype @1)))))

>  

>    /* (T)(P + A) - (T)(P + B) -> (T)A - (T)B */

>    (simplify

> @@ -1927,7 +1933,11 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)

>      /* The second argument of pointer_plus must be interpreted as signed, and

>         thus sign-extended if necessary.  */

>      (with { tree stype = signed_type_for (TREE_TYPE (@1)); }

> -     (minus (convert (convert:stype @1)) (convert (convert:stype @2)))))))

> +     /* Use view_convert instead of convert here, as POINTER_PLUS_EXPR

> +	second arg is unsigned even when we need to consider it as signed,

> +	we don't want to diagnose overflow here.  */

> +     (minus (convert (view_convert:stype @1))

> +	    (convert (view_convert:stype @2)))))))

>  

>  

>  /* Simplifications of MIN_EXPR, MAX_EXPR, fmin() and fmax().  */

> --- gcc/testsuite/gcc.dg/pr61240.c.jj	2018-01-17 17:25:45.821030898 +0100

> +++ gcc/testsuite/gcc.dg/pr61240.c	2018-01-17 17:26:24.118029550 +0100

> @@ -0,0 +1,20 @@

> +/* PR c/61240 */

> +/* { dg-do compile } */

> +

> +typedef __PTRDIFF_TYPE__ ptrdiff_t;

> +

> +ptrdiff_t

> +foo (ptrdiff_t a[4])

> +{

> +  int i[4];

> +  int *p = i + 2;

> +  static ptrdiff_t b = p - (p - 1);	/* { dg-bogus "integer overflow in expression" } */

> +  static ptrdiff_t c = (p - 1) - p;	/* { dg-bogus "integer overflow in expression" } */

> +  static ptrdiff_t d = (p - 2) - (p - 1);/* { dg-bogus "integer overflow in expression" } */

> +  static ptrdiff_t e = (p - 1) - (p - 2);/* { dg-bogus "integer overflow in expression" } */

> +  a[0] = p - (p - 1);			/* { dg-bogus "integer overflow in expression" } */

> +  a[1] = (p - 1) - p;			/* { dg-bogus "integer overflow in expression" } */

> +  a[2] = (p - 2) - (p - 1);		/* { dg-bogus "integer overflow in expression" } */

> +  a[3] = (p - 1) - (p - 2);		/* { dg-bogus "integer overflow in expression" } */

> +  return b + c + d + e;

> +}

> 

> 	Jakub

> 

> 


-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)

Patch

--- gcc/match.pd.jj	2018-01-15 10:02:04.000000000 +0100
+++ gcc/match.pd	2018-01-17 17:10:54.855061485 +0100
@@ -1832,7 +1832,10 @@  DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
     /* The second argument of pointer_plus must be interpreted as signed, and
        thus sign-extended if necessary.  */
     (with { tree stype = signed_type_for (TREE_TYPE (@1)); }
-     (convert (convert:stype @1))))
+     /* Use view_convert instead of convert here, as POINTER_PLUS_EXPR
+	second arg is unsigned even when we need to consider it as signed,
+	we don't want to diagnose overflow here.  */
+     (convert (view_convert:stype @1))))
 
   /* (T)P - (T)(P + A) -> -(T) A */
   (simplify
@@ -1876,7 +1879,10 @@  DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
     /* The second argument of pointer_plus must be interpreted as signed, and
        thus sign-extended if necessary.  */
     (with { tree stype = signed_type_for (TREE_TYPE (@1)); }
-     (negate (convert (convert:stype @1)))))
+     /* Use view_convert instead of convert here, as POINTER_PLUS_EXPR
+	second arg is unsigned even when we need to consider it as signed,
+	we don't want to diagnose overflow here.  */
+     (negate (convert (view_convert:stype @1)))))
 
   /* (T)(P + A) - (T)(P + B) -> (T)A - (T)B */
   (simplify
@@ -1927,7 +1933,11 @@  DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
     /* The second argument of pointer_plus must be interpreted as signed, and
        thus sign-extended if necessary.  */
     (with { tree stype = signed_type_for (TREE_TYPE (@1)); }
-     (minus (convert (convert:stype @1)) (convert (convert:stype @2)))))))
+     /* Use view_convert instead of convert here, as POINTER_PLUS_EXPR
+	second arg is unsigned even when we need to consider it as signed,
+	we don't want to diagnose overflow here.  */
+     (minus (convert (view_convert:stype @1))
+	    (convert (view_convert:stype @2)))))))
 
 
 /* Simplifications of MIN_EXPR, MAX_EXPR, fmin() and fmax().  */
--- gcc/testsuite/gcc.dg/pr61240.c.jj	2018-01-17 17:25:45.821030898 +0100
+++ gcc/testsuite/gcc.dg/pr61240.c	2018-01-17 17:26:24.118029550 +0100
@@ -0,0 +1,20 @@ 
+/* PR c/61240 */
+/* { dg-do compile } */
+
+typedef __PTRDIFF_TYPE__ ptrdiff_t;
+
+ptrdiff_t
+foo (ptrdiff_t a[4])
+{
+  int i[4];
+  int *p = i + 2;
+  static ptrdiff_t b = p - (p - 1);	/* { dg-bogus "integer overflow in expression" } */
+  static ptrdiff_t c = (p - 1) - p;	/* { dg-bogus "integer overflow in expression" } */
+  static ptrdiff_t d = (p - 2) - (p - 1);/* { dg-bogus "integer overflow in expression" } */
+  static ptrdiff_t e = (p - 1) - (p - 2);/* { dg-bogus "integer overflow in expression" } */
+  a[0] = p - (p - 1);			/* { dg-bogus "integer overflow in expression" } */
+  a[1] = (p - 1) - p;			/* { dg-bogus "integer overflow in expression" } */
+  a[2] = (p - 2) - (p - 1);		/* { dg-bogus "integer overflow in expression" } */
+  a[3] = (p - 1) - (p - 2);		/* { dg-bogus "integer overflow in expression" } */
+  return b + c + d + e;
+}