phiprop can sometimes prop loads back into loops and in some cases cause wrong code when the load was from a weak symbol as now it becomes an unconditional load before the loop.
Bootstrapped and tested on x86_64-linux-gnu with no regressions. PR tree-optimization/116835 gcc/ChangeLog: * tree-ssa-phiprop.cc (propagate_with_phi): Check the use is at the same or deeper loop depth than the phi node. (pass_phiprop::execute): Initialize loops. gcc/testsuite/ChangeLog: * gcc.dg/torture/pr116835.c: New test. Signed-off-by: Andrew Pinski <quic_apin...@quicinc.com> --- gcc/testsuite/gcc.dg/torture/pr116835.c | 33 +++++++++++++++++++++++++ gcc/tree-ssa-phiprop.cc | 14 ++++++++++- 2 files changed, 46 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gcc.dg/torture/pr116835.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/tree-ssa-phiprop.cc b/gcc/tree-ssa-phiprop.cc index a2e1fb16a30..c5fe231d3e9 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,11 +349,17 @@ propagate_with_phi (basic_block bb, gphi *phi, struct phiprop_d *phivn, calculate_dominance_info (CDI_POST_DOMINATORS); /* Only replace loads in blocks that post-dominate the PHI node. That - makes sure we don't end up speculating loads. */ + makes sure we don't end up speculating loads. */ if (!dominated_by_p (CDI_POST_DOMINATORS, bb, gimple_bb (use_stmt))) continue; + /* Only replace loads in blocks are in the same loop + are inside an deeper loop. This is to make sure not + to prop back into the loop. */ + if (bb_loop_depth (gimple_bb (use_stmt)) < bb_loop_depth (bb)) + continue; + /* Check whether this is a load of *ptr. */ if (!(is_gimple_assign (use_stmt) && gimple_assign_rhs_code (use_stmt) == MEM_REF @@ -520,6 +527,10 @@ pass_phiprop::execute (function *fun) size_t n; auto_bitmap dce_ssa_names; + /* Find the loops, so that we can prevent moving loads in + them. */ + loop_optimizer_init (AVOID_CFG_MODIFICATIONS); + calculate_dominance_info (CDI_DOMINATORS); n = num_ssa_names; @@ -548,6 +559,7 @@ pass_phiprop::execute (function *fun) free (phivn); free_dominance_info (CDI_POST_DOMINATORS); + loop_optimizer_finalize (); return did_something ? TODO_update_ssa_only_virtuals : 0; } -- 2.43.0