Patchwork [tree-optimization] : Add cleanup code for possible unused statements in binary optimization

login
register
mail settings
Submitter Kai Tietz
Date Aug. 2, 2011, 3:40 p.m.
Message ID <CAEwic4Y0aBtAOjkqv3R5VeWNpKUZRg1Mhu9TBMwrpLJtHU2brw@mail.gmail.com>
Download mbox | patch
Permalink /patch/107950/
State New
Headers show

Comments

Kai Tietz - Aug. 2, 2011, 3:40 p.m.
2011/8/2 Richard Guenther <richard.guenther@gmail.com>:
> On Tue, Aug 2, 2011 at 12:39 PM, Kai Tietz <ktietz70@googlemail.com> wrote:

Thanks, yes, I noticed that.  Patch adjusted for cfg_tree.

ChangeLog

2011-08-02  Kai Tietz  <ktietz@redhat.com>

        * tree-ssa-forwprop.c (simplify_bitwise_binary):
        Remove possible unused statement after optimization
       and change result to indicate cfg-tree change.
       (ssa_forward_propagate_and_combine): Adjust handling for
       simplify_bitwise_binary call.

2011-08-02  Kai Tietz  <ktietz@redhat.com>

        * gcc.dg/tree-ssa/forwprop-15.c: Add test for no int casts.

Bootstrapped and regression-tested for all languages (including Ada
and Obj-C++) on host x86_64-pc-linux-gnu.
Ok for apply?
Richard Guenther - Aug. 3, 2011, 8:29 a.m.
On Tue, Aug 2, 2011 at 5:40 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
> 2011/8/2 Richard Guenther <richard.guenther@gmail.com>:
>> On Tue, Aug 2, 2011 at 12:39 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
>
> Thanks, yes, I noticed that.  Patch adjusted for cfg_tree.

What about my other comment?

> ChangeLog
>
> 2011-08-02  Kai Tietz  <ktietz@redhat.com>
>
>         * tree-ssa-forwprop.c (simplify_bitwise_binary):
>         Remove possible unused statement after optimization
>       and change result to indicate cfg-tree change.
>       (ssa_forward_propagate_and_combine): Adjust handling for
>       simplify_bitwise_binary call.
>
> 2011-08-02  Kai Tietz  <ktietz@redhat.com>
>
>         * gcc.dg/tree-ssa/forwprop-15.c: Add test for no int casts.
>
> Bootstrapped and regression-tested for all languages (including Ada
> and Obj-C++) on host x86_64-pc-linux-gnu.
> Ok for apply?
>
> Index: gcc/gcc/tree-ssa-forwprop.c
> ===================================================================
> --- gcc.orig/gcc/tree-ssa-forwprop.c
> +++ gcc/gcc/tree-ssa-forwprop.c
> @@ -1703,9 +1703,10 @@ simplify_bitwise_binary_1 (enum tree_cod
>  }
>
>  /* Simplify bitwise binary operations.
> -   Return true if a transformation applied, otherwise return false.  */
> +   Returns 1 if there were any changes made, 2 if cfg-cleanup needs to
> +   run.  Else it returns 0.  */
>
> -static bool
> +static int
>  simplify_bitwise_binary (gimple_stmt_iterator *gsi)
>  {
>   gimple stmt = gsi_stmt (*gsi);
> @@ -1716,6 +1717,7 @@ simplify_bitwise_binary (gimple_stmt_ite
>   gimple def1 = NULL, def2 = NULL;
>   tree def1_arg1, def2_arg1;
>   enum tree_code def1_code, def2_code;
> +  bool ischanged = false;
>
>   def1_code = TREE_CODE (arg1);
>   def1_arg1 = arg1;
> @@ -1761,24 +1763,23 @@ simplify_bitwise_binary (gimple_stmt_ite
>       gimple_assign_set_rhs_with_ops_1 (gsi, NOP_EXPR,
>                                        tem, NULL_TREE, NULL_TREE);
>       update_stmt (gsi_stmt (*gsi));
> -      return true;
> +      ischanged = true;
>     }
> -
>   /* For bitwise binary operations apply operand conversions to the
>      binary operation result instead of to the operands.  This allows
>      to combine successive conversions and bitwise binary operations.  */
> -  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))))))
> +  else 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))))))
>     {
>       gimple newop;
>       tree tem = create_tmp_reg (TREE_TYPE (def1_arg1),
> @@ -1791,77 +1792,84 @@ simplify_bitwise_binary (gimple_stmt_ite
>       gimple_assign_set_rhs_with_ops_1 (gsi, NOP_EXPR,
>                                        tem, NULL_TREE, NULL_TREE);
>       update_stmt (gsi_stmt (*gsi));
> -      return true;
> +      ischanged = true;
>     }
> -
>   /* (a | CST1) & CST2  ->  (a & CST2) | (CST1 & CST2).  */
> -  if (code == BIT_AND_EXPR
> -      && def1_code == BIT_IOR_EXPR
> -      && TREE_CODE (arg2) == INTEGER_CST
> -      && TREE_CODE (gimple_assign_rhs2 (def1)) == INTEGER_CST)
> +  else if (code == BIT_AND_EXPR
> +          && def1_code == BIT_IOR_EXPR
> +          && TREE_CODE (arg2) == INTEGER_CST
> +          && TREE_CODE (gimple_assign_rhs2 (def1)) == INTEGER_CST)
>     {
>       tree cst = fold_build2 (BIT_AND_EXPR, TREE_TYPE (arg2),
>                              arg2, gimple_assign_rhs2 (def1));
>       tree tem;
>       gimple newop;
>       if (integer_zerop (cst))
> -       {
> -         gimple_assign_set_rhs1 (stmt, def1_arg1);
> -         update_stmt (stmt);
> -         return true;
> +       gimple_assign_set_rhs1 (stmt, def1_arg1);
> +      else
> +        {
> +         tem = create_tmp_reg (TREE_TYPE (arg2), NULL);
> +         newop = gimple_build_assign_with_ops (BIT_AND_EXPR,
> +                                               tem, def1_arg1, arg2);
> +         tem = make_ssa_name (tem, newop);
> +         gimple_assign_set_lhs (newop, tem);
> +         gimple_set_location (newop, gimple_location (stmt));
> +         /* Make sure to re-process the new stmt as it's walking upwards.  */
> +         gsi_insert_before (gsi, newop, GSI_NEW_STMT);
> +         gimple_assign_set_rhs1 (stmt, tem);
> +         gimple_assign_set_rhs2 (stmt, cst);
> +         gimple_assign_set_rhs_code (stmt, BIT_IOR_EXPR);
>        }
> -      tem = create_tmp_reg (TREE_TYPE (arg2), NULL);
> -      newop = gimple_build_assign_with_ops (BIT_AND_EXPR,
> -                                           tem, def1_arg1, arg2);
> -      tem = make_ssa_name (tem, newop);
> -      gimple_assign_set_lhs (newop, tem);
> -      gimple_set_location (newop, gimple_location (stmt));
> -      /* Make sure to re-process the new stmt as it's walking upwards.  */
> -      gsi_insert_before (gsi, newop, GSI_NEW_STMT);
> -      gimple_assign_set_rhs1 (stmt, tem);
> -      gimple_assign_set_rhs2 (stmt, cst);
> -      gimple_assign_set_rhs_code (stmt, BIT_IOR_EXPR);
>       update_stmt (stmt);
> -      return true;
> +      ischanged = true;
>     }
> -
>   /* Combine successive equal operations with constants.  */
> -  if ((code == BIT_AND_EXPR
> -       || code == BIT_IOR_EXPR
> -       || code == BIT_XOR_EXPR)
> -      && def1_code == code
> -      && TREE_CODE (arg2) == INTEGER_CST
> -      && TREE_CODE (gimple_assign_rhs2 (def1)) == INTEGER_CST)
> +  else if ((code == BIT_AND_EXPR
> +           || code == BIT_IOR_EXPR
> +           || code == BIT_XOR_EXPR)
> +          && def1_code == code
> +          && TREE_CODE (arg2) == INTEGER_CST
> +          && TREE_CODE (gimple_assign_rhs2 (def1)) == INTEGER_CST)
>     {
>       tree cst = fold_build2 (code, TREE_TYPE (arg2),
>                              arg2, gimple_assign_rhs2 (def1));
>       gimple_assign_set_rhs1 (stmt, def1_arg1);
>       gimple_assign_set_rhs2 (stmt, cst);
>       update_stmt (stmt);
> -      return true;
> +      ischanged = true;
>     }
> -
>   /* Canonicalize X ^ ~0 to ~X.  */
> -  if (code == BIT_XOR_EXPR
> -      && TREE_CODE (arg2) == INTEGER_CST
> -      && integer_all_onesp (arg2))
> +  else if (code == BIT_XOR_EXPR
> +          && TREE_CODE (arg2) == INTEGER_CST
> +          && integer_all_onesp (arg2))
>     {
>       gimple_assign_set_rhs_with_ops (gsi, BIT_NOT_EXPR, arg1, NULL_TREE);
>       gcc_assert (gsi_stmt (*gsi) == stmt);
>       update_stmt (stmt);
> -      return true;
> +      ischanged = true;
>     }
> -
> -  /* Try simple folding for X op !X, and X op X.  */
> -  res = simplify_bitwise_binary_1 (code, TREE_TYPE (arg1), arg1, arg2);
> -  if (res != NULL_TREE)
> +  else
>     {
> -      gimple_assign_set_rhs_from_tree (gsi, res);
> -      update_stmt (gsi_stmt (*gsi));
> -      return true;
> +      /* Try simple folding for X op !X, and X op X.  */
> +      res = simplify_bitwise_binary_1 (code, TREE_TYPE (arg1), arg1, arg2);
> +      if (res != NULL_TREE)
> +       {
> +         gimple_assign_set_rhs_from_tree (gsi, res);
> +         update_stmt (gsi_stmt (*gsi));
> +         ischanged = true;
> +       }
>     }
>
> -  return false;
> +  /* If we haven't done any changes, then return zero.  */
> +  if (!ischanged)
> +    return 0;
> +  ischanged = false;
> +  /* See if original arguments might be without use.  */
> +  if (TREE_CODE (arg1) == SSA_NAME)
> +    ischanged = remove_prop_source_from_use (arg1);
> +  if (TREE_CODE (arg2) == SSA_NAME)
> +    ischanged |= remove_prop_source_from_use (arg2);
> +  return ischanged ? 2 : 1;
>  }
>
>
> @@ -2466,7 +2474,12 @@ ssa_forward_propagate_and_combine (void)
>                else if (code == BIT_AND_EXPR
>                         || code == BIT_IOR_EXPR
>                         || code == BIT_XOR_EXPR)
> -                 changed = simplify_bitwise_binary (&gsi);
> +                 {
> +                   int did_something =  simplify_bitwise_binary (&gsi);
> +                   if (did_something == 2)
> +                     cfg_changed = true;
> +                   changed = did_something != 0;
> +                 }
>                else if (code == PLUS_EXPR
>                         || code == MINUS_EXPR)
>                  changed = associate_plusminus (stmt);
> Index: gcc/gcc/testsuite/gcc.dg/tree-ssa/forwprop-15.c
> ===================================================================
> --- gcc.orig/gcc/testsuite/gcc.dg/tree-ssa/forwprop-15.c
> +++ gcc/gcc/testsuite/gcc.dg/tree-ssa/forwprop-15.c
> @@ -11,4 +11,5 @@ foo (_Bool a, _Bool b, _Bool c
>
>  /* { dg-final { scan-tree-dump-times " == " 0 "forwprop1" } } */
>  /* { dg-final { scan-tree-dump-times " != " 0 "forwprop1" } } */
> +/* { dg-final { scan-tree-dump-times "\\\(int\\\)" 0 "forwprop1" } } */
>  /* { dg-final { cleanup-tree-dump "forwprop1" } } */
>

Patch

Index: gcc/gcc/tree-ssa-forwprop.c
===================================================================
--- gcc.orig/gcc/tree-ssa-forwprop.c
+++ gcc/gcc/tree-ssa-forwprop.c
@@ -1703,9 +1703,10 @@  simplify_bitwise_binary_1 (enum tree_cod
 }

 /* Simplify bitwise binary operations.
-   Return true if a transformation applied, otherwise return false.  */
+   Returns 1 if there were any changes made, 2 if cfg-cleanup needs to
+   run.  Else it returns 0.  */

-static bool
+static int
 simplify_bitwise_binary (gimple_stmt_iterator *gsi)
 {
   gimple stmt = gsi_stmt (*gsi);
@@ -1716,6 +1717,7 @@  simplify_bitwise_binary (gimple_stmt_ite
   gimple def1 = NULL, def2 = NULL;
   tree def1_arg1, def2_arg1;
   enum tree_code def1_code, def2_code;
+  bool ischanged = false;

   def1_code = TREE_CODE (arg1);
   def1_arg1 = arg1;
@@ -1761,24 +1763,23 @@  simplify_bitwise_binary (gimple_stmt_ite
       gimple_assign_set_rhs_with_ops_1 (gsi, NOP_EXPR,
 					tem, NULL_TREE, NULL_TREE);
       update_stmt (gsi_stmt (*gsi));
-      return true;
+      ischanged = true;
     }
-
   /* For bitwise binary operations apply operand conversions to the
      binary operation result instead of to the operands.  This allows
      to combine successive conversions and bitwise binary operations.  */
-  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))))))
+  else 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))))))
     {
       gimple newop;
       tree tem = create_tmp_reg (TREE_TYPE (def1_arg1),
@@ -1791,77 +1792,84 @@  simplify_bitwise_binary (gimple_stmt_ite
       gimple_assign_set_rhs_with_ops_1 (gsi, NOP_EXPR,
 					tem, NULL_TREE, NULL_TREE);
       update_stmt (gsi_stmt (*gsi));
-      return true;
+      ischanged = true;
     }
-
   /* (a | CST1) & CST2  ->  (a & CST2) | (CST1 & CST2).  */
-  if (code == BIT_AND_EXPR
-      && def1_code == BIT_IOR_EXPR
-      && TREE_CODE (arg2) == INTEGER_CST
-      && TREE_CODE (gimple_assign_rhs2 (def1)) == INTEGER_CST)
+  else if (code == BIT_AND_EXPR
+	   && def1_code == BIT_IOR_EXPR
+	   && TREE_CODE (arg2) == INTEGER_CST
+	   && TREE_CODE (gimple_assign_rhs2 (def1)) == INTEGER_CST)
     {
       tree cst = fold_build2 (BIT_AND_EXPR, TREE_TYPE (arg2),
 			      arg2, gimple_assign_rhs2 (def1));
       tree tem;
       gimple newop;
       if (integer_zerop (cst))
-	{
-	  gimple_assign_set_rhs1 (stmt, def1_arg1);
-	  update_stmt (stmt);
-	  return true;
+	gimple_assign_set_rhs1 (stmt, def1_arg1);
+      else
+        {
+	  tem = create_tmp_reg (TREE_TYPE (arg2), NULL);
+	  newop = gimple_build_assign_with_ops (BIT_AND_EXPR,
+						tem, def1_arg1, arg2);
+	  tem = make_ssa_name (tem, newop);
+	  gimple_assign_set_lhs (newop, tem);
+	  gimple_set_location (newop, gimple_location (stmt));
+	  /* Make sure to re-process the new stmt as it's walking upwards.  */
+	  gsi_insert_before (gsi, newop, GSI_NEW_STMT);
+	  gimple_assign_set_rhs1 (stmt, tem);
+	  gimple_assign_set_rhs2 (stmt, cst);
+	  gimple_assign_set_rhs_code (stmt, BIT_IOR_EXPR);
 	}
-      tem = create_tmp_reg (TREE_TYPE (arg2), NULL);
-      newop = gimple_build_assign_with_ops (BIT_AND_EXPR,
-					    tem, def1_arg1, arg2);
-      tem = make_ssa_name (tem, newop);
-      gimple_assign_set_lhs (newop, tem);
-      gimple_set_location (newop, gimple_location (stmt));
-      /* Make sure to re-process the new stmt as it's walking upwards.  */
-      gsi_insert_before (gsi, newop, GSI_NEW_STMT);
-      gimple_assign_set_rhs1 (stmt, tem);
-      gimple_assign_set_rhs2 (stmt, cst);
-      gimple_assign_set_rhs_code (stmt, BIT_IOR_EXPR);
       update_stmt (stmt);
-      return true;
+      ischanged = true;
     }
-
   /* Combine successive equal operations with constants.  */
-  if ((code == BIT_AND_EXPR
-       || code == BIT_IOR_EXPR
-       || code == BIT_XOR_EXPR)
-      && def1_code == code
-      && TREE_CODE (arg2) == INTEGER_CST
-      && TREE_CODE (gimple_assign_rhs2 (def1)) == INTEGER_CST)
+  else if ((code == BIT_AND_EXPR
+	    || code == BIT_IOR_EXPR
+	    || code == BIT_XOR_EXPR)
+	   && def1_code == code
+	   && TREE_CODE (arg2) == INTEGER_CST
+	   && TREE_CODE (gimple_assign_rhs2 (def1)) == INTEGER_CST)
     {
       tree cst = fold_build2 (code, TREE_TYPE (arg2),
 			      arg2, gimple_assign_rhs2 (def1));
       gimple_assign_set_rhs1 (stmt, def1_arg1);
       gimple_assign_set_rhs2 (stmt, cst);
       update_stmt (stmt);
-      return true;
+      ischanged = true;
     }
-
   /* Canonicalize X ^ ~0 to ~X.  */
-  if (code == BIT_XOR_EXPR
-      && TREE_CODE (arg2) == INTEGER_CST
-      && integer_all_onesp (arg2))
+  else if (code == BIT_XOR_EXPR
+	   && TREE_CODE (arg2) == INTEGER_CST
+	   && integer_all_onesp (arg2))
     {
       gimple_assign_set_rhs_with_ops (gsi, BIT_NOT_EXPR, arg1, NULL_TREE);
       gcc_assert (gsi_stmt (*gsi) == stmt);
       update_stmt (stmt);
-      return true;
+      ischanged = true;
     }
-
-  /* Try simple folding for X op !X, and X op X.  */
-  res = simplify_bitwise_binary_1 (code, TREE_TYPE (arg1), arg1, arg2);
-  if (res != NULL_TREE)
+  else
     {
-      gimple_assign_set_rhs_from_tree (gsi, res);
-      update_stmt (gsi_stmt (*gsi));
-      return true;
+      /* Try simple folding for X op !X, and X op X.  */
+      res = simplify_bitwise_binary_1 (code, TREE_TYPE (arg1), arg1, arg2);
+      if (res != NULL_TREE)
+	{
+	  gimple_assign_set_rhs_from_tree (gsi, res);
+	  update_stmt (gsi_stmt (*gsi));
+	  ischanged = true;
+	}
     }

-  return false;
+  /* If we haven't done any changes, then return zero.  */
+  if (!ischanged)
+    return 0;
+  ischanged = false;
+  /* See if original arguments might be without use.  */
+  if (TREE_CODE (arg1) == SSA_NAME)
+    ischanged = remove_prop_source_from_use (arg1);
+  if (TREE_CODE (arg2) == SSA_NAME)
+    ischanged |= remove_prop_source_from_use (arg2);
+  return ischanged ? 2 : 1;
 }


@@ -2466,7 +2474,12 @@  ssa_forward_propagate_and_combine (void)
 		else if (code == BIT_AND_EXPR
 			 || code == BIT_IOR_EXPR
 			 || code == BIT_XOR_EXPR)
-		  changed = simplify_bitwise_binary (&gsi);
+		  {
+		    int did_something =  simplify_bitwise_binary (&gsi);
+		    if (did_something == 2)
+		      cfg_changed = true;
+		    changed = did_something != 0;
+		  }
 		else if (code == PLUS_EXPR
 			 || code == MINUS_EXPR)
 		  changed = associate_plusminus (stmt);
Index: gcc/gcc/testsuite/gcc.dg/tree-ssa/forwprop-15.c
===================================================================
--- gcc.orig/gcc/testsuite/gcc.dg/tree-ssa/forwprop-15.c
+++ gcc/gcc/testsuite/gcc.dg/tree-ssa/forwprop-15.c
@@ -11,4 +11,5 @@  foo (_Bool a, _Bool b, _Bool c

 /* { dg-final { scan-tree-dump-times " == " 0 "forwprop1" } } */
 /* { dg-final { scan-tree-dump-times " != " 0 "forwprop1" } } */
+/* { dg-final { scan-tree-dump-times "\\\(int\\\)" 0 "forwprop1" } } */
 /* { dg-final { cleanup-tree-dump "forwprop1" } } */