phiprop: Avoid proping loads into loops [PR116835]

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.

	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 <andrew.pinski@oss.qualcomm.com>
Co-authored-by: Richard Biener <rguenther@suse.de>

diff --git a/gcc/testsuite/gcc.dg/torture/pr116835.c b/gcc/testsuite/gcc.dg/torture/pr116835.c
new file mode 100644 (file)
index 0000000..31d3b59
--- /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 (file)
index 0000000..8561cd1
--- /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 (file)
index 0000000..ebce03e
--- /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 a2e039fb72d9ac48ec1096ba861549dc7bc915bd..d24613d5893821324320c38877777054728da2cb 100644 (file)
--- 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