[Bug tree-optimization/84646] Missed optimisation for hoisting conditions outside nested loops

["rguenth at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84646

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
It's

edge
back_threader::maybe_register_path (back_threader_profitability &profit)
{
  edge taken_edge = find_taken_edge (m_path);

  if (taken_edge && taken_edge != UNREACHABLE_EDGE)
    {
      if (m_visited_bbs.contains (taken_edge->dest))
        {
          // Avoid circular paths by indicating there is nothing to
          // see in this direction.
          taken_edge = UNREACHABLE_EDGE;

not sure why though?  If we remove the above we get

Checking profitability of path (backwards):  bb:3 (2 insns) bb:7 (4 insns) bb:6
(1 insns) (latch) bb:5
  Control statement insns: 2
  Overall: 5 insns

 Registering value_relation (path_oracle) (j_17 < m_23(D)) (root: bb5)
Checking profitability of path (backwards):
Path crosses loop header but does not exit it:   Cancelling jump thread: (5, 6)
incoming edge;  (6, 7) normal (back) (7, 3) normal (3, 6) nocopy;

path: 5->6->7->3->xx REJECTED

so we refuse the thread because .. well, not exactly sure why.
r12-4526-gd8edfadfc7a979 added

+  if (crossed_loop_header)
+    {
+      cancel_thread (&path, "Path crosses loop header but does not exit it");
+      return true;
+    }

From the look we want to avoid creating sub-loops here and that's indeed
what would happen here if we elide this.  It also makes the loop have two
exits instead of one and one jump thread is still not done but we eventually
get to do the desired simplification.

diff --git a/gcc/tree-ssa-threadbackward.cc b/gcc/tree-ssa-threadbackward.cc
index 2a8cfa3ee01..31c3eef9bb3 100644
--- a/gcc/tree-ssa-threadbackward.cc
+++ b/gcc/tree-ssa-threadbackward.cc
@@ -249,7 +249,7 @@ back_threader::maybe_register_path
(back_threader_profitabil
ity &profit)

   if (taken_edge && taken_edge != UNREACHABLE_EDGE)
     {
-      if (m_visited_bbs.contains (taken_edge->dest))
+      if (0 && m_visited_bbs.contains (taken_edge->dest))
        {
          // Avoid circular paths by indicating there is nothing to
          // see in this direction.
diff --git a/gcc/tree-ssa-threadupdate.cc b/gcc/tree-ssa-threadupdate.cc
index 59c268a3567..14df3ee42a7 100644
--- a/gcc/tree-ssa-threadupdate.cc
+++ b/gcc/tree-ssa-threadupdate.cc
@@ -2765,7 +2765,6 @@ jt_path_registry::cancel_invalid_paths
(vec<jump_thread_ed
ge *> &path)
   bool seen_latch = false;
   int loops_crossed = 0;
   bool crossed_latch = false;
-  bool crossed_loop_header = false;
   // Use ->dest here instead of ->src to ignore the first block.  The
   // first block is allowed to be in a different loop, since it'll be
   // redirected.  See similar comment in profitable_path_p: "we don't
@@ -2799,14 +2798,6 @@ jt_path_registry::cancel_invalid_paths
(vec<jump_thread_edge *> &path)
          ++loops_crossed;
        }

-      // ?? Avoid threading through loop headers that remain in the
-      // loop, as such threadings tend to create sub-loops which
-      // _might_ be OK ??.
-      if (e->dest->loop_father->header == e->dest
-         && !flow_loop_nested_p (exit->dest->loop_father,
-                                 e->dest->loop_father))
-       crossed_loop_header = true;
-
       if (flag_checking && !m_backedge_threads)
        gcc_assert ((path[i]->e->flags & EDGE_DFS_BACK) == 0);
     }
@@ -2842,11 +2833,6 @@ jt_path_registry::cancel_invalid_paths
(vec<jump_thread_edge *> &path)
       cancel_thread (&path, "Path rotates loop");
       return true;
     }
-  if (crossed_loop_header)
-    {
-      cancel_thread (&path, "Path crosses loop header but does not exit it");
-      return true;
-    }
   return false;
 }