On Tue, Sep 24, 2024 at 6:15 PM Andrew Pinski <quic_apin...@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_apin...@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
>
