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

Reply via email to