On 10/05/2015 03:02 AM, Richard Biener wrote:
On Fri, Oct 2, 2015 at 9:30 PM, Jeff Law <l...@redhat.com> wrote:
On 10/02/2015 05:15 AM, Renlin Li wrote:

Hi Jeff,

Your patch causes an ICE regression.
The test case is " gcc.c-torture/compile/pr27087.c", I observed it on
aarch64-none-elf target when compiling the test case with '-Os' flag.

A quick check shows, the cfg has been changed, but the loop information
is not updated. Thus the information about the number of basic block in
a loop is not reliable.

Could you please have a look?

As I mentioned, when we collapse a conditional inside a loop, we may change
the # of nodes in a loop which edges are exit edges and possibly other
stuff.  So we need to mark loops as needing fixups.

Verified this fixes the aarch64-elf regression and did a bootstrap &
regression test on x86_64-linux-gnu.

Installed on the trunk.

jeff

commit 992d281b2d1ba53a49198db44fee92a505e16f5d
Author: Jeff Law <l...@tor.usersys.redhat.com>
Date:   Fri Oct 2 15:22:04 2015 -0400

     Re: [PATCH] Improve DOM's optimization of control statements

         * tree-ssa-dom.c (optimize_stmt): Note when loop structures need
         fixups.

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 3f7561a..e541df3 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,8 @@
+2015-10-02  Jeff Law  <l...@redhat.com>
+
+       * tree-ssa-dom.c (optimize_stmt): Note when loop structures need
+       fixups.
+
  2015-10-02  Uros Bizjak  <ubiz...@gmail.com>

         * system.h (ROUND_UP): New macro definition.
diff --git a/gcc/tree-ssa-dom.c b/gcc/tree-ssa-dom.c
index a8b7038..d940816 100644
--- a/gcc/tree-ssa-dom.c
+++ b/gcc/tree-ssa-dom.c
@@ -1843,6 +1843,12 @@ optimize_stmt (basic_block bb, gimple_stmt_iterator
si,
               /* Delete threads that start at BB.  */
               remove_jump_threads_starting_at (bb);

+             /* If BB is in a loop, then removing an outgoing edge from BB
+                may cause BB to move outside the loop, changes in the
+                loop exit edges, etc.  So note that loops need fixing.  */
+             if (bb_loop_depth (bb) > 0)
+               loops_state_set (LOOPS_NEED_FIXUP);
+

I would rather do this in remove_ctrl_stmt_and_useless_edges and only
if taken_edge is a loop exit.  loop fixup is a pretty big hammer which
we should avoid at all cost.

So please try to be more specific on the cases you invoke it.
What's probably the most interesting is we don't actually have EDGE_LOOP_EXIT set in DOM. So we can't use that, but that also implies it doesn't need updating. Thankfully, there's an alternate test we can use instead.

And after more ponderings, I'm pretty sure the only case that's of concern right now is nodes moving out of the loop, which only happens when we have a BB with a loop exit edge and we delete the *other* edges. We've got simple latches & preheaders and I don't think we destroy that property. Both properties also simplify the things we need look for.

So I've minimized use of the hammer to just the case where blocks are going to be moving out of the loop.

Bootstrapped and regression tested on x86_64-unknown-linux-gnu and verified the ARM test which originally spurred this change continues to work.

Installed on the trunk.

Jeff
        * tree-ssa-dom.c (optimize_stmt): Don't set LOOPS_NEED_FIXUP here.
        * tree-ssa-threadupdate.c (remove_ctrl_stmt_and_useless_edges): Do it
        here instead.  Tighten test to avoid setting LOOPS_NEED_FIXUP 
        unnecessarily.

diff --git a/gcc/tree-ssa-dom.c b/gcc/tree-ssa-dom.c
index 941087d..38cceff 100644
--- a/gcc/tree-ssa-dom.c
+++ b/gcc/tree-ssa-dom.c
@@ -1848,12 +1848,6 @@ optimize_stmt (basic_block bb, gimple_stmt_iterator si,
              FOR_EACH_EDGE (e, ei, bb->succs)
                remove_jump_threads_including (e);
 
-             /* If BB is in a loop, then removing an outgoing edge from BB
-                may cause BB to move outside the loop, changes in the
-                loop exit edges, etc.  So note that loops need fixing.  */
-             if (bb_loop_depth (bb) > 0)
-               loops_state_set (LOOPS_NEED_FIXUP);
-
              /* Now clean up the control statement at the end of
                 BB and remove unexecutable edges.  */
              remove_ctrl_stmt_and_useless_edges (bb, taken_edge->dest);
diff --git a/gcc/tree-ssa-threadupdate.c b/gcc/tree-ssa-threadupdate.c
index 26b199b..e426c1d 100644
--- a/gcc/tree-ssa-threadupdate.c
+++ b/gcc/tree-ssa-threadupdate.c
@@ -300,6 +300,17 @@ remove_ctrl_stmt_and_useless_edges (basic_block bb, 
basic_block dest_bb)
       else
        ei_next (&ei);
     }
+
+  /* If the remaining edge is a loop exit, there must have
+     a removed edge that was not a loop exit.
+
+     In that case BB and possibly other blocks were previously
+     in the loop, but are now outside the loop.  Thus, we need
+     to update the loop structures.  */
+  if (single_succ_p (bb)
+      && loop_outer (bb->loop_father)
+      && loop_exit_edge_p (bb->loop_father, single_succ_edge (bb)))
+    loops_state_set (LOOPS_NEED_FIXUP);
 }
 
 /* Create a duplicate of BB.  Record the duplicate block in an array

Reply via email to