[PATCH 2/3] phiprop: Skip over clobbers [PR116823]

[Richard Biener]

On Tue, Sep 24, 2024 at 6:15 PM Andrew Pinski <quic_apinski@quicinc.com> wrote:
>
> In C++ code the clobber gets in the way of phiprop.
> E.g.
> ```
>   if (lr_bitpos.2401_412 < rr_bitpos.2402_413)
>     goto <bb 198>; [INV]
>   else
>     goto <bb 197>; [INV]
>
>   <bb 197> :
>
>   <bb 198> :
>   MEM[(struct poly_int *)&D.192544] ={v} {CLOBBER(bob)};
>   _1060 = MEM[(const long int &)iftmp.2400_515];
> ```
>
> The above comes from fold-const.cc. The clobber in the above case
> is the clobber from the start of the constructor but other clobbers
> can also get in the way, see gcc.dg/tree-ssa/phiprop-2.c for an example.
> This shows up in a lot of C++ code where std::min/max (or even ?: like in the
> fold-const.cc case) is used with in connection of constructors.
> So optimizing this early in phiprop can improve code generation and compile
> time speed.
>
> g++.dg/tree-ssa/phiprop-2.C contains the reduced testcase from fold-const.cc.
>
> Bootstrapped and tested on x86_64-linux-gnu.

OK.

>         PR tree-optimization/116823
>
> gcc/ChangeLog:
>
>         * tree-ssa-phiprop.cc (phiprop_insert_phi): Get
>         the use_vuse before the looping of the phi arguments,
>         also skip over clobbers to get the use_vuse.
>         (propagate_with_phi): Skip over clobbers for the vuse.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.dg/tree-ssa/phiprop-2.c: New test.
>         * g++.dg/tree-ssa/phiprop-1.C: New test.
>         * g++.dg/tree-ssa/phiprop-2.C: New test.
>
> Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>
> ---
>  gcc/testsuite/g++.dg/tree-ssa/phiprop-1.C | 23 +++++++++++++++++++
>  gcc/testsuite/g++.dg/tree-ssa/phiprop-2.C | 25 +++++++++++++++++++++
>  gcc/testsuite/gcc.dg/tree-ssa/phiprop-2.c | 27 +++++++++++++++++++++++
>  gcc/tree-ssa-phiprop.cc                   | 25 ++++++++++++++++++++-
>  4 files changed, 99 insertions(+), 1 deletion(-)
>  create mode 100644 gcc/testsuite/g++.dg/tree-ssa/phiprop-1.C
>  create mode 100644 gcc/testsuite/g++.dg/tree-ssa/phiprop-2.C
>  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phiprop-2.c
>
> diff --git a/gcc/testsuite/g++.dg/tree-ssa/phiprop-1.C b/gcc/testsuite/g++.dg/tree-ssa/phiprop-1.C
> new file mode 100644
> index 00000000000..e3388d1d157
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/tree-ssa/phiprop-1.C
> @@ -0,0 +1,23 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O1 -fdump-tree-phiprop1-details -fdump-tree-release_ssa" } */
> +
> +/* PR tree-optimization/116823 */
> +/* The clobber on a should not get in the way of phiprop here even if
> +   this is undefined code. */
> +/* We should have MIN_EXPR early on then too. */
> +
> +static inline
> +const int &c(const int &d, const int &e) {
> +  if (d < e)
> +    return d;
> +  return e;
> +}
> +
> +int g(int i, struct f *ff)
> +{
> +  const int &a = c(i, 10);
> +  return a;
> +}
> +/* { dg-final { scan-tree-dump-times "Inserting PHI for result of load" 1 "phiprop1"} } */
> +/* { dg-final { scan-tree-dump-times "MIN_EXPR" 1 "release_ssa"} } */
> +
> diff --git a/gcc/testsuite/g++.dg/tree-ssa/phiprop-2.C b/gcc/testsuite/g++.dg/tree-ssa/phiprop-2.C
> new file mode 100644
> index 00000000000..1a0d6ed92ee
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/tree-ssa/phiprop-2.C
> @@ -0,0 +1,25 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O1 -fdump-tree-phiprop1-details -fdump-tree-release_ssa" } */
> +
> +/* PR tree-optimization/116823 */
> +/* The clobber on the temp s2 should not get in the way of phiprop here. */
> +/* We should have MAX_EXPR early on then too. */
> +/* This is derived from fold-const.cc; s2 is similar to poly_int. */
> +
> +struct s2
> +{
> +  int i;
> +  s2(const int &a) : i (a) {}
> +};
> +
> +
> +int h(s2 b);
> +
> +int g(int l, int r)
> +{
> +  return h(l > r ? l : r);
> +}
> +
> +/* { dg-final { scan-tree-dump-times "Inserting PHI for result of load" 1 "phiprop1"} } */
> +/* { dg-final { scan-tree-dump-times "MAX_EXPR" 1 "release_ssa"} } */
> +
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phiprop-2.c b/gcc/testsuite/gcc.dg/tree-ssa/phiprop-2.c
> new file mode 100644
> index 00000000000..546031e63d7
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/phiprop-2.c
> @@ -0,0 +1,27 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O1 -fdump-tree-phiprop1-details -fdump-tree-release_ssa" } */
> +
> +/* PR tree-optimization/116823 */
> +/* The clobber on b should not get in the way of phiprop here. */
> +/* We should have MIN_EXPR early on. */
> +
> +void f(int *);
> +
> +int g(int i)
> +{
> +  const int t = 10;
> +  const int *a;
> +  {
> +    int b;
> +    f(&b);
> +    if (t < i)
> +      a = &t;
> +    else
> +      a = &i;
> +  }
> +  return *a;
> +}
> +
> +/* { dg-final { scan-tree-dump-times "Inserting PHI for result of load" 1 "phiprop1"} } */
> +/* { dg-final { scan-tree-dump-times "MIN_EXPR" 1 "release_ssa"} } */
> +
> diff --git a/gcc/tree-ssa-phiprop.cc b/gcc/tree-ssa-phiprop.cc
> index 2a1cdae46d2..f04990e8cb4 100644
> --- a/gcc/tree-ssa-phiprop.cc
> +++ b/gcc/tree-ssa-phiprop.cc
> @@ -159,6 +159,20 @@ phiprop_insert_phi (basic_block bb, gphi *phi, gimple *use_stmt,
>      }
>
>    gphi *vphi = get_virtual_phi (bb);
> +  tree use_vuse = gimple_vuse (use_stmt);
> +  gimple *def_stmt = SSA_NAME_DEF_STMT (use_vuse);
> +  /* Skip over clobbers in the same bb as the use
> +     as they don't interfer with loads. */
> +  while (!SSA_NAME_IS_DEFAULT_DEF (use_vuse)
> +        && gimple_clobber_p (def_stmt)
> +        && gimple_bb (def_stmt) == bb)
> +    {
> +      use_vuse = gimple_vuse (def_stmt);
> +      def_stmt = SSA_NAME_DEF_STMT (use_vuse);
> +    }
> +  /* If there is a vphi, then the def statement of the
> +     vuse has to be a phi. */
> +  gcc_assert (!vphi || is_a<gphi *>(def_stmt));
>
>    /* Add PHI arguments for each edge inserting loads of the
>       addressable operands.  */
> @@ -204,7 +218,7 @@ phiprop_insert_phi (basic_block bb, gphi *phi, gimple *use_stmt,
>               if (vphi)
>                 vuse = PHI_ARG_DEF_FROM_EDGE (vphi, e);
>               else
> -               vuse = gimple_vuse (use_stmt);
> +               vuse = use_vuse;
>             }
>           else
>             /* For the aggregate copy case updating virtual operands
> @@ -377,6 +391,15 @@ propagate_with_phi (basic_block bb, gphi *phi, struct phiprop_d *phivn,
>          def is an edge-inserted one we know it dominates us.  */
>        vuse = gimple_vuse (use_stmt);
>        def_stmt = SSA_NAME_DEF_STMT (vuse);
> +      /* Skip over clobbers in the same bb as the use
> +        as they don't interfer with loads. */
> +      while (!SSA_NAME_IS_DEFAULT_DEF (vuse)
> +            && gimple_clobber_p (def_stmt)
> +            && gimple_bb (def_stmt) == bb)
> +       {
> +         vuse = gimple_vuse (def_stmt);
> +         def_stmt = SSA_NAME_DEF_STMT (vuse);
> +       }
>        if (!SSA_NAME_IS_DEFAULT_DEF (vuse)
>           && (gimple_bb (def_stmt) == bb
>               || (gimple_bb (def_stmt)
> --
> 2.43.0
>