This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] Fix A < 0 ? <sign bit of A> : 0 optimization (PR middle-end/29695)


Hi!

As the attached testcases show, the
A < 0 ? <sign bit of A> : 0 is simply (A & <sign bit of A>)
optimization is buggy.  The main problem is that sign_bit_p
function doesn't say that the second argument has only the
sign bit set and no other bits set, all it says is that
it has the sign bit set (corresponding to returned value type's
precision) and all lower bits are zero.  That's fine for the
other uses of sign_bit_p (namely the (A & <sign bit of A>) == 0
-> A >= 0 optimization), but not in this case.
If TEM's precision is smaller than TYPE and ARG1 precision,
we need to look at all the bits above the sign bit in ARG1 resp.
TYPE precision (whichever is smaller).
For unsigned int d there are 3 different cases we need to handle:

(d & 0x80000000) ? 0x80000000LL : 0LL

This should be optimized as (long long) ((unsigned int) d & 0x80000000U),
because all the bits above the sign bit are zero.

(d & 0x80000000) ? -2147483648LL : 0LL

This is the only case that was optimized properly, all bits above sign
bit are 1 and thus we want (long long) ((int) d & (int) 0x80000000).

(d & 0x80000000) ? 0x380000000LL : 0LL

This case doesn't have bits above sign bit neither all 0s nor all 1s,
so this optimization is wrong.

Bootstrapped/regtested on x86_64-linux, ok for 4.1/4.2/4.3?

2006-11-03  Jakub Jelinek  <jakub@redhat.com>

	PR middle-end/29695
	* fold-const.c (fold_ternary): Fix A < 0 ? <sign bit of A> : 0
	simplification.

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

--- gcc/fold-const.c.jj	2006-11-01 10:31:36.000000000 +0100
+++ gcc/fold-const.c	2006-11-03 13:52:38.000000000 +0100
@@ -11380,13 +11380,76 @@ fold_ternary (enum tree_code code, tree 
 
       /* A < 0 ? <sign bit of A> : 0 is simply (A & <sign bit of A>).  */
       if (TREE_CODE (arg0) == LT_EXPR
-          && integer_zerop (TREE_OPERAND (arg0, 1))
-          && integer_zerop (op2)
-          && (tem = sign_bit_p (TREE_OPERAND (arg0, 0), arg1)))
-        return fold_convert (type,
-			     fold_build2 (BIT_AND_EXPR,
-					  TREE_TYPE (tem), tem,
-					  fold_convert (TREE_TYPE (tem), arg1)));
+	  && integer_zerop (TREE_OPERAND (arg0, 1))
+	  && integer_zerop (op2)
+	  && (tem = sign_bit_p (TREE_OPERAND (arg0, 0), arg1)))
+	{
+	  /* sign_bit_p only checks ARG1 bits within A's precision.
+	     If <sign bit of A> has wider type than A, bits outside
+	     of A's precision in <sign bit of A> need to be checked.
+	     If they are all 0, this optimization needs to be done
+	     in unsigned A's type, if they are all 1 in signed A's type,
+	     otherwise this can't be done.  */
+	  if (TYPE_PRECISION (TREE_TYPE (tem))
+	      < TYPE_PRECISION (TREE_TYPE (arg1))
+	      && TYPE_PRECISION (TREE_TYPE (tem))
+		 < TYPE_PRECISION (type))
+	    {
+	      unsigned HOST_WIDE_INT mask_lo;
+	      HOST_WIDE_INT mask_hi;
+	      int inner_width, outer_width;
+	      tree tem_type;
+
+	      inner_width = TYPE_PRECISION (TREE_TYPE (tem));
+	      outer_width = TYPE_PRECISION (TREE_TYPE (arg1));
+	      if (outer_width > TYPE_PRECISION (type))
+		outer_width = TYPE_PRECISION (type);
+
+	      if (outer_width > HOST_BITS_PER_WIDE_INT)
+		{
+		  mask_hi = ((unsigned HOST_WIDE_INT) -1
+			     >> (2 * HOST_BITS_PER_WIDE_INT - outer_width));
+		  mask_lo = -1;
+		}
+	      else
+		{
+		  mask_hi = 0;
+		  mask_lo = ((unsigned HOST_WIDE_INT) -1
+			     >> (HOST_BITS_PER_WIDE_INT - outer_width));
+		}
+	      if (inner_width > HOST_BITS_PER_WIDE_INT)
+		{
+		  mask_hi &= ~((unsigned HOST_WIDE_INT) -1
+			       >> (HOST_BITS_PER_WIDE_INT - inner_width));
+		  mask_lo = 0;
+		}
+	      else
+		mask_lo &= ~((unsigned HOST_WIDE_INT) -1
+			     >> (HOST_BITS_PER_WIDE_INT - inner_width));
+
+	      if ((TREE_INT_CST_HIGH (arg1) & mask_hi) == mask_hi
+		  && (TREE_INT_CST_LOW (arg1) & mask_lo) == mask_lo)
+		{
+		  tem_type = lang_hooks.types.signed_type (TREE_TYPE (tem));
+		  tem = fold_convert (tem_type, tem);
+		}
+	      else if ((TREE_INT_CST_HIGH (arg1) & mask_hi) == 0
+		       && (TREE_INT_CST_LOW (arg1) & mask_lo) == 0)
+		{
+		  tem_type = lang_hooks.types.unsigned_type (TREE_TYPE (tem));
+		  tem = fold_convert (tem_type, tem);
+		}
+	      else
+		tem = NULL;
+	    }
+
+	  if (tem)
+	    return fold_convert (type,
+				 fold_build2 (BIT_AND_EXPR,
+					      TREE_TYPE (tem), tem,
+					      fold_convert (TREE_TYPE (tem),
+							    arg1)));
+	}
 
       /* (A >> N) & 1 ? (1 << N) : 0 is simply A & (1 << N).  A & 1 was
 	 already handled above.  */
--- gcc/testsuite/gcc.c-torture/execute/pr29695-1.c.jj	2006-11-03 14:03:04.000000000 +0100
+++ gcc/testsuite/gcc.c-torture/execute/pr29695-1.c	2006-11-03 13:59:05.000000000 +0100
@@ -0,0 +1,83 @@
+/* PR middle-end/29695 */
+
+extern void abort (void);
+
+int
+f1 (void)
+{
+  int a = 128;
+  return (a & 0x80) ? 0x80 : 0;
+}
+
+int
+f2 (void)
+{
+  unsigned char a = 128;
+  return (a & 0x80) ? 0x80 : 0;
+}
+
+int
+f3 (void)
+{
+  unsigned char a = 128;
+  return (a & 0x80) ? 0x380 : 0;
+}
+
+int
+f4 (void)
+{
+  unsigned char a = 128;
+  return (a & 0x80) ? -128 : 0;
+}
+
+long long
+f5 (void)
+{
+  long long a = 0x80000000LL;
+  return (a & 0x80000000) ? 0x80000000LL : 0LL;
+}
+
+long long
+f6 (void)
+{
+  unsigned int a = 0x80000000;
+  return (a & 0x80000000) ? 0x80000000LL : 0LL;
+}
+
+long long
+f7 (void)
+{
+  unsigned int a = 0x80000000;
+  return (a & 0x80000000) ? 0x380000000LL : 0LL;
+}
+
+long long
+f8 (void)
+{
+  unsigned int a = 0x80000000;
+  return (a & 0x80000000) ? -2147483648LL : 0LL;
+}
+
+int
+main (void)
+{
+  if ((char) 128 != -128 || (int) 0x80000000 != -2147483648)
+    return 0;
+  if (f1 () != 128)
+    abort ();
+  if (f2 () != 128)
+    abort ();
+  if (f3 () != 896)
+    abort ();
+  if (f4 () != -128)
+    abort ();
+  if (f5 () != 0x80000000LL)
+    abort ();
+  if (f6 () != 0x80000000LL)
+    abort ();
+  if (f7 () != 0x380000000LL)
+    abort ();
+  if (f8 () != -2147483648LL)
+    abort ();
+  return 0;
+}
--- gcc/testsuite/gcc.c-torture/execute/pr29695-2.c.jj	2006-11-03 14:03:06.000000000 +0100
+++ gcc/testsuite/gcc.c-torture/execute/pr29695-2.c	2006-11-03 14:01:06.000000000 +0100
@@ -0,0 +1,80 @@
+/* PR middle-end/29695 */
+
+extern void abort (void);
+
+int a = 128;
+unsigned char b = 128;
+long long c = 0x80000000LL;
+unsigned int d = 0x80000000;
+
+int
+f1 (void)
+{
+  return (a & 0x80) ? 0x80 : 0;
+}
+
+int
+f2 (void)
+{
+  return (b & 0x80) ? 0x80 : 0;
+}
+
+int
+f3 (void)
+{
+  return (b & 0x80) ? 0x380 : 0;
+}
+
+int
+f4 (void)
+{
+  return (b & 0x80) ? -128 : 0;
+}
+
+long long
+f5 (void)
+{
+  return (c & 0x80000000) ? 0x80000000LL : 0LL;
+}
+
+long long
+f6 (void)
+{
+  return (d & 0x80000000) ? 0x80000000LL : 0LL;
+}
+
+long long
+f7 (void)
+{
+  return (d & 0x80000000) ? 0x380000000LL : 0LL;
+}
+
+long long
+f8 (void)
+{
+  return (d & 0x80000000) ? -2147483648LL : 0LL;
+}
+
+int
+main (void)
+{
+  if ((char) 128 != -128 || (int) 0x80000000 != -2147483648)
+    return 0;
+  if (f1 () != 128)
+    abort ();
+  if (f2 () != 128)
+    abort ();
+  if (f3 () != 896)
+    abort ();
+  if (f4 () != -128)
+    abort ();
+  if (f5 () != 0x80000000LL)
+    abort ();
+  if (f6 () != 0x80000000LL)
+    abort ();
+  if (f7 () != 0x380000000LL)
+    abort ();
+  if (f8 () != -2147483648LL)
+    abort ();
+  return 0;
+}

	Jakub


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]