On Sat, Oct 12, 2024 at 3:42 AM Andrew Pinski <quic_apin...@quicinc.com> wrote:
>
> Clobbers in a condition don't cause any improvements and are getting
> in the way of doing match and simplify with phiopt. Need to ignore/skip
> over them in when seeing if there is only one statement that can moved.
> Also since clobbers have vops, skipping over the vops phi node is needed
> to be done.
But you can't necessarily move things across a clobber, do all users
of those functions then remove the clobber?
> Bootstrapped and tested on x86_64-linux-gnu.
>
> PR tree-optimization/117096
>
> gcc/ChangeLog:
>
> * tree-ssa-phiopt.cc (single_non_singleton_phi_for_edges): Skip
> over vops rather than return false.
> (empty_bb_or_one_feeding_into_p): Skip over clobbers too.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/tree-ssa/phiopt-2.C: New test.
>
> Signed-off-by: Andrew Pinski <quic_apin...@quicinc.com>
> ---
> gcc/testsuite/g++.dg/tree-ssa/phiopt-2.C | 46 ++++++++++++++++++++++++
> gcc/tree-ssa-phiopt.cc | 14 ++++----
> 2 files changed, 54 insertions(+), 6 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/tree-ssa/phiopt-2.C
>
> diff --git a/gcc/testsuite/g++.dg/tree-ssa/phiopt-2.C
> b/gcc/testsuite/g++.dg/tree-ssa/phiopt-2.C
> new file mode 100644
> index 00000000000..9bc8fb6e8dc
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/tree-ssa/phiopt-2.C
> @@ -0,0 +1,46 @@
> +// { dg-do compile }
> +// { dg-options "-O1 -fdump-tree-phiopt1-details" }
> +// PR tree-optimization/117096
> +// Clobbers should NOT make a difference
> +// when it comes to phiopt
> +
> +struct s1{
> + unsigned b;
> +
> + // Declare as always inline just in some someone turns down the inlining
> limits
> + // we also want to inline early.
> + __attribute__((always_inline)) inline
> + s1() : b(0) {}
> +};
> +void g();
> +int f(signed a, int c)
> +{
> + signed b = 0;
> + if (a < 0)
> + {
> + s1();
> + b = a;
> + }
> + else { s1(); }
> + g();
> + return b;
> +}
> +
> +
> +int f1(signed a, int c)
> +{
> + signed b = 0;
> + if (a < 0)
> + {
> + s1 t;
> + b = a;
> + }
> + else { s1 t; }
> + g();
> + return b;
> +}
> +
> +/* both if should be converted into MIN_EXPR */
> +/* { dg-final { scan-tree-dump-not "if " "phiopt1" } }*/
> +/* { dg-final { scan-tree-dump-times "MIN_EXPR " 2 "phiopt1" } }*/
> +
> diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc
> index f3ee3a80c0f..b22086f4de8 100644
> --- a/gcc/tree-ssa-phiopt.cc
> +++ b/gcc/tree-ssa-phiopt.cc
> @@ -72,9 +72,10 @@ single_non_singleton_phi_for_edges (gimple_seq seq, edge
> e0, edge e1)
> gimple_phi_arg_def (p, e1->dest_idx)))
> continue;
>
> - /* Punt on virtual phis with different arguments from the edges. */
> + /* Skip virtual phis with different arguments
> + from the edges as clobbers might cause the vop to be different. */
> if (virtual_operand_p (gimple_phi_result (p)))
> - return NULL;
> + continue;
>
> /* If we already have a PHI that has the two edge arguments are
> different, then return it is not a singleton for these PHIs. */
> @@ -663,9 +664,10 @@ empty_bb_or_one_feeding_into_p (basic_block bb,
> {
> gimple *s = gsi_stmt (gsi);
> gsi_next_nondebug (&gsi);
> - /* Skip over Predict and nop statements. */
> + /* Skip over Predict, clobber and nop statements. */
> if (gimple_code (s) == GIMPLE_PREDICT
> - || gimple_code (s) == GIMPLE_NOP)
> + || gimple_code (s) == GIMPLE_NOP
> + || gimple_clobber_p (s))
> continue;
> /* If there is more one statement return false. */
> if (stmt_to_move)
> @@ -673,8 +675,8 @@ empty_bb_or_one_feeding_into_p (basic_block bb,
> stmt_to_move = s;
> }
>
> - /* The only statement here was a Predict or a nop statement
> - so return true. */
> + /* The only statement here was a Predict, nop,
> + or a clobber statement so return true. */
> if (!stmt_to_move)
> return true;
>
> --
> 2.43.0
>
