On Sat, Sep 14, 2024 at 1:02 AM Andrew Pinski <quic_apin...@quicinc.com> wrote:
>
> The heuristics for factoring out with a constant checks that the assignment
> statement
> is the last statement of the basic block but sometimes there is a predicate
> or a nop statement
> after the assignment. Rejecting this case does not make sense since both
> predicates and nop
> statements are removed and don't contribute any instructions. So we should
> skip over them
> when checking if the assignment statement was the last statement in the basic
> block.
>
> phi-opt-factor-1.c's f0 is such an example where it should catch it at
> phiopt1 (before predicates are removed)
> and should happen in a similar way as f1 (which uses a temporary variable
> rather than return).
OK
> Bootstrapped and tested on x86_64-linux-gnu.
>
> PR tree-optimization/116699
>
> gcc/ChangeLog:
>
> * tree-ssa-phiopt.cc (factor_out_conditional_operation): Skip over
> nop/predicates
> for seeing the assignment is the last statement.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.dg/tree-ssa/phi-opt-factor-1.c: New test.
>
> Signed-off-by: Andrew Pinski <quic_apin...@quicinc.com>
> ---
> .../gcc.dg/tree-ssa/phi-opt-factor-1.c | 26 +++++++++++++++++++
> gcc/tree-ssa-phiopt.cc | 6 +++++
> 2 files changed, 32 insertions(+)
> create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phi-opt-factor-1.c
>
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-factor-1.c
> b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-factor-1.c
> new file mode 100644
> index 00000000000..12b846b9337
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-factor-1.c
> @@ -0,0 +1,26 @@
> +/* { dg-options "-O2 -fdump-tree-phiopt" } */
> +
> +/* PR tree-optimization/116699
> + Make sure the return PREDICT has no factor in deciding
> + if we factor out the conversion. */
> +
> +short f0(int a, int b, int c)
> +{
> + int t1 = 4;
> + if (c < t1) return (c > -1 ? c : -1);
> + return t1;
> +}
> +
> +
> +short f1(int a, int b, int c)
> +{
> + int t1 = 4;
> + short t = t1;
> + if (c < t1) t = (c > -1 ? c : -1);
> + return t;
> +}
> +
> +/* Both f1 and f0 should be optimized at phiopt1 to the same thing. */
> +/* { dg-final { scan-tree-dump-times "MAX_EXPR " 2 "phiopt1" } } */
> +/* { dg-final { scan-tree-dump-times "MIN_EXPR " 2 "phiopt1" } } */
> +/* { dg-final { scan-tree-dump-not "if " "phiopt1" } } */
> diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc
> index e5413e40572..7b12692237e 100644
> --- a/gcc/tree-ssa-phiopt.cc
> +++ b/gcc/tree-ssa-phiopt.cc
> @@ -360,6 +360,12 @@ factor_out_conditional_operation (edge e0, edge e1, gphi
> *phi,
> }
> gsi = gsi_for_stmt (arg0_def_stmt);
> gsi_next_nondebug (&gsi);
> + /* Skip past nops and predicates. */
> + while (!gsi_end_p (gsi)
> + && (gimple_code (gsi_stmt (gsi)) == GIMPLE_NOP
> + || gimple_code (gsi_stmt (gsi)) == GIMPLE_PREDICT))
> + gsi_next_nondebug (&gsi);
> + /* Reject if the statement was not at the end of the block. */
> if (!gsi_end_p (gsi))
> return NULL;
> }
> --
> 2.43.0
>
