diff mbox series

slsr: Use simple_dce_from_worklist in SLSR [PR116554]

Message ID 20240901040618.1308809-1-quic_apinski@quicinc.com
State New
Headers show
Series slsr: Use simple_dce_from_worklist in SLSR [PR116554] | expand

Commit Message

Andrew Pinski Sept. 1, 2024, 4:06 a.m. UTC
While working on a phiopt patch, it was noticed that
SLSR would leave around some unused ssa names. Let's
add simple_dce_from_worklist usage to SLSR to remove
the dead statements. This should give a small improvemnent
for passes afterwards.

Boostrapped and tested on x86_64.

gcc/ChangeLog:

	PR tree-optimization/116554
	* gimple-ssa-strength-reduction.cc: Include tree-ssa-dce.h.
	(replace_mult_candidate): Add sdce_worklist argument, mark
	the rhs1/rhs2 for maybe dceing.
	(replace_unconditional_candidate): Add sdce_worklist argument,
	Update call to replace_mult_candidate.
	(replace_conditional_candidate): Add sdce_worklist argument,
	update call to replace_mult_candidate.
	(replace_uncond_cands_and_profitable_phis): Add sdce_worklist argument,
	update call to replace_conditional_candidate,
	replace_unconditional_candidate, and replace_uncond_cands_and_profitable_phis.
	(replace_one_candidate): Add sdce_worklist argument, mark
	the orig_rhs1/orig_rhs2 for maybe dceing.
	(replace_profitable_candidates): Add sdce_worklist argument,
	update call to replace_one_candidate and replace_profitable_candidates.
	(analyze_candidates_and_replace): Call simple_dce_from_worklist and
	update calls to replace_profitable_candidates, and
	replace_uncond_cands_and_profitable_phis.

Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>
---
 gcc/gimple-ssa-strength-reduction.cc | 59 +++++++++++++++++++---------
 1 file changed, 41 insertions(+), 18 deletions(-)

Comments

Richard Biener Sept. 1, 2024, 6:32 a.m. UTC | #1
> Am 01.09.2024 um 06:07 schrieb Andrew Pinski <quic_apinski@quicinc.com>:
> 
> While working on a phiopt patch, it was noticed that
> SLSR would leave around some unused ssa names. Let's
> add simple_dce_from_worklist usage to SLSR to remove
> the dead statements. This should give a small improvemnent
> for passes afterwards.
> 
> Boostrapped and tested on x86_64.

Ok

Richard 

> gcc/ChangeLog:
> 
>    PR tree-optimization/116554
>    * gimple-ssa-strength-reduction.cc: Include tree-ssa-dce.h.
>    (replace_mult_candidate): Add sdce_worklist argument, mark
>    the rhs1/rhs2 for maybe dceing.
>    (replace_unconditional_candidate): Add sdce_worklist argument,
>    Update call to replace_mult_candidate.
>    (replace_conditional_candidate): Add sdce_worklist argument,
>    update call to replace_mult_candidate.
>    (replace_uncond_cands_and_profitable_phis): Add sdce_worklist argument,
>    update call to replace_conditional_candidate,
>    replace_unconditional_candidate, and replace_uncond_cands_and_profitable_phis.
>    (replace_one_candidate): Add sdce_worklist argument, mark
>    the orig_rhs1/orig_rhs2 for maybe dceing.
>    (replace_profitable_candidates): Add sdce_worklist argument,
>    update call to replace_one_candidate and replace_profitable_candidates.
>    (analyze_candidates_and_replace): Call simple_dce_from_worklist and
>    update calls to replace_profitable_candidates, and
>    replace_uncond_cands_and_profitable_phis.
> 
> Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>
> ---
> gcc/gimple-ssa-strength-reduction.cc | 59 +++++++++++++++++++---------
> 1 file changed, 41 insertions(+), 18 deletions(-)
> 
> diff --git a/gcc/gimple-ssa-strength-reduction.cc b/gcc/gimple-ssa-strength-reduction.cc
> index 1cb3625c7eb..39cd9339c77 100644
> --- a/gcc/gimple-ssa-strength-reduction.cc
> +++ b/gcc/gimple-ssa-strength-reduction.cc
> @@ -56,6 +56,7 @@ along with GCC; see the file COPYING3.  If not see
> #include "tree-affine.h"
> #include "tree-eh.h"
> #include "builtins.h"
> +#include "tree-ssa-dce.h"
> 
> /* Information about a strength reduction candidate.  Each statement
>    in the candidate table represents an expression of one of the
> @@ -2126,7 +2127,8 @@ cand_already_replaced (slsr_cand_t c)
>    replace_conditional_candidate.  */
> 
> static void
> -replace_mult_candidate (slsr_cand_t c, tree basis_name, offset_int bump)
> +replace_mult_candidate (slsr_cand_t c, tree basis_name, offset_int bump,
> +            auto_bitmap &sdce_worklist)
> {
>   tree target_type = TREE_TYPE (gimple_assign_lhs (c->cand_stmt));
>   enum tree_code cand_code = gimple_assign_rhs_code (c->cand_stmt);
> @@ -2193,6 +2195,11 @@ replace_mult_candidate (slsr_cand_t c, tree basis_name, offset_int bump)
>       if (cand_code != NEGATE_EXPR) {
>    rhs1 = gimple_assign_rhs1 (c->cand_stmt);
>    rhs2 = gimple_assign_rhs2 (c->cand_stmt);
> +    /* Mark the 2 original rhs for maybe DCEing.  */
> +    if (TREE_CODE (rhs1) == SSA_NAME)
> +      bitmap_set_bit (sdce_worklist, SSA_NAME_VERSION (rhs1));
> +    if (TREE_CODE (rhs2) == SSA_NAME)
> +      bitmap_set_bit (sdce_worklist, SSA_NAME_VERSION (rhs2));
>       }
>       if (cand_code != NEGATE_EXPR
>      && ((operand_equal_p (rhs1, basis_name, 0)
> @@ -2237,7 +2244,7 @@ replace_mult_candidate (slsr_cand_t c, tree basis_name, offset_int bump)
>    folded value ((i - i') * S) is referred to here as the "bump."  */
> 
> static void
> -replace_unconditional_candidate (slsr_cand_t c)
> +replace_unconditional_candidate (slsr_cand_t c, auto_bitmap &sdce_worklist)
> {
>   slsr_cand_t basis;
> 
> @@ -2247,7 +2254,8 @@ replace_unconditional_candidate (slsr_cand_t c)
>   basis = lookup_cand (c->basis);
>   offset_int bump = cand_increment (c) * wi::to_offset (c->stride);
> 
> -  replace_mult_candidate (c, gimple_assign_lhs (basis->cand_stmt), bump);
> +  replace_mult_candidate (c, gimple_assign_lhs (basis->cand_stmt), bump,
> +              sdce_worklist);
> }
> 
> /* Return the index in the increment vector of the given INCREMENT,
> @@ -2507,7 +2515,8 @@ create_phi_basis (slsr_cand_t c, gimple *from_phi, tree basis_name,
>    basis.  */
> 
> static void
> -replace_conditional_candidate (slsr_cand_t c)
> +replace_conditional_candidate (slsr_cand_t c, auto_bitmap &sdce_worklist)
> +
> {
>   tree basis_name, name;
>   slsr_cand_t basis;
> @@ -2527,7 +2536,7 @@ replace_conditional_candidate (slsr_cand_t c)
>   /* Replace C with an add of the new basis phi and a constant.  */
>   offset_int bump = c->index * wi::to_offset (c->stride);
> 
> -  replace_mult_candidate (c, name, bump);
> +  replace_mult_candidate (c, name, bump, sdce_worklist);
> }
> 
> /* Recursive helper function for phi_add_costs.  SPREAD is a measure of
> @@ -2608,7 +2617,8 @@ phi_add_costs (gimple *phi, slsr_cand_t c, int one_add_cost)
>    so, replace the candidate and introduce the compensation code.  */
> 
> static void
> -replace_uncond_cands_and_profitable_phis (slsr_cand_t c)
> +replace_uncond_cands_and_profitable_phis (slsr_cand_t c,
> +                      auto_bitmap &sdce_worklist)
> {
>   if (phi_dependent_cand_p (c))
>     {
> @@ -2643,17 +2653,19 @@ replace_uncond_cands_and_profitable_phis (slsr_cand_t c)
>        }
> 
>      if (cost <= COST_NEUTRAL)
> -        replace_conditional_candidate (c);
> +        replace_conditional_candidate (c, sdce_worklist);
>    }
>     }
>   else
> -    replace_unconditional_candidate (c);
> +    replace_unconditional_candidate (c, sdce_worklist);
> 
>   if (c->sibling)
> -    replace_uncond_cands_and_profitable_phis (lookup_cand (c->sibling));
> +    replace_uncond_cands_and_profitable_phis (lookup_cand (c->sibling),
> +                          sdce_worklist);
> 
>   if (c->dependent)
> -    replace_uncond_cands_and_profitable_phis (lookup_cand (c->dependent));
> +    replace_uncond_cands_and_profitable_phis (lookup_cand (c->dependent),
> +                          sdce_worklist);
> }
> 
> /* Count the number of candidates in the tree rooted at C that have
> @@ -3675,7 +3687,8 @@ replace_rhs_if_not_dup (enum tree_code new_code, tree new_rhs1, tree new_rhs2,
>    is the rhs1 to use in creating the add/subtract.  */
> 
> static void
> -replace_one_candidate (slsr_cand_t c, unsigned i, tree basis_name)
> +replace_one_candidate (slsr_cand_t c, unsigned i, tree basis_name,
> +               auto_bitmap &sdce_worklist)
> {
>   gimple *stmt_to_print = NULL;
>   tree orig_rhs1, orig_rhs2;
> @@ -3693,6 +3706,12 @@ replace_one_candidate (slsr_cand_t c, unsigned i, tree basis_name)
>   if (!orig_rhs2)
>     return;
> 
> +  /* Mark the 2 original rhs for maybe DCEing.  */
> +  if (TREE_CODE (orig_rhs1) == SSA_NAME)
> +    bitmap_set_bit (sdce_worklist, SSA_NAME_VERSION (orig_rhs1));
> +  if (TREE_CODE (orig_rhs2) == SSA_NAME)
> +   bitmap_set_bit (sdce_worklist, SSA_NAME_VERSION (orig_rhs2));
> +
>   if (dump_file && (dump_flags & TDF_DETAILS))
>     {
>       fputs ("Replacing: ", dump_file);
> @@ -3835,7 +3854,7 @@ replace_one_candidate (slsr_cand_t c, unsigned i, tree basis_name)
>    an increment if such has been shown to be profitable.  */
> 
> static void
> -replace_profitable_candidates (slsr_cand_t c)
> +replace_profitable_candidates (slsr_cand_t c, auto_bitmap &sdce_worklist)
> {
>   if (!cand_already_replaced (c))
>     {
> @@ -3872,23 +3891,23 @@ replace_profitable_candidates (slsr_cand_t c)
> 
>          /* Replace C with an add of the new basis phi and the
>             increment.  */
> -          replace_one_candidate (c, i, name);
> +          replace_one_candidate (c, i, name, sdce_worklist);
>        }
>        }
>      else
>        {
>          slsr_cand_t basis = lookup_cand (c->basis);
>          tree basis_name = gimple_assign_lhs (basis->cand_stmt);
> -          replace_one_candidate (c, i, basis_name);
> +          replace_one_candidate (c, i, basis_name, sdce_worklist);
>        }
>    }
>     }
> 
>   if (c->sibling)
> -    replace_profitable_candidates (lookup_cand (c->sibling));
> +    replace_profitable_candidates (lookup_cand (c->sibling), sdce_worklist);
> 
>   if (c->dependent)
> -    replace_profitable_candidates (lookup_cand (c->dependent));
> +    replace_profitable_candidates (lookup_cand (c->dependent), sdce_worklist);
> }
> 
> /* Analyze costs of related candidates in the candidate vector,
> @@ -3899,6 +3918,7 @@ analyze_candidates_and_replace (void)
> {
>   unsigned i;
>   slsr_cand_t c;
> +  auto_bitmap simple_dce_worklist;
> 
>   /* Each candidate that has a null basis and a non-null
>      dependent is the root of a tree of related statements.
> @@ -3932,7 +3952,8 @@ analyze_candidates_and_replace (void)
>     compensation code it requires is offset by the strength
>     reduction savings.  */
>       else if (TREE_CODE (c->stride) == INTEGER_CST)
> -    replace_uncond_cands_and_profitable_phis (first_dep);
> +    replace_uncond_cands_and_profitable_phis (first_dep,
> +                          simple_dce_worklist);
> 
>       /* When the stride is an SSA name, it may still be profitable
>     to replace some or all of the dependent candidates, depending
> @@ -3969,7 +3990,7 @@ analyze_candidates_and_replace (void)
>      dump_incr_vec ();
> 
>      /* Perform the replacements.  */
> -      replace_profitable_candidates (first_dep);
> +      replace_profitable_candidates (first_dep, simple_dce_worklist);
>      free (incr_vec);
>    }
>     }
> @@ -3977,6 +3998,8 @@ analyze_candidates_and_replace (void)
>   /* For conditional candidates, we may have uncommitted insertions
>      on edges to clean up.  */
>   gsi_commit_edge_inserts ();
> +
> +  simple_dce_from_worklist (simple_dce_worklist);
> }
> 
> namespace {
> --
> 2.43.0
>
diff mbox series

Patch

diff --git a/gcc/gimple-ssa-strength-reduction.cc b/gcc/gimple-ssa-strength-reduction.cc
index 1cb3625c7eb..39cd9339c77 100644
--- a/gcc/gimple-ssa-strength-reduction.cc
+++ b/gcc/gimple-ssa-strength-reduction.cc
@@ -56,6 +56,7 @@  along with GCC; see the file COPYING3.  If not see
 #include "tree-affine.h"
 #include "tree-eh.h"
 #include "builtins.h"
+#include "tree-ssa-dce.h"
 
 /* Information about a strength reduction candidate.  Each statement
    in the candidate table represents an expression of one of the
@@ -2126,7 +2127,8 @@  cand_already_replaced (slsr_cand_t c)
    replace_conditional_candidate.  */
 
 static void
-replace_mult_candidate (slsr_cand_t c, tree basis_name, offset_int bump)
+replace_mult_candidate (slsr_cand_t c, tree basis_name, offset_int bump,
+			auto_bitmap &sdce_worklist)
 {
   tree target_type = TREE_TYPE (gimple_assign_lhs (c->cand_stmt));
   enum tree_code cand_code = gimple_assign_rhs_code (c->cand_stmt);
@@ -2193,6 +2195,11 @@  replace_mult_candidate (slsr_cand_t c, tree basis_name, offset_int bump)
       if (cand_code != NEGATE_EXPR) {
 	rhs1 = gimple_assign_rhs1 (c->cand_stmt);
 	rhs2 = gimple_assign_rhs2 (c->cand_stmt);
+	/* Mark the 2 original rhs for maybe DCEing.  */
+	if (TREE_CODE (rhs1) == SSA_NAME)
+	  bitmap_set_bit (sdce_worklist, SSA_NAME_VERSION (rhs1));
+	if (TREE_CODE (rhs2) == SSA_NAME)
+	  bitmap_set_bit (sdce_worklist, SSA_NAME_VERSION (rhs2));
       }
       if (cand_code != NEGATE_EXPR
 	  && ((operand_equal_p (rhs1, basis_name, 0)
@@ -2237,7 +2244,7 @@  replace_mult_candidate (slsr_cand_t c, tree basis_name, offset_int bump)
    folded value ((i - i') * S) is referred to here as the "bump."  */
 
 static void
-replace_unconditional_candidate (slsr_cand_t c)
+replace_unconditional_candidate (slsr_cand_t c, auto_bitmap &sdce_worklist)
 {
   slsr_cand_t basis;
 
@@ -2247,7 +2254,8 @@  replace_unconditional_candidate (slsr_cand_t c)
   basis = lookup_cand (c->basis);
   offset_int bump = cand_increment (c) * wi::to_offset (c->stride);
 
-  replace_mult_candidate (c, gimple_assign_lhs (basis->cand_stmt), bump);
+  replace_mult_candidate (c, gimple_assign_lhs (basis->cand_stmt), bump,
+			  sdce_worklist);
 }
 
 /* Return the index in the increment vector of the given INCREMENT,
@@ -2507,7 +2515,8 @@  create_phi_basis (slsr_cand_t c, gimple *from_phi, tree basis_name,
    basis.  */
 
 static void
-replace_conditional_candidate (slsr_cand_t c)
+replace_conditional_candidate (slsr_cand_t c, auto_bitmap &sdce_worklist)
+
 {
   tree basis_name, name;
   slsr_cand_t basis;
@@ -2527,7 +2536,7 @@  replace_conditional_candidate (slsr_cand_t c)
   /* Replace C with an add of the new basis phi and a constant.  */
   offset_int bump = c->index * wi::to_offset (c->stride);
 
-  replace_mult_candidate (c, name, bump);
+  replace_mult_candidate (c, name, bump, sdce_worklist);
 }
 
 /* Recursive helper function for phi_add_costs.  SPREAD is a measure of
@@ -2608,7 +2617,8 @@  phi_add_costs (gimple *phi, slsr_cand_t c, int one_add_cost)
    so, replace the candidate and introduce the compensation code.  */
 
 static void
-replace_uncond_cands_and_profitable_phis (slsr_cand_t c)
+replace_uncond_cands_and_profitable_phis (slsr_cand_t c,
+					  auto_bitmap &sdce_worklist)
 {
   if (phi_dependent_cand_p (c))
     {
@@ -2643,17 +2653,19 @@  replace_uncond_cands_and_profitable_phis (slsr_cand_t c)
 	    }
 
 	  if (cost <= COST_NEUTRAL)
-	    replace_conditional_candidate (c);
+	    replace_conditional_candidate (c, sdce_worklist);
 	}
     }
   else
-    replace_unconditional_candidate (c);
+    replace_unconditional_candidate (c, sdce_worklist);
 
   if (c->sibling)
-    replace_uncond_cands_and_profitable_phis (lookup_cand (c->sibling));
+    replace_uncond_cands_and_profitable_phis (lookup_cand (c->sibling),
+					      sdce_worklist);
 
   if (c->dependent)
-    replace_uncond_cands_and_profitable_phis (lookup_cand (c->dependent));
+    replace_uncond_cands_and_profitable_phis (lookup_cand (c->dependent),
+					      sdce_worklist);
 }
 
 /* Count the number of candidates in the tree rooted at C that have
@@ -3675,7 +3687,8 @@  replace_rhs_if_not_dup (enum tree_code new_code, tree new_rhs1, tree new_rhs2,
    is the rhs1 to use in creating the add/subtract.  */
 
 static void
-replace_one_candidate (slsr_cand_t c, unsigned i, tree basis_name)
+replace_one_candidate (slsr_cand_t c, unsigned i, tree basis_name,
+		       auto_bitmap &sdce_worklist)
 {
   gimple *stmt_to_print = NULL;
   tree orig_rhs1, orig_rhs2;
@@ -3693,6 +3706,12 @@  replace_one_candidate (slsr_cand_t c, unsigned i, tree basis_name)
   if (!orig_rhs2)
     return;
 
+  /* Mark the 2 original rhs for maybe DCEing.  */
+  if (TREE_CODE (orig_rhs1) == SSA_NAME)
+    bitmap_set_bit (sdce_worklist, SSA_NAME_VERSION (orig_rhs1));
+  if (TREE_CODE (orig_rhs2) == SSA_NAME)
+   bitmap_set_bit (sdce_worklist, SSA_NAME_VERSION (orig_rhs2));
+
   if (dump_file && (dump_flags & TDF_DETAILS))
     {
       fputs ("Replacing: ", dump_file);
@@ -3835,7 +3854,7 @@  replace_one_candidate (slsr_cand_t c, unsigned i, tree basis_name)
    an increment if such has been shown to be profitable.  */
 
 static void
-replace_profitable_candidates (slsr_cand_t c)
+replace_profitable_candidates (slsr_cand_t c, auto_bitmap &sdce_worklist)
 {
   if (!cand_already_replaced (c))
     {
@@ -3872,23 +3891,23 @@  replace_profitable_candidates (slsr_cand_t c)
 
 		  /* Replace C with an add of the new basis phi and the
 		     increment.  */
-		  replace_one_candidate (c, i, name);
+		  replace_one_candidate (c, i, name, sdce_worklist);
 		}
 	    }
 	  else
 	    {
 	      slsr_cand_t basis = lookup_cand (c->basis);
 	      tree basis_name = gimple_assign_lhs (basis->cand_stmt);
-	      replace_one_candidate (c, i, basis_name);
+	      replace_one_candidate (c, i, basis_name, sdce_worklist);
 	    }
 	}
     }
 
   if (c->sibling)
-    replace_profitable_candidates (lookup_cand (c->sibling));
+    replace_profitable_candidates (lookup_cand (c->sibling), sdce_worklist);
 
   if (c->dependent)
-    replace_profitable_candidates (lookup_cand (c->dependent));
+    replace_profitable_candidates (lookup_cand (c->dependent), sdce_worklist);
 }
 
 /* Analyze costs of related candidates in the candidate vector,
@@ -3899,6 +3918,7 @@  analyze_candidates_and_replace (void)
 {
   unsigned i;
   slsr_cand_t c;
+  auto_bitmap simple_dce_worklist;
 
   /* Each candidate that has a null basis and a non-null
      dependent is the root of a tree of related statements.
@@ -3932,7 +3952,8 @@  analyze_candidates_and_replace (void)
 	 compensation code it requires is offset by the strength
 	 reduction savings.  */
       else if (TREE_CODE (c->stride) == INTEGER_CST)
-	replace_uncond_cands_and_profitable_phis (first_dep);
+	replace_uncond_cands_and_profitable_phis (first_dep,
+						  simple_dce_worklist);
 
       /* When the stride is an SSA name, it may still be profitable
 	 to replace some or all of the dependent candidates, depending
@@ -3969,7 +3990,7 @@  analyze_candidates_and_replace (void)
 	  dump_incr_vec ();
 
 	  /* Perform the replacements.  */
-	  replace_profitable_candidates (first_dep);
+	  replace_profitable_candidates (first_dep, simple_dce_worklist);
 	  free (incr_vec);
 	}
     }
@@ -3977,6 +3998,8 @@  analyze_candidates_and_replace (void)
   /* For conditional candidates, we may have uncommitted insertions
      on edges to clean up.  */
   gsi_commit_edge_inserts ();
+
+  simple_dce_from_worklist (simple_dce_worklist);
 }
 
 namespace {