[PATCH] PHIOPT: Mark the conditional lhs and rhs as to look at to see if DCEable

[PATCH] PHIOPT: Mark the conditional lhs and rhs as to look at to see if DCEable

[Andrew Pinski]

In some cases (usually dealing with bools only), there could be some statements
left behind which are considered trivial dead.
An example is:
```
bool f(bool a, bool b)
{
    if (!a && !b)
        return 0;
    if (!a && b)
        return 0;
    if (a && !b)
        return 0;
    return 1;
}
```
Where during phiopt2, the IR had:
```
  _3 = ~b_7(D);
  _4 = _3 & a_6(D);
  _4 != 0 ? 0 : 1
```
match-and-simplify would transform that into:
```
  _11 = ~a_6(D);
  _12 = b_7(D) | _11;
```
But phiopt would leave around the statements defining _4 and _3.
This helps by marking the conditional's lhs and rhs to see if they are
trivial dead.

OK? Bootstrapped and tested on x86_64-linux-gnu.

gcc/ChangeLog:

	* tree-ssa-phiopt.cc (match_simplify_replacement): Mark's cond
	statement's lhs and rhs to check if trivial dead.
	Rename inserted_exprs to exprs_maybe_dce; also move it so
	bitmap is not allocated if not needed.
---
 gcc/tree-ssa-phiopt.cc | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc
index cb4e2da023d..ff36bb0119b 100644
--- a/gcc/tree-ssa-phiopt.cc
+++ b/gcc/tree-ssa-phiopt.cc
@@ -767,7 +767,6 @@ match_simplify_replacement (basic_block cond_bb, basic_block middle_bb,
   tree result;
   gimple *stmt_to_move = NULL;
   gimple *stmt_to_move_alt = NULL;
-  auto_bitmap inserted_exprs;
   tree arg_true, arg_false;
 
   /* Special case A ? B : B as this will always simplify to B. */
@@ -844,6 +843,18 @@ match_simplify_replacement (basic_block cond_bb, basic_block middle_bb,
   if (!result)
     return false;
 
+  auto_bitmap exprs_maybe_dce;
+
+  /* Mark the cond statements' lhs/rhs as maybe dce.  */
+  if (TREE_CODE (gimple_cond_lhs (stmt)) == SSA_NAME
+      && !SSA_NAME_IS_DEFAULT_DEF (gimple_cond_lhs (stmt)))
+    bitmap_set_bit (exprs_maybe_dce,
+		    SSA_NAME_VERSION (gimple_cond_lhs (stmt)));
+  if (TREE_CODE (gimple_cond_rhs (stmt)) == SSA_NAME
+      && !SSA_NAME_IS_DEFAULT_DEF (gimple_cond_rhs (stmt)))
+    bitmap_set_bit (exprs_maybe_dce,
+		    SSA_NAME_VERSION (gimple_cond_rhs (stmt)));
+
   gsi = gsi_last_bb (cond_bb);
   /* Insert the sequence generated from gimple_simplify_phiopt.  */
   if (seq)
@@ -855,7 +866,7 @@ match_simplify_replacement (basic_block cond_bb, basic_block middle_bb,
 	  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));
+	    bitmap_set_bit (exprs_maybe_dce, SSA_NAME_VERSION (name));
 	}
       if (dump_file && (dump_flags & TDF_FOLDING))
 	{
@@ -867,10 +878,10 @@ match_simplify_replacement (basic_block cond_bb, basic_block middle_bb,
 
   /* If there was a statement to move, move it to right before
      the original conditional.  */
-  move_stmt (stmt_to_move, &gsi, inserted_exprs);
-  move_stmt (stmt_to_move_alt, &gsi, inserted_exprs);
+  move_stmt (stmt_to_move, &gsi, exprs_maybe_dce);
+  move_stmt (stmt_to_move_alt, &gsi, exprs_maybe_dce);
 
-  replace_phi_edge_with_variable (cond_bb, e1, phi, result, inserted_exprs);
+  replace_phi_edge_with_variable (cond_bb, e1, phi, result, exprs_maybe_dce);
 
   /* 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
-- 
2.31.1

Re: [PATCH] PHIOPT: Mark the conditional lhs and rhs as to look at to see if DCEable

[Richard Biener]

On Mon, Jul 31, 2023 at 10:17 PM Andrew Pinski via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> In some cases (usually dealing with bools only), there could be some statements
> left behind which are considered trivial dead.
> An example is:
> ```
> bool f(bool a, bool b)
> {
>     if (!a && !b)
>         return 0;
>     if (!a && b)
>         return 0;
>     if (a && !b)
>         return 0;
>     return 1;
> }
> ```
> Where during phiopt2, the IR had:
> ```
>   _3 = ~b_7(D);
>   _4 = _3 & a_6(D);
>   _4 != 0 ? 0 : 1
> ```
> match-and-simplify would transform that into:
> ```
>   _11 = ~a_6(D);
>   _12 = b_7(D) | _11;
> ```
> But phiopt would leave around the statements defining _4 and _3.
> This helps by marking the conditional's lhs and rhs to see if they are
> trivial dead.
>
> OK? Bootstrapped and tested on x86_64-linux-gnu.

OK.

> gcc/ChangeLog:
>
>         * tree-ssa-phiopt.cc (match_simplify_replacement): Mark's cond
>         statement's lhs and rhs to check if trivial dead.
>         Rename inserted_exprs to exprs_maybe_dce; also move it so
>         bitmap is not allocated if not needed.
> ---
>  gcc/tree-ssa-phiopt.cc | 21 ++++++++++++++++-----
>  1 file changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc
> index cb4e2da023d..ff36bb0119b 100644
> --- a/gcc/tree-ssa-phiopt.cc
> +++ b/gcc/tree-ssa-phiopt.cc
> @@ -767,7 +767,6 @@ match_simplify_replacement (basic_block cond_bb, basic_block middle_bb,
>    tree result;
>    gimple *stmt_to_move = NULL;
>    gimple *stmt_to_move_alt = NULL;
> -  auto_bitmap inserted_exprs;
>    tree arg_true, arg_false;
>
>    /* Special case A ? B : B as this will always simplify to B. */
> @@ -844,6 +843,18 @@ match_simplify_replacement (basic_block cond_bb, basic_block middle_bb,
>    if (!result)
>      return false;
>
> +  auto_bitmap exprs_maybe_dce;
> +
> +  /* Mark the cond statements' lhs/rhs as maybe dce.  */
> +  if (TREE_CODE (gimple_cond_lhs (stmt)) == SSA_NAME
> +      && !SSA_NAME_IS_DEFAULT_DEF (gimple_cond_lhs (stmt)))
> +    bitmap_set_bit (exprs_maybe_dce,
> +                   SSA_NAME_VERSION (gimple_cond_lhs (stmt)));
> +  if (TREE_CODE (gimple_cond_rhs (stmt)) == SSA_NAME
> +      && !SSA_NAME_IS_DEFAULT_DEF (gimple_cond_rhs (stmt)))
> +    bitmap_set_bit (exprs_maybe_dce,
> +                   SSA_NAME_VERSION (gimple_cond_rhs (stmt)));
> +
>    gsi = gsi_last_bb (cond_bb);
>    /* Insert the sequence generated from gimple_simplify_phiopt.  */
>    if (seq)
> @@ -855,7 +866,7 @@ match_simplify_replacement (basic_block cond_bb, basic_block middle_bb,
>           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));
> +           bitmap_set_bit (exprs_maybe_dce, SSA_NAME_VERSION (name));
>         }
>        if (dump_file && (dump_flags & TDF_FOLDING))
>         {
> @@ -867,10 +878,10 @@ match_simplify_replacement (basic_block cond_bb, basic_block middle_bb,
>
>    /* If there was a statement to move, move it to right before
>       the original conditional.  */
> -  move_stmt (stmt_to_move, &gsi, inserted_exprs);
> -  move_stmt (stmt_to_move_alt, &gsi, inserted_exprs);
> +  move_stmt (stmt_to_move, &gsi, exprs_maybe_dce);
> +  move_stmt (stmt_to_move_alt, &gsi, exprs_maybe_dce);
>
> -  replace_phi_edge_with_variable (cond_bb, e1, phi, result, inserted_exprs);
> +  replace_phi_edge_with_variable (cond_bb, e1, phi, result, exprs_maybe_dce);
>
>    /* 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
> --
> 2.31.1
>