diff mbox

Fix match.pd narrowing opt (PR tree-optimization/66187)

Message ID 20150518194836.GW1751@tucnak.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek May 18, 2015, 7:48 p.m. UTC
Hi!

As the testcases show, for signed types we really should use SIGNED rather
than UNSIGNED as tree_int_cst_min_precision argument, that function doesn't
really do the desired thing with UNSIGNED for negative values and with
-fwrapv we just want the narrowing cast to not lose anything from the
number.  Additionally, as for TYPE_OVERFLOW_WRAPS case we perform the +/-/& in
unsigned type,  we have to avoid all negative masks, because those surely
have the higher bits set.

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

For plus and unsigned type, we could improve it even further, as in that
case we could just check if the bit immediately above precision is clear
in the mask, higher bits than that are uninteresting, because the addition
will have always zeros there.  Perhaps as a follow-up?
I mean say uchar foo (uchar a, uchar b) { return (x + y) & 0x72fe; }
can be done as uchar addition & 0xfe, but not e.g. & 0x71fe.

2015-05-18  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/66187
	* match.pd ((bit_and (plus/minus (convert @0) (convert @1)) mask)):
	Pass TYPE_SIGN to tree_int_cst_min_precision.  If
	!TYPE_OVERFLOW_WRAPS, ensure @4 is non-negative.

	* gcc.c-torture/execute/pr66187.c: New test.
	* gcc.dg/pr66187-1.c: New test.
	* gcc.dg/pr66187-2.c: New test.


	Jakub

Comments

Richard Biener May 19, 2015, 8:52 a.m. UTC | #1
On Mon, 18 May 2015, Jakub Jelinek wrote:

> Hi!
> 
> As the testcases show, for signed types we really should use SIGNED rather
> than UNSIGNED as tree_int_cst_min_precision argument, that function doesn't
> really do the desired thing with UNSIGNED for negative values and with
> -fwrapv we just want the narrowing cast to not lose anything from the
> number.  Additionally, as for TYPE_OVERFLOW_WRAPS case we perform the +/-/& in
> unsigned type,  we have to avoid all negative masks, because those surely
> have the higher bits set.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Ok.

Thanks,
Richard.

> For plus and unsigned type, we could improve it even further, as in that
> case we could just check if the bit immediately above precision is clear
> in the mask, higher bits than that are uninteresting, because the addition
> will have always zeros there.  Perhaps as a follow-up?
> I mean say uchar foo (uchar a, uchar b) { return (x + y) & 0x72fe; }
> can be done as uchar addition & 0xfe, but not e.g. & 0x71fe.
> 
> 2015-05-18  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR tree-optimization/66187
> 	* match.pd ((bit_and (plus/minus (convert @0) (convert @1)) mask)):
> 	Pass TYPE_SIGN to tree_int_cst_min_precision.  If
> 	!TYPE_OVERFLOW_WRAPS, ensure @4 is non-negative.
> 
> 	* gcc.c-torture/execute/pr66187.c: New test.
> 	* gcc.dg/pr66187-1.c: New test.
> 	* gcc.dg/pr66187-2.c: New test.
> 
> --- gcc/match.pd.jj	2015-05-18 09:46:39.000000000 +0200
> +++ gcc/match.pd	2015-05-18 10:20:24.944475737 +0200
> @@ -1115,8 +1115,10 @@ (define_operator_list CBRT BUILT_IN_CBRT
>  	 /* The inner conversion must be a widening conversion.  */
>  	 && TYPE_PRECISION (TREE_TYPE (@2)) > TYPE_PRECISION (TREE_TYPE (@0))
>  	 && types_match (@0, @1)
> -	 && (tree_int_cst_min_precision (@4, UNSIGNED)
> +	 && (tree_int_cst_min_precision (@4, TYPE_SIGN (TREE_TYPE (@0)))
>  	     <= TYPE_PRECISION (TREE_TYPE (@0)))
> +	 && (TYPE_OVERFLOW_WRAPS (TREE_TYPE (@0))
> +	     || tree_int_cst_sgn (@4) >= 0)
>  	 && single_use (@5))
>        (if (TYPE_OVERFLOW_WRAPS (TREE_TYPE (@0)))
>  	(with { tree ntype = TREE_TYPE (@0); }
> --- gcc/testsuite/gcc.c-torture/execute/pr66187.c.jj	2015-05-18 10:53:43.953654893 +0200
> +++ gcc/testsuite/gcc.c-torture/execute/pr66187.c	2015-05-18 10:53:28.000000000 +0200
> @@ -0,0 +1,16 @@
> +/* PR tree-optimization/66187 */
> +
> +int a = 1, e = -1;
> +short b, f;
> +
> +int
> +main ()
> +{
> +  f = e;
> +  int g = b < 0 ? 0 : f + b;
> +  if ((g & -4) < 0)
> +    a = 0;
> +  if (a)
> +    __builtin_abort ();
> +  return 0;
> +}
> --- gcc/testsuite/gcc.dg/pr66187-1.c.jj	2015-05-18 10:54:30.677906029 +0200
> +++ gcc/testsuite/gcc.dg/pr66187-1.c	2015-05-18 12:30:17.000000000 +0200
> @@ -0,0 +1,97 @@
> +/* PR tree-optimization/66187 */
> +/* { dg-do run } */
> +/* { dg-options "-O2 -fno-wrapv" } */
> +
> +__attribute__((noinline, noclone)) int
> +f0 (unsigned char x, unsigned char y)
> +{
> +  return (x + y) & 0x2ff;
> +}
> +
> +__attribute__((noinline, noclone)) int
> +f1 (unsigned char x, unsigned char y)
> +{
> +  return (x - y) & 0x2ff;
> +}
> +
> +__attribute__((noinline, noclone)) int
> +f2 (signed char x, signed char y)
> +{
> +  return (x + y) & -4;
> +}
> +
> +__attribute__((noinline, noclone)) int
> +f3 (signed char x, signed char y)
> +{
> +  return (x + y) & 0xf8;
> +}
> +
> +__attribute__((noinline, noclone)) int
> +f4 (signed char x, signed char y)
> +{
> +  return (x + y) & 0x78;
> +}
> +
> +__attribute__((noinline, noclone)) int
> +f5 (unsigned char x, unsigned char y)
> +{
> +  int a = x;
> +  int b = y;
> +  int c = a + b;
> +  return c & 0x2ff;
> +}
> +
> +__attribute__((noinline, noclone)) int
> +f6 (unsigned char x, unsigned char y)
> +{
> +  int a = x;
> +  int b = y;
> +  int c = a - b;
> +  return c & 0x2ff;
> +}
> +
> +__attribute__((noinline, noclone)) int
> +f7 (signed char x, signed char y)
> +{
> +  int a = x;
> +  int b = y;
> +  int c = a + b;
> +  return c & -4;
> +}
> +
> +__attribute__((noinline, noclone)) int
> +f8 (signed char x, signed char y)
> +{
> +  int a = x;
> +  int b = y;
> +  int c = a + b;
> +  return c & 0xf8;
> +}
> +
> +__attribute__((noinline, noclone)) int
> +f9 (signed char x, signed char y)
> +{
> +  int a = x;
> +  int b = y;
> +  int c = a + b;
> +  return c & 0x78;
> +}
> +
> +int
> +main ()
> +{
> +  if (__SCHAR_MAX__ != 127 || sizeof (int) != 4)
> +    return 0;
> +  if (f0 (0xff, 0xff) != 0xfe
> +      || f1 (0, 1) != 0x2ff
> +      || f2 (-2, 1) != -4
> +      || f3 (-2, 1) != 0xf8
> +      || f4 (-2, 1) != 0x78
> +      || f5 (0xff, 0xff) != 0xfe
> +      || f6 (0, 1) != 0x2ff
> +      || f7 (-2, 1) != -4
> +      || f8 (-2, 1) != 0xf8
> +      || f9 (-2, 1) != 0x78)
> +    __builtin_abort ();
> +  return 0;
> +}
> --- gcc/testsuite/gcc.dg/pr66187-2.c.jj	2015-05-18 12:31:39.398176973 +0200
> +++ gcc/testsuite/gcc.dg/pr66187-2.c	2015-05-18 12:31:46.319067151 +0200
> @@ -0,0 +1,97 @@
> +/* PR tree-optimization/66187 */
> +/* { dg-do run } */
> +/* { dg-options "-O2 -fwrapv" } */
> +
> +__attribute__((noinline, noclone)) int
> +f0 (unsigned char x, unsigned char y)
> +{
> +  return (x + y) & 0x2ff;
> +}
> +
> +__attribute__((noinline, noclone)) int
> +f1 (unsigned char x, unsigned char y)
> +{
> +  return (x - y) & 0x2ff;
> +}
> +
> +__attribute__((noinline, noclone)) int
> +f2 (signed char x, signed char y)
> +{
> +  return (x + y) & -4;
> +}
> +
> +__attribute__((noinline, noclone)) int
> +f3 (signed char x, signed char y)
> +{
> +  return (x + y) & 0xf8;
> +}
> +
> +__attribute__((noinline, noclone)) int
> +f4 (signed char x, signed char y)
> +{
> +  return (x + y) & 0x78;
> +}
> +
> +__attribute__((noinline, noclone)) int
> +f5 (unsigned char x, unsigned char y)
> +{
> +  int a = x;
> +  int b = y;
> +  int c = a + b;
> +  return c & 0x2ff;
> +}
> +
> +__attribute__((noinline, noclone)) int
> +f6 (unsigned char x, unsigned char y)
> +{
> +  int a = x;
> +  int b = y;
> +  int c = a - b;
> +  return c & 0x2ff;
> +}
> +
> +__attribute__((noinline, noclone)) int
> +f7 (signed char x, signed char y)
> +{
> +  int a = x;
> +  int b = y;
> +  int c = a + b;
> +  return c & -4;
> +}
> +
> +__attribute__((noinline, noclone)) int
> +f8 (signed char x, signed char y)
> +{
> +  int a = x;
> +  int b = y;
> +  int c = a + b;
> +  return c & 0xf8;
> +}
> +
> +__attribute__((noinline, noclone)) int
> +f9 (signed char x, signed char y)
> +{
> +  int a = x;
> +  int b = y;
> +  int c = a + b;
> +  return c & 0x78;
> +}
> +
> +int
> +main ()
> +{
> +  if (__SCHAR_MAX__ != 127 || sizeof (int) != 4)
> +    return 0;
> +  if (f0 (0xff, 0xff) != 0xfe
> +      || f1 (0, 1) != 0x2ff
> +      || f2 (-2, 1) != -4
> +      || f3 (-2, 1) != 0xf8
> +      || f4 (-2, 1) != 0x78
> +      || f5 (0xff, 0xff) != 0xfe
> +      || f6 (0, 1) != 0x2ff
> +      || f7 (-2, 1) != -4
> +      || f8 (-2, 1) != 0xf8
> +      || f9 (-2, 1) != 0x78)
> +    __builtin_abort ();
> +  return 0;
> +}
> 
> 	Jakub
> 
>
diff mbox

Patch

--- gcc/match.pd.jj	2015-05-18 09:46:39.000000000 +0200
+++ gcc/match.pd	2015-05-18 10:20:24.944475737 +0200
@@ -1115,8 +1115,10 @@  (define_operator_list CBRT BUILT_IN_CBRT
 	 /* The inner conversion must be a widening conversion.  */
 	 && TYPE_PRECISION (TREE_TYPE (@2)) > TYPE_PRECISION (TREE_TYPE (@0))
 	 && types_match (@0, @1)
-	 && (tree_int_cst_min_precision (@4, UNSIGNED)
+	 && (tree_int_cst_min_precision (@4, TYPE_SIGN (TREE_TYPE (@0)))
 	     <= TYPE_PRECISION (TREE_TYPE (@0)))
+	 && (TYPE_OVERFLOW_WRAPS (TREE_TYPE (@0))
+	     || tree_int_cst_sgn (@4) >= 0)
 	 && single_use (@5))
       (if (TYPE_OVERFLOW_WRAPS (TREE_TYPE (@0)))
 	(with { tree ntype = TREE_TYPE (@0); }
--- gcc/testsuite/gcc.c-torture/execute/pr66187.c.jj	2015-05-18 10:53:43.953654893 +0200
+++ gcc/testsuite/gcc.c-torture/execute/pr66187.c	2015-05-18 10:53:28.000000000 +0200
@@ -0,0 +1,16 @@ 
+/* PR tree-optimization/66187 */
+
+int a = 1, e = -1;
+short b, f;
+
+int
+main ()
+{
+  f = e;
+  int g = b < 0 ? 0 : f + b;
+  if ((g & -4) < 0)
+    a = 0;
+  if (a)
+    __builtin_abort ();
+  return 0;
+}
--- gcc/testsuite/gcc.dg/pr66187-1.c.jj	2015-05-18 10:54:30.677906029 +0200
+++ gcc/testsuite/gcc.dg/pr66187-1.c	2015-05-18 12:30:17.000000000 +0200
@@ -0,0 +1,97 @@ 
+/* PR tree-optimization/66187 */
+/* { dg-do run } */
+/* { dg-options "-O2 -fno-wrapv" } */
+
+__attribute__((noinline, noclone)) int
+f0 (unsigned char x, unsigned char y)
+{
+  return (x + y) & 0x2ff;
+}
+
+__attribute__((noinline, noclone)) int
+f1 (unsigned char x, unsigned char y)
+{
+  return (x - y) & 0x2ff;
+}
+
+__attribute__((noinline, noclone)) int
+f2 (signed char x, signed char y)
+{
+  return (x + y) & -4;
+}
+
+__attribute__((noinline, noclone)) int
+f3 (signed char x, signed char y)
+{
+  return (x + y) & 0xf8;
+}
+
+__attribute__((noinline, noclone)) int
+f4 (signed char x, signed char y)
+{
+  return (x + y) & 0x78;
+}
+
+__attribute__((noinline, noclone)) int
+f5 (unsigned char x, unsigned char y)
+{
+  int a = x;
+  int b = y;
+  int c = a + b;
+  return c & 0x2ff;
+}
+
+__attribute__((noinline, noclone)) int
+f6 (unsigned char x, unsigned char y)
+{
+  int a = x;
+  int b = y;
+  int c = a - b;
+  return c & 0x2ff;
+}
+
+__attribute__((noinline, noclone)) int
+f7 (signed char x, signed char y)
+{
+  int a = x;
+  int b = y;
+  int c = a + b;
+  return c & -4;
+}
+
+__attribute__((noinline, noclone)) int
+f8 (signed char x, signed char y)
+{
+  int a = x;
+  int b = y;
+  int c = a + b;
+  return c & 0xf8;
+}
+
+__attribute__((noinline, noclone)) int
+f9 (signed char x, signed char y)
+{
+  int a = x;
+  int b = y;
+  int c = a + b;
+  return c & 0x78;
+}
+
+int
+main ()
+{
+  if (__SCHAR_MAX__ != 127 || sizeof (int) != 4)
+    return 0;
+  if (f0 (0xff, 0xff) != 0xfe
+      || f1 (0, 1) != 0x2ff
+      || f2 (-2, 1) != -4
+      || f3 (-2, 1) != 0xf8
+      || f4 (-2, 1) != 0x78
+      || f5 (0xff, 0xff) != 0xfe
+      || f6 (0, 1) != 0x2ff
+      || f7 (-2, 1) != -4
+      || f8 (-2, 1) != 0xf8
+      || f9 (-2, 1) != 0x78)
+    __builtin_abort ();
+  return 0;
+}
--- gcc/testsuite/gcc.dg/pr66187-2.c.jj	2015-05-18 12:31:39.398176973 +0200
+++ gcc/testsuite/gcc.dg/pr66187-2.c	2015-05-18 12:31:46.319067151 +0200
@@ -0,0 +1,97 @@ 
+/* PR tree-optimization/66187 */
+/* { dg-do run } */
+/* { dg-options "-O2 -fwrapv" } */
+
+__attribute__((noinline, noclone)) int
+f0 (unsigned char x, unsigned char y)
+{
+  return (x + y) & 0x2ff;
+}
+
+__attribute__((noinline, noclone)) int
+f1 (unsigned char x, unsigned char y)
+{
+  return (x - y) & 0x2ff;
+}
+
+__attribute__((noinline, noclone)) int
+f2 (signed char x, signed char y)
+{
+  return (x + y) & -4;
+}
+
+__attribute__((noinline, noclone)) int
+f3 (signed char x, signed char y)
+{
+  return (x + y) & 0xf8;
+}
+
+__attribute__((noinline, noclone)) int
+f4 (signed char x, signed char y)
+{
+  return (x + y) & 0x78;
+}
+
+__attribute__((noinline, noclone)) int
+f5 (unsigned char x, unsigned char y)
+{
+  int a = x;
+  int b = y;
+  int c = a + b;
+  return c & 0x2ff;
+}
+
+__attribute__((noinline, noclone)) int
+f6 (unsigned char x, unsigned char y)
+{
+  int a = x;
+  int b = y;
+  int c = a - b;
+  return c & 0x2ff;
+}
+
+__attribute__((noinline, noclone)) int
+f7 (signed char x, signed char y)
+{
+  int a = x;
+  int b = y;
+  int c = a + b;
+  return c & -4;
+}
+
+__attribute__((noinline, noclone)) int
+f8 (signed char x, signed char y)
+{
+  int a = x;
+  int b = y;
+  int c = a + b;
+  return c & 0xf8;
+}
+
+__attribute__((noinline, noclone)) int
+f9 (signed char x, signed char y)
+{
+  int a = x;
+  int b = y;
+  int c = a + b;
+  return c & 0x78;
+}
+
+int
+main ()
+{
+  if (__SCHAR_MAX__ != 127 || sizeof (int) != 4)
+    return 0;
+  if (f0 (0xff, 0xff) != 0xfe
+      || f1 (0, 1) != 0x2ff
+      || f2 (-2, 1) != -4
+      || f3 (-2, 1) != 0xf8
+      || f4 (-2, 1) != 0x78
+      || f5 (0xff, 0xff) != 0xfe
+      || f6 (0, 1) != 0x2ff
+      || f7 (-2, 1) != -4
+      || f8 (-2, 1) != 0xf8
+      || f9 (-2, 1) != 0x78)
+    __builtin_abort ();
+  return 0;
+}