Patchwork Fix up X < (cast) (1 << Y) folding (PR tree-optimization/56051)

login
register
mail settings
Submitter Jakub Jelinek
Date Jan. 20, 2013, 7:46 p.m.
Message ID <20130120194658.GF7269@tucnak.redhat.com>
Download mbox | patch
Permalink /patch/213988/
State New
Headers show

Comments

Jakub Jelinek - Jan. 20, 2013, 7:46 p.m.
Hi!

As the first hunk in the testcase shows, we can't perform this optimization
if the conversion is narrowing.
As the second hunk shows, if we allow 1 << 31 (we don't optimize
int foo (int x) { return (1 << x) < 0; } so I think it would be surprising
if we did optimize it in this case, plus I'm afraid it could break a lot
of code if we started optimizing (1 << x) < 0 into 0), we can't optimize
a widening conversion from signed to unsigned either.

Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
trunk?

For 4.9, I think we should see why e.g.
X < (unsigned) (1 << Y)
optimization isn't performed e.g. when X is just an unsigned int variable
(rather than say ARRAY_REF as in the testcase), probably we need to handle
it for arg1 being X and arg0 being (1 << Y) too.

2013-01-20  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/56051
	* fold-const.c (fold_binary_loc): Don't fold
	X < (cast) (1 << Y) into (X >> Y) != 0 if cast is either
	a narrowing conversion, or widening conversion from signed
	to unsigned.

	* gcc.c-torture/execute/pr56051.c: New test.


	Jakub
Jeff Law - Jan. 21, 2013, 5:05 p.m.
On 01/20/2013 12:46 PM, Jakub Jelinek wrote:
> Hi!
>
> As the first hunk in the testcase shows, we can't perform this optimization
> if the conversion is narrowing.
> As the second hunk shows, if we allow 1 << 31 (we don't optimize
> int foo (int x) { return (1 << x) < 0; } so I think it would be surprising
> if we did optimize it in this case, plus I'm afraid it could break a lot
> of code if we started optimizing (1 << x) < 0 into 0), we can't optimize
> a widening conversion from signed to unsigned either.
>
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> trunk?
>
> For 4.9, I think we should see why e.g.
> X < (unsigned) (1 << Y)
> optimization isn't performed e.g. when X is just an unsigned int variable
> (rather than say ARRAY_REF as in the testcase), probably we need to handle
> it for arg1 being X and arg0 being (1 << Y) too.
>
> 2013-01-20  Jakub Jelinek  <jakub@redhat.com>
>
> 	PR tree-optimization/56051
> 	* fold-const.c (fold_binary_loc): Don't fold
> 	X < (cast) (1 << Y) into (X >> Y) != 0 if cast is either
> 	a narrowing conversion, or widening conversion from signed
> 	to unsigned.
>
> 	* gcc.c-torture/execute/pr56051.c: New test.
Please add a comment to the fold-const.c change indicating why we can't 
apply this optimization as aggressively in the presence of conversions.

Pre-approved with that comment.

Thanks,
Jeff

Patch

--- gcc/fold-const.c.jj	2013-01-11 09:02:35.000000000 +0100
+++ gcc/fold-const.c	2013-01-20 17:37:45.787761247 +0100
@@ -13560,6 +13560,11 @@  fold_binary_loc (location_t loc,
 	  && TYPE_UNSIGNED (TREE_TYPE (arg0))
 	  && CONVERT_EXPR_P (arg1)
 	  && TREE_CODE (TREE_OPERAND (arg1, 0)) == LSHIFT_EXPR
+	  && (TYPE_PRECISION (TREE_TYPE (arg1))
+	      >= TYPE_PRECISION (TREE_TYPE (TREE_OPERAND (arg1, 0))))
+	  && (TYPE_UNSIGNED (TREE_TYPE (TREE_OPERAND (arg1, 0)))
+	      || (TYPE_PRECISION (TREE_TYPE (arg1))
+		  == TYPE_PRECISION (TREE_TYPE (TREE_OPERAND (arg1, 0)))))
 	  && integer_onep (TREE_OPERAND (TREE_OPERAND (arg1, 0), 0)))
 	{
 	  tem = build2 (RSHIFT_EXPR, TREE_TYPE (arg0), arg0,
--- gcc/testsuite/gcc.c-torture/execute/pr56051.c.jj	2013-01-20 17:40:09.070945296 +0100
+++ gcc/testsuite/gcc.c-torture/execute/pr56051.c	2013-01-20 17:38:31.000000000 +0100
@@ -0,0 +1,32 @@ 
+/* PR tree-optimization/56051 */
+
+extern void abort (void);
+
+int
+main ()
+{
+  unsigned char x1[1] = { 0 };
+  unsigned int s1 = __CHAR_BIT__;
+  int a1 = x1[0] < (unsigned char) (1 << s1);
+  unsigned char y1 = (unsigned char) (1 << s1);
+  int b1 = x1[0] < y1;
+  if (a1 != b1)
+    abort ();
+#if __SIZEOF_LONG_LONG__ > __SIZEOF_INT__
+  unsigned long long x2[1] = { 2ULL << (sizeof (int) * __CHAR_BIT__) };
+  unsigned int s2 = sizeof (int) * __CHAR_BIT__ - 1;
+  int a2 = x2[0] >= (unsigned long long) (1 << s2);
+  unsigned long long y2 = 1 << s2;
+  int b2 = x2[0] >= y2;
+  if (a2 != b2)
+    abort ();
+  unsigned long long x3[1] = { 2ULL << (sizeof (int) * __CHAR_BIT__) };
+  unsigned int s3 = sizeof (int) * __CHAR_BIT__ - 1;
+  int a3 = x3[0] >= (unsigned long long) (1U << s3);
+  unsigned long long y3 = 1U << s3;
+  int b3 = x3[0] >= y3;
+  if (a3 != b3)
+    abort ();
+#endif
+  return 0;
+}