> Am 04.05.2026 um 15:52 schrieb Andrew Pinski <[email protected]>:
>
> Like the already approved patch at
> https://inbox.sourceware.org/gcc-patches/[email protected]/
> but reworked to fit into the new simplified code and
> also fixed a bug noticed for aggregates.
>
> Aggregates include a store when doing phiprop so we need to
> check if there are also loads between the original store/load
> and the clobber we are skipping. Like the skipping of the
> store case, I didn't see this happening enough to add the
> extra checks. I did add a testcase (phiprop-5.C) which checks
> this.
>
> changes since v2:
> * v3: treat aggregates special earlier and don't duplicate code.
> * v2: adapt to can_handle_load instead of inline.
Ok
Richard
> PR tree-optimization/116823
>
> gcc/ChangeLog:
>
> * tree-ssa-phiprop.cc (can_handle_load): Skip past
> clobbers for !aggregate.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/tree-ssa/phiprop-2.C: New test.
> * g++.dg/tree-ssa/phiprop-4.C: New test.
> * g++.dg/tree-ssa/phiprop-5.C: New test.
>
> Signed-off-by: Andrew Pinski <[email protected]>
> ---
> gcc/testsuite/g++.dg/tree-ssa/phiprop-2.C | 23 +++++++++++
> gcc/testsuite/g++.dg/tree-ssa/phiprop-4.C | 30 ++++++++++++++
> gcc/testsuite/g++.dg/tree-ssa/phiprop-5.C | 36 +++++++++++++++++
> gcc/tree-ssa-phiprop.cc | 49 +++++++++++++++++++----
> 4 files changed, 131 insertions(+), 7 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/tree-ssa/phiprop-2.C
> create mode 100644 gcc/testsuite/g++.dg/tree-ssa/phiprop-4.C
> create mode 100644 gcc/testsuite/g++.dg/tree-ssa/phiprop-5.C
>
> 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..e3388d1d157
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/tree-ssa/phiprop-2.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-4.C
> b/gcc/testsuite/g++.dg/tree-ssa/phiprop-4.C
> new file mode 100644
> index 00000000000..b630148faa5
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/tree-ssa/phiprop-4.C
> @@ -0,0 +1,30 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fno-tree-sra -fdump-tree-phiprop-details" } */
> +/* PR tree-optimization/123120 */
> +/* Make sure the store to *e conflicts with the store to *d */
> +
> +
> +struct s1
> +{
> + int a;
> +};
> +
> +void f(int *);
> +
> +void g(struct s1 i, struct s1 *d, struct s1 *e)
> +{
> + const struct s1 t = {10};
> + const struct s1 *a;
> + struct s1 t1 = {2};
> + if (t.a < i.a)
> + a = &t;
> + else
> + a = &i;
> + *e = t1;
> + t1 = *a;
> + *d = t1;
> +}
> +
> +/* { dg-final { scan-tree-dump-not "Inserting PHI for result of load"
> "phiprop1"} } */
> +/* { dg-final { scan-tree-dump-not "Inserting PHI for result of load"
> "phiprop2"} } */
> +
> diff --git a/gcc/testsuite/g++.dg/tree-ssa/phiprop-5.C
> b/gcc/testsuite/g++.dg/tree-ssa/phiprop-5.C
> new file mode 100644
> index 00000000000..5a3a04dfaa4
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/tree-ssa/phiprop-5.C
> @@ -0,0 +1,36 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-phiprop-details" } */
> +/* PR tree-optimization/116823 */
> +/* Make sure the store to *d conflicts with the load from d->a; */
> +
> +
> +struct s1
> +{
> + int a;
> +};
> +
> +void f(int *);
> +
> +int g(struct s1 i, struct s1 *d, struct s1 *e)
> +{
> + int t3;
> + struct s1 t1 = {2};
> + const struct s1 t = {10};
> + const struct s1 *a;
> + {
> + int t67;
> + f(&t67);
> + if (t.a < i.a)
> + a = &t;
> + else
> + a = &i;
> + }
> + t3 = d->a;
> + *d = *a;
> + return t3;
> +}
> +
> +
> +/* { dg-final { scan-tree-dump-not "Inserting PHI for result of load"
> "phiprop1"} } */
> +/* { dg-final { scan-tree-dump-not "Inserting PHI for result of load"
> "phiprop2"} } */
> +
> diff --git a/gcc/tree-ssa-phiprop.cc b/gcc/tree-ssa-phiprop.cc
> index 5985beae26a..54f3ad8d9f8 100644
> --- a/gcc/tree-ssa-phiprop.cc
> +++ b/gcc/tree-ssa-phiprop.cc
> @@ -274,22 +274,57 @@ can_handle_load (gimple *load_stmt,
> expected_vuse = gimple_phi_result (vphi);
> else if (up_vuse)
> expected_vuse = up_vuse;
> + /* Try to see if the store does not effect the load. */
> + gimple *other_store = SSA_NAME_DEF_STMT (vuse);
> + /* For aggregates, skipping the store is too
> + hard to handle as you need to check for loads
> + and it is not worth the extra checks so just handle expected vuse
> + and the dominated by case. */
> + if (aggregate)
> + {
> + /* If the vuse on the load is the same as the expected vuse,
> + there are no stores inbetween. */
> + if (vuse == expected_vuse)
> + return vuse;
> + if (expected_vuse)
> + return NULL_TREE;
> + if (gimple_bb (other_store) != bb
> + && dominated_by_p (CDI_DOMINATORS,
> + bb, gimple_bb (other_store)))
> + return vuse;
> + return NULL_TREE;
> + }
> +
> + /* Skip over clobbers in the same bb as the use
> + as they don't interfere with loads. */
> + while (!SSA_NAME_IS_DEFAULT_DEF (vuse)
> + && gimple_clobber_p (other_store)
> + && gimple_bb (other_store) == bb)
> + {
> + vuse = gimple_vuse (other_store);
> + other_store = SSA_NAME_DEF_STMT (vuse);
> + }
> + /* If the load does not have a store beforehand,
> + then we can do the load in conditional. */
> + if (SSA_NAME_IS_DEFAULT_DEF (vuse))
> + {
> + /* For loads that have no stores before, there should be no
> + vphi. */
> + gcc_checking_assert (!vphi);
> + /* The common vuse is the same as the default or there is none. */
> + gcc_checking_assert (!up_vuse || up_vuse == vuse);
> + return vuse;
> + }
>
> /* If the vuse on the load is the same as the expected vuse,
> there are no stores inbetween. */
> if (vuse == expected_vuse)
> return vuse;
> - /* Try to see if the store does not effect the load. */
> - gimple *other_store = SSA_NAME_DEF_STMT (vuse);
> +
> /* Only handling the case where the store is in the same
> bb as the phi. */
> if (gimple_bb (other_store) == bb)
> {
> - /* For aggregates, skipping the store is too
> - hard to handle as you need to check for loads
> - and it is not worth the extra checks. */
> - if (aggregate)
> - return NULL_TREE;
> tree src = gimple_assign_rhs1 (load_stmt);
> ao_ref read;
> ao_ref_init (&read, src);
> --
> 2.43.0
>
