[PATCH] Use simple_dce_from_worklist with match_simplify_replacement.

[PATCH] Use simple_dce_from_worklist with match_simplify_replacement.

[apinski]

From: Andrew Pinski <apinski@marvell.com>

This is a simple patch to do some DCE after a successful
match and simplify replacement in PHI-OPT. match and simplify
likes to generate some extra statements which should be cleaned
up.

OK? Bootstrapped and tested on x86_64-linux with no regressions.

Thanks,
Andrew Pinski

gcc/ChangeLog:

	* tree-ssa-phiopt.cc: Include tree-ssa-dce.h
	(replace_phi_edge_with_variable):
	New argument, dce_ssa_names. Call simple_dce_from_worklist.
	(match_simplify_replacement): If we inserted a sequence,
	mark the lhs of the new sequence to be possible dce.
	Always move the statement and mark the lhs (if it is a name)
	as possible to remove.
---
 gcc/tree-ssa-phiopt.cc | 35 ++++++++++++++++++++++++++---------
 1 file changed, 26 insertions(+), 9 deletions(-)

diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc
index 925bd7d..996700b 100644
--- a/gcc/tree-ssa-phiopt.cc
+++ b/gcc/tree-ssa-phiopt.cc
@@ -53,6 +53,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "gimple-match.h"
 #include "dbgcnt.h"
 #include "tree-ssa-propagate.h"
+#include "tree-ssa-dce.h"
 
 static unsigned int tree_ssa_phiopt_worker (bool, bool, bool);
 static bool two_value_replacement (basic_block, basic_block, edge, gphi *,
@@ -74,7 +75,6 @@ static bool cond_store_replacement (basic_block, basic_block, edge, edge,
 				    hash_set<tree> *);
 static bool cond_if_else_store_replacement (basic_block, basic_block, basic_block);
 static hash_set<tree> * get_non_trapping ();
-static void replace_phi_edge_with_variable (basic_block, edge, gphi *, tree);
 static void hoist_adjacent_loads (basic_block, basic_block,
 				  basic_block, basic_block);
 static bool gate_hoist_loads (void);
@@ -402,7 +402,8 @@ tree_ssa_phiopt_worker (bool do_store_elim, bool do_hoist_loads, bool early_p)
 
 static void
 replace_phi_edge_with_variable (basic_block cond_block,
-				edge e, gphi *phi, tree new_tree)
+				edge e, gphi *phi, tree new_tree,
+				bitmap dce_ssa_names = auto_bitmap())
 {
   basic_block bb = gimple_bb (phi);
   gimple_stmt_iterator gsi;
@@ -477,6 +478,8 @@ replace_phi_edge_with_variable (basic_block cond_block,
 	gimple_cond_make_true (cond);
     }
 
+  simple_dce_from_worklist (dce_ssa_names);
+
   statistics_counter_event (cfun, "Replace PHI with variable", 1);
 
   if (dump_file && (dump_flags & TDF_DETAILS))
@@ -986,6 +989,7 @@ match_simplify_replacement (basic_block cond_bb, basic_block middle_bb,
   gimple_seq seq = NULL;
   tree result;
   gimple *stmt_to_move = NULL;
+  auto_bitmap inserted_exprs;
 
   /* Special case A ? B : B as this will always simplify to B. */
   if (operand_equal_for_phi_arg_p (arg0, arg1))
@@ -1060,14 +1064,22 @@ match_simplify_replacement (basic_block cond_bb, basic_block middle_bb,
   gsi = gsi_last_bb (cond_bb);
   /* Insert the sequence generated from gimple_simplify_phiopt.  */
   if (seq)
+    {
+      // Mark the lhs of the new statements maybe for dce
+      gimple_stmt_iterator gsi1 = gsi_start (seq);
+      for (; !gsi_end_p (gsi1); gsi_next (&gsi1))
+	{
+	  gimple *stmt = gsi_stmt (gsi1);
+	  tree name = gimple_get_lhs (stmt);
+	  if (name && TREE_CODE (name) == SSA_NAME)
+	    bitmap_set_bit (inserted_exprs, SSA_NAME_VERSION (name));
+	}
     gsi_insert_seq_before (&gsi, seq, GSI_CONTINUE_LINKING);
+  }
 
-  /* If there was a statement to move and the result of the statement
-     is going to be used, move it to right before the original
-     conditional.  */
-  if (stmt_to_move
-      && (gimple_assign_lhs (stmt_to_move) == result
-	  || !has_single_use (gimple_assign_lhs (stmt_to_move))))
+  /* If there was a statement to move, move it to right before
+     the original conditional.  */
+  if (stmt_to_move)
     {
       if (dump_file && (dump_flags & TDF_DETAILS))
 	{
@@ -1075,12 +1087,17 @@ match_simplify_replacement (basic_block cond_bb, basic_block middle_bb,
 	  print_gimple_stmt (dump_file, stmt_to_move, 0,
 			   TDF_VOPS|TDF_MEMSYMS);
 	}
+
+      tree name = gimple_get_lhs (stmt_to_move);
+      // Mark the name to be renamed if there is one.
+      if (name && TREE_CODE (name) == SSA_NAME)
+	bitmap_set_bit (inserted_exprs, SSA_NAME_VERSION (name));
       gimple_stmt_iterator gsi1 = gsi_for_stmt (stmt_to_move);
       gsi_move_before (&gsi1, &gsi);
       reset_flow_sensitive_info (gimple_assign_lhs (stmt_to_move));
     }
 
-  replace_phi_edge_with_variable (cond_bb, e1, phi, result);
+  replace_phi_edge_with_variable (cond_bb, e1, phi, result, inserted_exprs);
 
   /* Add Statistic here even though replace_phi_edge_with_variable already
      does it as we want to be able to count when match-simplify happens vs
-- 
1.8.3.1

Re: [PATCH] Use simple_dce_from_worklist with match_simplify_replacement.

[Richard Biener]

> Am 27.10.2022 um 17:11 schrieb apinski--- via Gcc-patches <gcc-patches@gcc.gnu.org>:
> 
> From: Andrew Pinski <apinski@marvell.com>
> 
> This is a simple patch to do some DCE after a successful
> match and simplify replacement in PHI-OPT. match and simplify
> likes to generate some extra statements which should be cleaned
> up.
> 
> OK? Bootstrapped and tested on x86_64-linux with no regressions.

Ok.

Richard 

> Thanks,
> Andrew Pinski
> 
> gcc/ChangeLog:
> 
>    * tree-ssa-phiopt.cc: Include tree-ssa-dce.h
>    (replace_phi_edge_with_variable):
>    New argument, dce_ssa_names. Call simple_dce_from_worklist.
>    (match_simplify_replacement): If we inserted a sequence,
>    mark the lhs of the new sequence to be possible dce.
>    Always move the statement and mark the lhs (if it is a name)
>    as possible to remove.
> ---
> gcc/tree-ssa-phiopt.cc | 35 ++++++++++++++++++++++++++---------
> 1 file changed, 26 insertions(+), 9 deletions(-)
> 
> diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc
> index 925bd7d..996700b 100644
> --- a/gcc/tree-ssa-phiopt.cc
> +++ b/gcc/tree-ssa-phiopt.cc
> @@ -53,6 +53,7 @@ along with GCC; see the file COPYING3.  If not see
> #include "gimple-match.h"
> #include "dbgcnt.h"
> #include "tree-ssa-propagate.h"
> +#include "tree-ssa-dce.h"
> 
> static unsigned int tree_ssa_phiopt_worker (bool, bool, bool);
> static bool two_value_replacement (basic_block, basic_block, edge, gphi *,
> @@ -74,7 +75,6 @@ static bool cond_store_replacement (basic_block, basic_block, edge, edge,
>                    hash_set<tree> *);
> static bool cond_if_else_store_replacement (basic_block, basic_block, basic_block);
> static hash_set<tree> * get_non_trapping ();
> -static void replace_phi_edge_with_variable (basic_block, edge, gphi *, tree);
> static void hoist_adjacent_loads (basic_block, basic_block,
>                  basic_block, basic_block);
> static bool gate_hoist_loads (void);
> @@ -402,7 +402,8 @@ tree_ssa_phiopt_worker (bool do_store_elim, bool do_hoist_loads, bool early_p)
> 
> static void
> replace_phi_edge_with_variable (basic_block cond_block,
> -                edge e, gphi *phi, tree new_tree)
> +                edge e, gphi *phi, tree new_tree,
> +                bitmap dce_ssa_names = auto_bitmap())
> {
>   basic_block bb = gimple_bb (phi);
>   gimple_stmt_iterator gsi;
> @@ -477,6 +478,8 @@ replace_phi_edge_with_variable (basic_block cond_block,
>    gimple_cond_make_true (cond);
>     }
> 
> +  simple_dce_from_worklist (dce_ssa_names);
> +
>   statistics_counter_event (cfun, "Replace PHI with variable", 1);
> 
>   if (dump_file && (dump_flags & TDF_DETAILS))
> @@ -986,6 +989,7 @@ match_simplify_replacement (basic_block cond_bb, basic_block middle_bb,
>   gimple_seq seq = NULL;
>   tree result;
>   gimple *stmt_to_move = NULL;
> +  auto_bitmap inserted_exprs;
> 
>   /* Special case A ? B : B as this will always simplify to B. */
>   if (operand_equal_for_phi_arg_p (arg0, arg1))
> @@ -1060,14 +1064,22 @@ match_simplify_replacement (basic_block cond_bb, basic_block middle_bb,
>   gsi = gsi_last_bb (cond_bb);
>   /* Insert the sequence generated from gimple_simplify_phiopt.  */
>   if (seq)
> +    {
> +      // Mark the lhs of the new statements maybe for dce
> +      gimple_stmt_iterator gsi1 = gsi_start (seq);
> +      for (; !gsi_end_p (gsi1); gsi_next (&gsi1))
> +    {
> +      gimple *stmt = gsi_stmt (gsi1);
> +      tree name = gimple_get_lhs (stmt);
> +      if (name && TREE_CODE (name) == SSA_NAME)
> +        bitmap_set_bit (inserted_exprs, SSA_NAME_VERSION (name));
> +    }
>     gsi_insert_seq_before (&gsi, seq, GSI_CONTINUE_LINKING);
> +  }
> 
> -  /* If there was a statement to move and the result of the statement
> -     is going to be used, move it to right before the original
> -     conditional.  */
> -  if (stmt_to_move
> -      && (gimple_assign_lhs (stmt_to_move) == result
> -      || !has_single_use (gimple_assign_lhs (stmt_to_move))))
> +  /* If there was a statement to move, move it to right before
> +     the original conditional.  */
> +  if (stmt_to_move)
>     {
>       if (dump_file && (dump_flags & TDF_DETAILS))
>    {
> @@ -1075,12 +1087,17 @@ match_simplify_replacement (basic_block cond_bb, basic_block middle_bb,
>      print_gimple_stmt (dump_file, stmt_to_move, 0,
>               TDF_VOPS|TDF_MEMSYMS);
>    }
> +
> +      tree name = gimple_get_lhs (stmt_to_move);
> +      // Mark the name to be renamed if there is one.
> +      if (name && TREE_CODE (name) == SSA_NAME)
> +    bitmap_set_bit (inserted_exprs, SSA_NAME_VERSION (name));
>       gimple_stmt_iterator gsi1 = gsi_for_stmt (stmt_to_move);
>       gsi_move_before (&gsi1, &gsi);
>       reset_flow_sensitive_info (gimple_assign_lhs (stmt_to_move));
>     }
> 
> -  replace_phi_edge_with_variable (cond_bb, e1, phi, result);
> +  replace_phi_edge_with_variable (cond_bb, e1, phi, result, inserted_exprs);
> 
>   /* Add Statistic here even though replace_phi_edge_with_variable already
>      does it as we want to be able to count when match-simplify happens vs
> -- 
> 1.8.3.1
>