On Fri, Nov 21, 2025 at 7:57 PM Andrew Pinski <[email protected]> wrote: > > This is v2 which uses Richi's code. > This amends "the post-dominance check to deal with > SSA cycles [...]. We need to constrain the load to > be in the same or a subloop (use > flow_loop_nested_p, not loop depth) or in the same > BB when either the load or the PHI is in an irreducible region." > (as described by Richard). > > Bootstrapped and tested on x86_64-linux-gnu.
OK. Richard. > PR tree-optimization/116835 > > gcc/ChangeLog: > > * tree-ssa-phiprop.cc (propagate_with_phi): Admend the > post-dom check to deal with ssa cycles. > > gcc/testsuite/ChangeLog: > > * gcc.dg/torture/pr116835.c: New test. > * gcc.dg/tree-ssa/phiprop-6.c: New test. > * gcc.dg/tree-ssa/phiprop-7.c: New test. > > Signed-off-by: Andrew Pinski <[email protected]> > Co-authored-by: Richard Biener <[email protected]> > --- > gcc/testsuite/gcc.dg/torture/pr116835.c | 33 +++++++++++++++++++++++ > gcc/testsuite/gcc.dg/tree-ssa/phiprop-6.c | 21 +++++++++++++++ > gcc/testsuite/gcc.dg/tree-ssa/phiprop-7.c | 21 +++++++++++++++ > gcc/tree-ssa-phiprop.cc | 12 +++++++++ > 4 files changed, 87 insertions(+) > create mode 100644 gcc/testsuite/gcc.dg/torture/pr116835.c > create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phiprop-6.c > create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phiprop-7.c > > diff --git a/gcc/testsuite/gcc.dg/torture/pr116835.c > b/gcc/testsuite/gcc.dg/torture/pr116835.c > new file mode 100644 > index 00000000000..31d3b59d945 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/torture/pr116835.c > @@ -0,0 +1,33 @@ > +/* { dg-do run { target { weak_undefined } } } */ > +/* { dg-add-options weak_undefined } */ > + > +/* PR tree-optimization/116835 */ > +/* phiprop would prop into the loop the load of b > + and also prop the load of a before the loop. > + Which is incorrect as a is a weak symbol. */ > + > +struct s1 > +{ > + int t; > + int t1; > +}; > +typedef struct s1 type; > +extern type a __attribute__((weak)); > +int t; > +type b; > +type bar (int c) __attribute__((noipa, noinline)); > +type > +bar (int c) > +{ > + t = 1; > + type *p = &a; > + for (int j = 0; j < c; ++j) > + p = &b; > + return *p; > +} > + > +int main(void) > +{ > + if (bar(&a == nullptr).t) > + __builtin_abort(); > +} > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phiprop-6.c > b/gcc/testsuite/gcc.dg/tree-ssa/phiprop-6.c > new file mode 100644 > index 00000000000..8561cd14480 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/tree-ssa/phiprop-6.c > @@ -0,0 +1,21 @@ > +/* { dg-do compile { target { weak_undefined } } } */ > +/* { dg-options "-O2 -fdump-tree-phiprop-details" } */ > +/* { dg-add-options weak_undefined } */ > + > +/* PR tree-optimization/116835 */ > + > +extern int a __attribute__((weak)); > +int b; > + > +int > +bar (int c) > +{ > + int *p = &a; > + for (int j = 0; j < c; ++j) > + p = &b; > + return *p; > +} > +/* The weak load is conditional with the loop so we cannot make it > unconditional. */ > +/* { dg-final { scan-tree-dump-not "Removing dead stmt:" "phiprop1"} } */ > +/* { dg-final { scan-tree-dump-not "Inserting PHI for result of load" > "phiprop1"} } */ > + > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phiprop-7.c > b/gcc/testsuite/gcc.dg/tree-ssa/phiprop-7.c > new file mode 100644 > index 00000000000..ebce03ea630 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/tree-ssa/phiprop-7.c > @@ -0,0 +1,21 @@ > +/* { dg-do compile { target { weak_undefined } } } */ > +/* { dg-options "-O2 -fdump-tree-phiprop-details" } */ > +/* { dg-add-options weak_undefined } */ > + > +/* PR tree-optimization/116835 */ > + > +extern int a __attribute__((weak)); > +int b; > + > +int > +bar (int c) > +{ > + int *p = &a; > + for (int j = 0; j < *p; ++j) > + p = &b; > + return *p; > +} > +/* The weak load is unconditional due to the conditional so we can remove it > unconditionally. */ > +/* { dg-final { scan-tree-dump "Removing dead stmt:" "phiprop1"} } */ > +/* { dg-final { scan-tree-dump "Inserting PHI for result of load" > "phiprop1"} } */ > + > diff --git a/gcc/tree-ssa-phiprop.cc b/gcc/tree-ssa-phiprop.cc > index a2e039fb72d..d24613d5893 100644 > --- a/gcc/tree-ssa-phiprop.cc > +++ b/gcc/tree-ssa-phiprop.cc > @@ -35,6 +35,7 @@ along with GCC; see the file COPYING3. If not see > #include "tree-ssa-loop.h" > #include "tree-cfg.h" > #include "tree-ssa-dce.h" > +#include "cfgloop.h" > > /* This pass propagates indirect loads through the PHI node for its > address to make the load source possibly non-addressable and to > @@ -348,6 +349,17 @@ propagate_with_phi (basic_block bb, gphi *vphi, gphi > *phi, > bb, gimple_bb (use_stmt))) > delay = true; > > + /* Amend the post-dominance check for SSA cycles, we need to > + make sure each PHI result value is dereferenced. > + We only want to delay this if we don't insert a phi. */ > + if (!(gimple_bb (use_stmt) == bb > + || (!(bb->flags & BB_IRREDUCIBLE_LOOP) > + && !(gimple_bb (use_stmt)->flags & BB_IRREDUCIBLE_LOOP) > + && (bb->loop_father == gimple_bb (use_stmt)->loop_father > + || flow_loop_nested_p (bb->loop_father, > + gimple_bb > (use_stmt)->loop_father))))) > + delay = true; > + > /* Check whether this is a load of *ptr. */ > if (!(is_gimple_assign (use_stmt) > && gimple_assign_rhs_code (use_stmt) == MEM_REF > -- > 2.43.0 >
