diff mbox

Fix PR56175

Message ID alpine.LNX.2.00.1302251624380.3543@zhemvz.fhfr.qr
State New
Headers show

Commit Message

Richard Biener Feb. 25, 2013, 3:27 p.m. UTC 
The following fixes a regression that shows that currently
conversion hoisting in bitwise operations as implemented by
tree forwprop is doing the opposite canonicalization from
what fold does.  The following patch restricts it
to perform the sub-set of transforms that fold
wouldn't immediately un-do.  Which is, canonicalize to
narrower operation operands (which is conveniently already
what the more complex case in forwprop is guarded with).

Bootstrapped and tested on x86_64-unknown-linux-gnu, applied on trunk.

Whether it fully fixes PR56175 (a performance regression) remains
to be seen.

Richard.

2013-02-25  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/56175
	* tree-ssa-forwprop.c (hoist_conversion_for_bitop_p): New predicate,
	split out from ...
	(simplify_bitwise_binary): ... here.  Also guard the conversion
	of (type) X op CST to (type) (X op ((type-x) CST)) with it.

	* gcc.dg/tree-ssa/forwprop-24.c: New testcase.


Property changes on: gcc/testsuite/gcc.dg/tree-ssa/forwprop-24.c
diff mbox

Patch

Index: gcc/tree-ssa-forwprop.c
===================================================================
--- gcc/tree-ssa-forwprop.c	(revision 196257)
+++ gcc/tree-ssa-forwprop.c	(working copy)
@@ -1772,6 +1772,29 @@  defcodefor_name (tree name, enum tree_co
   /* Ignore arg3 currently. */
 }
 
+/* Return true if a conversion of an operand from type FROM to type TO
+   should be applied after performing the operation instead.  */
+
+static bool
+hoist_conversion_for_bitop_p (tree to, tree from)
+{
+  /* That's a good idea if the conversion widens the operand, thus
+     after hoisting the conversion the operation will be narrower.  */
+  if (TYPE_PRECISION (from) < TYPE_PRECISION (to))
+    return true;
+
+  /* It's also a good idea if the conversion is to a non-integer mode.  */
+  if (GET_MODE_CLASS (TYPE_MODE (to)) != MODE_INT)
+    return true;
+
+  /* Or if the precision of TO is not the same as the precision
+     of its mode.  */
+  if (TYPE_PRECISION (to) != GET_MODE_PRECISION (TYPE_MODE (to)))
+    return true;
+
+  return false;
+}
+
 /* Simplify bitwise binary operations.
    Return true if a transformation applied, otherwise return false.  */
 
@@ -1789,9 +1812,11 @@  simplify_bitwise_binary (gimple_stmt_ite
   defcodefor_name (arg1, &def1_code, &def1_arg1, &def1_arg2);
   defcodefor_name (arg2, &def2_code, &def2_arg1, &def2_arg2);
 
-  /* Try to fold (type) X op CST -> (type) (X op ((type-x) CST)).  */
+  /* Try to fold (type) X op CST -> (type) (X op ((type-x) CST))
+     when profitable.  */
   if (TREE_CODE (arg2) == INTEGER_CST
       && CONVERT_EXPR_CODE_P (def1_code)
+      && hoist_conversion_for_bitop_p (TREE_TYPE (arg1), TREE_TYPE (def1_arg1))
       && INTEGRAL_TYPE_P (TREE_TYPE (def1_arg1))
       && int_fits_type_p (arg2, TREE_TYPE (def1_arg1)))
     {
@@ -1816,15 +1841,7 @@  simplify_bitwise_binary (gimple_stmt_ite
   if (CONVERT_EXPR_CODE_P (def1_code)
       && CONVERT_EXPR_CODE_P (def2_code)
       && types_compatible_p (TREE_TYPE (def1_arg1), TREE_TYPE (def2_arg1))
-      /* Make sure that the conversion widens the operands, or has same
-	 precision,  or that it changes the operation to a bitfield
-	 precision.  */
-      && ((TYPE_PRECISION (TREE_TYPE (def1_arg1))
-	   <= TYPE_PRECISION (TREE_TYPE (arg1)))
-	  || (GET_MODE_CLASS (TYPE_MODE (TREE_TYPE (arg1)))
-	      != MODE_INT)
-	  || (TYPE_PRECISION (TREE_TYPE (arg1))
-	      != GET_MODE_PRECISION (TYPE_MODE (TREE_TYPE (arg1))))))
+      && hoist_conversion_for_bitop_p (TREE_TYPE (arg1), TREE_TYPE (def1_arg1)))
     {
       gimple newop;
       tree tem = make_ssa_name (TREE_TYPE (def1_arg1), NULL);
Index: gcc/testsuite/gcc.dg/tree-ssa/forwprop-24.c
===================================================================
--- gcc/testsuite/gcc.dg/tree-ssa/forwprop-24.c	(revision 0)
+++ gcc/testsuite/gcc.dg/tree-ssa/forwprop-24.c	(working copy)
@@ -0,0 +1,18 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O -fdump-tree-cddce1" } */
+
+void bar (void);
+unsigned short
+foo (unsigned char x, unsigned short y)
+{
+  unsigned char t = (unsigned char)((x & 1) ^ ((unsigned char)y & 1));
+  if (t == 1)
+    bar ();
+  return y;
+}
+
+/* We should have combined this to require only one bitwise and
+   as in (x ^ (char) y) & 1.  */
+
+/* { dg-final { scan-tree-dump-times " & " 1 "cddce1" } } */
+/* { dg-final { cleanup-tree-dump "cddce1" } } */