[PATCH 1/2] MATCH: change single_non_singleton_phi_for_edges for singleton phis

[PATCH 1/2] MATCH: change single_non_singleton_phi_for_edges for singleton phis

[Andrew Pinski]

I noticed that single_non_singleton_phi_for_edges could
return a phi whos entry are all the same for the edge.
This happens only if there was a single phis in the first place.
Also gimple_seq_singleton_p walks the sequence to see if it the one
element in the sequence so there is removing that check actually
reduces the number of pointer walks needed.

Bootstrapped and tested on x86_64-linux-gnu with no regressions.

gcc/ChangeLog:

	* tree-ssa-phiopt.cc (single_non_singleton_phi_for_edges):
	Remove the special case of gimple_seq_singleton_p.

Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>
---
 gcc/tree-ssa-phiopt.cc | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc
index d1746c4b468..f1e07502b02 100644
--- a/gcc/tree-ssa-phiopt.cc
+++ b/gcc/tree-ssa-phiopt.cc
@@ -62,14 +62,6 @@ single_non_singleton_phi_for_edges (gimple_seq seq, edge e0, edge e1)
 {
   gimple_stmt_iterator i;
   gphi *phi = NULL;
-  if (gimple_seq_singleton_p (seq))
-    {
-      phi = as_a <gphi *> (gsi_stmt (gsi_start (seq)));
-      /* Never return virtual phis.  */
-      if (virtual_operand_p (gimple_phi_result (phi)))
-	return NULL;
-      return phi;
-    }
   for (i = gsi_start (seq); !gsi_end_p (i); gsi_next (&i))
     {
       gphi *p = as_a <gphi *> (gsi_stmt (i));
-- 
2.43.0

[PATCH 2/2] PHI-OPT: speed up value_replacement slightly

[Andrew Pinski]

This adds a few early outs to value_replacement that I noticed
while rewriting this to use match-and-simplify but could be committed
seperately.
* virtual operands won't change so return early for them
* special case `A ? B : B` as that is already just `B`

Also moves the check for NE/EQ earlier as calculating empty_or_with_defined_p
is an IR walk for a BB and that might be big.

Bootstrapped and tested on x86_64-linux-gnu with no regressions.

gcc/ChangeLog:

	* tree-ssa-phiopt.cc (value_replacement): Move check for
	NE/EQ earlier.

Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>
---
 gcc/tree-ssa-phiopt.cc | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc
index f1e07502b02..a2bdcb5eae8 100644
--- a/gcc/tree-ssa-phiopt.cc
+++ b/gcc/tree-ssa-phiopt.cc
@@ -1131,6 +1131,21 @@ value_replacement (basic_block cond_bb, basic_block middle_bb,
   enum tree_code code;
   bool empty_or_with_defined_p = true;
 
+  /* Virtual operands don't need to be handled. */
+  if (virtual_operand_p (arg1))
+    return 0;
+
+  /* Special case A ? B : B as this will always simplify to B. */
+  if (operand_equal_for_phi_arg_p (arg0, arg1))
+    return 0;
+
+  gcond *cond = as_a <gcond *> (*gsi_last_bb (cond_bb));
+  code = gimple_cond_code (cond);
+
+  /* This transformation is only valid for equality comparisons.  */
+  if (code != NE_EXPR && code != EQ_EXPR)
+    return 0;
+
   /* If the type says honor signed zeros we cannot do this
      optimization.  */
   if (HONOR_SIGNED_ZEROS (arg1))
@@ -1161,13 +1176,6 @@ value_replacement (basic_block cond_bb, basic_block middle_bb,
 	empty_or_with_defined_p = false;
     }
 
-  gcond *cond = as_a <gcond *> (*gsi_last_bb (cond_bb));
-  code = gimple_cond_code (cond);
-
-  /* This transformation is only valid for equality comparisons.  */
-  if (code != NE_EXPR && code != EQ_EXPR)
-    return 0;
-
   /* We need to know which is the true edge and which is the false
       edge so that we know if have abs or negative abs.  */
   extract_true_false_edges_from_block (cond_bb, &true_edge, &false_edge);
-- 
2.43.0

Re: [PATCH 1/2] MATCH: change single_non_singleton_phi_for_edges for singleton phis

[Richard Biener]

On Sun, Apr 28, 2024 at 8:31 AM Andrew Pinski <quic_apinski@quicinc.com> wrote:
>
> I noticed that single_non_singleton_phi_for_edges could
> return a phi whos entry are all the same for the edge.
> This happens only if there was a single phis in the first place.
> Also gimple_seq_singleton_p walks the sequence to see if it the one
> element in the sequence so there is removing that check actually
> reduces the number of pointer walks needed.
>
> Bootstrapped and tested on x86_64-linux-gnu with no regressions.

OK.

Richard.

> gcc/ChangeLog:
>
>         * tree-ssa-phiopt.cc (single_non_singleton_phi_for_edges):
>         Remove the special case of gimple_seq_singleton_p.
>
> Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>
> ---
>  gcc/tree-ssa-phiopt.cc | 8 --------
>  1 file changed, 8 deletions(-)
>
> diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc
> index d1746c4b468..f1e07502b02 100644
> --- a/gcc/tree-ssa-phiopt.cc
> +++ b/gcc/tree-ssa-phiopt.cc
> @@ -62,14 +62,6 @@ single_non_singleton_phi_for_edges (gimple_seq seq, edge e0, edge e1)
>  {
>    gimple_stmt_iterator i;
>    gphi *phi = NULL;
> -  if (gimple_seq_singleton_p (seq))
> -    {
> -      phi = as_a <gphi *> (gsi_stmt (gsi_start (seq)));
> -      /* Never return virtual phis.  */
> -      if (virtual_operand_p (gimple_phi_result (phi)))
> -       return NULL;
> -      return phi;
> -    }
>    for (i = gsi_start (seq); !gsi_end_p (i); gsi_next (&i))
>      {
>        gphi *p = as_a <gphi *> (gsi_stmt (i));
> --
> 2.43.0
>

Re: [PATCH 2/2] PHI-OPT: speed up value_replacement slightly

[Richard Biener]

On Sun, Apr 28, 2024 at 8:31 AM Andrew Pinski <quic_apinski@quicinc.com> wrote:
>
> This adds a few early outs to value_replacement that I noticed
> while rewriting this to use match-and-simplify but could be committed
> seperately.
> * virtual operands won't change so return early for them
> * special case `A ? B : B` as that is already just `B`
>
> Also moves the check for NE/EQ earlier as calculating empty_or_with_defined_p
> is an IR walk for a BB and that might be big.
>
> Bootstrapped and tested on x86_64-linux-gnu with no regressions.

OK.

> gcc/ChangeLog:
>
>         * tree-ssa-phiopt.cc (value_replacement): Move check for
>         NE/EQ earlier.
>
> Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>
> ---
>  gcc/tree-ssa-phiopt.cc | 22 +++++++++++++++-------
>  1 file changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc
> index f1e07502b02..a2bdcb5eae8 100644
> --- a/gcc/tree-ssa-phiopt.cc
> +++ b/gcc/tree-ssa-phiopt.cc
> @@ -1131,6 +1131,21 @@ value_replacement (basic_block cond_bb, basic_block middle_bb,
>    enum tree_code code;
>    bool empty_or_with_defined_p = true;
>
> +  /* Virtual operands don't need to be handled. */
> +  if (virtual_operand_p (arg1))
> +    return 0;
> +
> +  /* Special case A ? B : B as this will always simplify to B. */
> +  if (operand_equal_for_phi_arg_p (arg0, arg1))
> +    return 0;
> +
> +  gcond *cond = as_a <gcond *> (*gsi_last_bb (cond_bb));
> +  code = gimple_cond_code (cond);
> +
> +  /* This transformation is only valid for equality comparisons.  */
> +  if (code != NE_EXPR && code != EQ_EXPR)
> +    return 0;
> +
>    /* If the type says honor signed zeros we cannot do this
>       optimization.  */
>    if (HONOR_SIGNED_ZEROS (arg1))
> @@ -1161,13 +1176,6 @@ value_replacement (basic_block cond_bb, basic_block middle_bb,
>         empty_or_with_defined_p = false;
>      }
>
> -  gcond *cond = as_a <gcond *> (*gsi_last_bb (cond_bb));
> -  code = gimple_cond_code (cond);
> -
> -  /* This transformation is only valid for equality comparisons.  */
> -  if (code != NE_EXPR && code != EQ_EXPR)
> -    return 0;
> -
>    /* We need to know which is the true edge and which is the false
>        edge so that we know if have abs or negative abs.  */
>    extract_true_false_edges_from_block (cond_bb, &true_edge, &false_edge);
> --
> 2.43.0
>