[PATCH] [PHIOPT/MATCH] Remove the statement to move if not used

[PATCH] [PHIOPT/MATCH] Remove the statement to move if not used

[apinski]

From: Andrew Pinski <apinski@marvell.com>

Instead of waiting for DCE to remove the unused statement,
and maybe optimize another conditional, it is better if
we don't move the statement and have the statement
removed.

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

Changes from v1:
* v2: Change the order of insertation and check to see if the lhs
  is used rather than see if the lhs was used in the sequence.

gcc/ChangeLog:

	* tree-ssa-phiopt.c (match_simplify_replacement): Move
	insert of the sequence before the movement of the
	statement. Check if to see if the statement is used
	outside of the original phi to see if we should move it.

gcc/testsuite/ChangeLog:

	* gcc.dg/tree-ssa/pr96928-1.c: Update to similar as pr96928.c.
---
 gcc/testsuite/gcc.dg/tree-ssa/pr96928-1.c |  5 ++++-
 gcc/tree-ssa-phiopt.c                     | 13 ++++++++++---
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr96928-1.c b/gcc/testsuite/gcc.dg/tree-ssa/pr96928-1.c
index 2e86620da11..9e505ac9900 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/pr96928-1.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr96928-1.c
@@ -2,7 +2,10 @@
 /* { dg-do compile } */
 /* { dg-options "-O2 -fdump-tree-phiopt2 -fdump-tree-optimized" } */
 /* { dg-final { scan-tree-dump-times " = a_\[0-9]*\\\(D\\\) >> " 5 "phiopt2" } } */
-/* { dg-final { scan-tree-dump-times " = ~c_\[0-9]*\\\(D\\\);" 1 "phiopt2" } } */
+/* The following check is done at optimized because a ^ (~b) is rewritten as ~(a^b)
+   and in the case of match.pd optimizing these ?:, the ~ is moved out already
+   by the time we get to phiopt2. */
+/* { dg-final { scan-tree-dump-times "c_\[0-9]*\\\(D\\\) \\\^" 1 "optimized" } } */
 /* { dg-final { scan-tree-dump-times " = ~" 1 "optimized" } } */
 /* { dg-final { scan-tree-dump-times " = \[abc_0-9\\\(\\\)D]* \\\^ " 5 "phiopt2" } } */
 /* { dg-final { scan-tree-dump-not "a < 0" "phiopt2" } } */
diff --git a/gcc/tree-ssa-phiopt.c b/gcc/tree-ssa-phiopt.c
index 7a98b7afdf1..c6adbbd28a0 100644
--- a/gcc/tree-ssa-phiopt.c
+++ b/gcc/tree-ssa-phiopt.c
@@ -1020,7 +1020,16 @@ match_simplify_replacement (basic_block cond_bb, basic_block middle_bb,
     return false;
 
   gsi = gsi_last_bb (cond_bb);
-  if (stmt_to_move)
+  /* Insert the sequence generated from gimple_simplify_phiopt.  */
+  if (seq)
+    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 (dump_file && (dump_flags & TDF_DETAILS))
 	{
@@ -1032,8 +1041,6 @@ match_simplify_replacement (basic_block cond_bb, basic_block middle_bb,
       gsi_move_before (&gsi1, &gsi);
       reset_flow_sensitive_info (gimple_assign_lhs (stmt_to_move));
     }
-  if (seq)
-    gsi_insert_seq_before (&gsi, seq, GSI_SAME_STMT);
 
   replace_phi_edge_with_variable (cond_bb, e1, phi, result);
 
-- 
2.27.0

Re: [PATCH] [PHIOPT/MATCH] Remove the statement to move if not used

[Richard Biener]

On Fri, Jul 9, 2021 at 10:05 PM apinski--- via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> From: Andrew Pinski <apinski@marvell.com>
>
> Instead of waiting for DCE to remove the unused statement,
> and maybe optimize another conditional, it is better if
> we don't move the statement and have the statement
> removed.
>
> OK? Bootstrapped and tested on x86_64-linux-gnu.

OK.

Thanks,
Richard.

> Changes from v1:
> * v2: Change the order of insertation and check to see if the lhs
>   is used rather than see if the lhs was used in the sequence.
>
> gcc/ChangeLog:
>
>         * tree-ssa-phiopt.c (match_simplify_replacement): Move
>         insert of the sequence before the movement of the
>         statement. Check if to see if the statement is used
>         outside of the original phi to see if we should move it.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.dg/tree-ssa/pr96928-1.c: Update to similar as pr96928.c.
> ---
>  gcc/testsuite/gcc.dg/tree-ssa/pr96928-1.c |  5 ++++-
>  gcc/tree-ssa-phiopt.c                     | 13 ++++++++++---
>  2 files changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr96928-1.c b/gcc/testsuite/gcc.dg/tree-ssa/pr96928-1.c
> index 2e86620da11..9e505ac9900 100644
> --- a/gcc/testsuite/gcc.dg/tree-ssa/pr96928-1.c
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr96928-1.c
> @@ -2,7 +2,10 @@
>  /* { dg-do compile } */
>  /* { dg-options "-O2 -fdump-tree-phiopt2 -fdump-tree-optimized" } */
>  /* { dg-final { scan-tree-dump-times " = a_\[0-9]*\\\(D\\\) >> " 5 "phiopt2" } } */
> -/* { dg-final { scan-tree-dump-times " = ~c_\[0-9]*\\\(D\\\);" 1 "phiopt2" } } */
> +/* The following check is done at optimized because a ^ (~b) is rewritten as ~(a^b)
> +   and in the case of match.pd optimizing these ?:, the ~ is moved out already
> +   by the time we get to phiopt2. */
> +/* { dg-final { scan-tree-dump-times "c_\[0-9]*\\\(D\\\) \\\^" 1 "optimized" } } */
>  /* { dg-final { scan-tree-dump-times " = ~" 1 "optimized" } } */
>  /* { dg-final { scan-tree-dump-times " = \[abc_0-9\\\(\\\)D]* \\\^ " 5 "phiopt2" } } */
>  /* { dg-final { scan-tree-dump-not "a < 0" "phiopt2" } } */
> diff --git a/gcc/tree-ssa-phiopt.c b/gcc/tree-ssa-phiopt.c
> index 7a98b7afdf1..c6adbbd28a0 100644
> --- a/gcc/tree-ssa-phiopt.c
> +++ b/gcc/tree-ssa-phiopt.c
> @@ -1020,7 +1020,16 @@ match_simplify_replacement (basic_block cond_bb, basic_block middle_bb,
>      return false;
>
>    gsi = gsi_last_bb (cond_bb);
> -  if (stmt_to_move)
> +  /* Insert the sequence generated from gimple_simplify_phiopt.  */
> +  if (seq)
> +    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 (dump_file && (dump_flags & TDF_DETAILS))
>         {
> @@ -1032,8 +1041,6 @@ match_simplify_replacement (basic_block cond_bb, basic_block middle_bb,
>        gsi_move_before (&gsi1, &gsi);
>        reset_flow_sensitive_info (gimple_assign_lhs (stmt_to_move));
>      }
> -  if (seq)
> -    gsi_insert_seq_before (&gsi, seq, GSI_SAME_STMT);
>
>    replace_phi_edge_with_variable (cond_bb, e1, phi, result);
>
> --
> 2.27.0
>