[PATCH] cselim: Don't create a phi node if the rhs side are the same [PR122155]

[Richard Biener]

On Sun, Oct 5, 2025 at 12:01 AM Andrew Pinski
<andrew.pinski@oss.qualcomm.com> wrote:
>
> This is a small compile time optimization where if commonalizing stores
> that have the same rhs, a phi node does not need to be created.
> This uses the same code as what was added for the `= {};` case.
> The reason why it is a compile time optimization is that Copy prop
> later on will do the same thing so not creating a new phi and a new
> ssa name will have a small compile time improvement.

OK.

I'll note that both this and sinking will eventually create PHIs that
can be CSEd.  Not sure whether it's worth implementing a simple
CSE for this here (also considering CSE to already existing PHIs)

RIchard.

> Bootstrapped and tested on x86_64-linux-gnu.
>
>         PR tree-optimization/122155
>
> gcc/ChangeLog:
>
>         * tree-ssa-phiopt.cc (cond_if_else_store_replacement_1): Don't
>         create a phi if the 2 rhs are the same.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.dg/tree-ssa/cselim-3.c: New test.
>
> Signed-off-by: Andrew Pinski <andrew.pinski@oss.qualcomm.com>
> ---
>  gcc/testsuite/gcc.dg/tree-ssa/cselim-3.c | 16 ++++++++++++++++
>  gcc/tree-ssa-phiopt.cc                   | 10 ++++------
>  2 files changed, 20 insertions(+), 6 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/cselim-3.c
>
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/cselim-3.c b/gcc/testsuite/gcc.dg/tree-ssa/cselim-3.c
> new file mode 100644
> index 00000000000..d0c8e6a9e18
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/cselim-3.c
> @@ -0,0 +1,16 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-phiopt1-details-stats" } */
> +/* PR tree-optimization/122155 */
> +
> +void f(int *a, int b, int c)
> +{
> +  if (c)
> +    *a = b;
> +  else
> +    *a = b;
> +}
> +
> +/* When commonalizing a store with the same rhs, a PHI does not need to be created. */
> +/* { dg-final { scan-tree-dump "if-then-else store replacement: 1" "phiopt1" } } */
> +/* { dg-final { scan-tree-dump-not "to use phi:" "phiopt1" } } */
> +/* { dg-final { scan-tree-dump-not "PHI <" "phiopt1" } } */
> diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc
> index 5d0c867f99a..094828ff9f1 100644
> --- a/gcc/tree-ssa-phiopt.cc
> +++ b/gcc/tree-ssa-phiopt.cc
> @@ -3643,9 +3643,8 @@ cond_if_else_store_replacement_1 (basic_block then_bb, basic_block else_bb,
>    tree lhs_base, lhs, then_rhs, else_rhs, name;
>    location_t then_locus, else_locus;
>    gimple_stmt_iterator gsi;
> -  gphi *newphi;
> +  gphi *newphi = nullptr;
>    gassign *new_stmt;
> -  bool empty_constructor = false;
>
>    if (then_assign == NULL
>        || !gimple_assign_single_p (then_assign)
> @@ -3680,7 +3679,6 @@ cond_if_else_store_replacement_1 (basic_block then_bb, basic_block else_bb,
>        /* Currently only handle commoning of `= {}`.   */
>        if (TREE_CODE (then_rhs) != CONSTRUCTOR)
>         return false;
> -      empty_constructor = true;
>      }
>
>    if (dump_file && (dump_flags & TDF_DETAILS))
> @@ -3709,8 +3707,8 @@ cond_if_else_store_replacement_1 (basic_block then_bb, basic_block else_bb,
>    /* 2) Create a PHI node at the join block, with one argument
>         holding the old RHS, and the other holding the temporary
>         where we stored the old memory contents.  */
> -  if (empty_constructor)
> -    name = unshare_expr (then_rhs);
> +  if (operand_equal_p (then_rhs, else_rhs))
> +    name = then_rhs;
>    else
>      {
>        name = make_temp_ssa_name (TREE_TYPE (lhs), NULL, "cstore");
> @@ -3730,7 +3728,7 @@ cond_if_else_store_replacement_1 (basic_block then_bb, basic_block else_bb,
>    update_stmt (vphi);
>    if (dump_file && (dump_flags & TDF_DETAILS))
>      {
> -      if (!empty_constructor)
> +      if (newphi)
>         {
>          fprintf(dump_file, "to use phi:\n");
>           print_gimple_stmt (dump_file, newphi, 0,
> --
> 2.43.0
>