[Bug middle-end/67005] [5/6 Regression] ICE: in verify_loop_structure, at cfgloop.c:1647 (loop with header n not in loop tree)

[Bug middle-end/67005] [5/6 Regression] ICE: in verify_loop_structure, at cfgloop.c:1647 (loop with header n not in loop tree)

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2015-07-27
      Known to work|                            |4.9.3
           Assignee|unassigned at gcc dot gnu.org      |rguenth at gcc dot gnu.org
   Target Milestone|---                         |5.3
            Summary|ICE: in                     |[5/6 Regression] ICE: in
                   |verify_loop_structure, at   |verify_loop_structure, at
                   |cfgloop.c:1647 (loop with   |cfgloop.c:1647 (loop with
                   |header n not in loop tree)  |header n not in loop tree)
     Ever confirmed|0                           |1
      Known to fail|                            |5.2.0, 6.0

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
Confirmed.  The loop "appears" after CDDCE1 where a loop with multiple
entries becomes one with a single entry.  DCE expects cleanup-cfg to fix
things but that doesn't do anything and thus doesn't end up fixing up loops.

Mine.

[Bug middle-end/67005] [5/6 Regression] ICE: in verify_loop_structure, at cfgloop.c:1647 (loop with header n not in loop tree)

[mpolacek at gcc dot gnu.org]

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

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mpolacek at gcc dot gnu.org

--- Comment #2 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
Still ICEs; probably it's just about adding loops_state_set (LOOPS_NEED_FIXUP);
somewhere into tree-ssa-dce.c?

[Bug middle-end/67005] [5/6 Regression] ICE: in verify_loop_structure, at cfgloop.c:1647 (loop with header n not in loop tree)

[rguenth at gcc dot gnu.org]

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

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
Yeah, though "somewhere" would be all the time if we and up removing an edge.
I've meant to see whether we can restrict it to "removed an edge with
IRREDUCIBLE_LOOP flag set, but then we'd have to compute that first and we
only do that for 'aggressive' aka CDDCE.

Didn't get around to play with that ;)  (note the ICE appears after CDDCE
only, but I would need to convince myself that regular DCE can't trigger
the same issue - I think it can't)

[Bug middle-end/67005] [5/6 Regression] ICE: in verify_loop_structure, at cfgloop.c:1647 (loop with header n not in loop tree)

[rguenth at gcc dot gnu.org]

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

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
Note we already do

            /* If we made a BB unconditionally exit a loop then this
               transform alters the set of BBs in the loop.  Schedule
               a fixup.  */
            if (loop_exit_edge_p (bb->loop_father, e))
              loops_state_set (LOOPS_NEED_FIXUP);
            remove_edge (e2);

thus this would need to add sth like || e2->flags & IRREDUCIBLE_LOOP

[Bug middle-end/67005] [5/6 Regression] ICE: in verify_loop_structure, at cfgloop.c:1647 (loop with header n not in loop tree)

[mpolacek at gcc dot gnu.org]

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

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|rguenth at gcc dot gnu.org         |mpolacek at gcc dot gnu.org

--- Comment #5 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
Taking this over for now.

[Bug middle-end/67005] [5/6 Regression] ICE: in verify_loop_structure, at cfgloop.c:1647 (loop with header n not in loop tree)

[mpolacek at gcc dot gnu.org]

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

--- Comment #6 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
Hm, adding || (e2->flags & EDGE_IRREDUCIBLE_LOOP) doesn't work; the E2 edge
doesn't have the EDGE_IRREDUCIBLE_LOOP flag, even if I recompute that flag via
mark_irreducible_loops.  So maybe set LOOPS_NEED_FIXUP every time we remove an
edge?

FTR, the CFG looks like

f ()
{
  int a.0_4;
  int a.0_7;

  <bb 2>:
  a.0_4 = a;
  if (a.0_4 == 0)
    goto <bb 3> (lbl);
  else
    goto <bb 4>;

lbl:

  <bb 4>:
  a.0_7 = a;
  if (a.0_7 != 0)
    goto <bb 5>;
  else
    goto <bb 3> (lbl);

  <bb 5>:
  a = 8;
  goto <bb 3> (lbl);

}

so the 2 -> 4 and 2 -> 3 edges are multiple entries of a loop -- why aren't
they EDGE_IRREDUCIBLE_LOOP?

[Bug middle-end/67005] [5/6 Regression] ICE: in verify_loop_structure, at cfgloop.c:1647 (loop with header n not in loop tree)

[mpolacek at gcc dot gnu.org]

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

--- Comment #7 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
(In reply to Marek Polacek from comment #6)
> Hm, adding || (e2->flags & EDGE_IRREDUCIBLE_LOOP) doesn't work; the E2 edge
> doesn't have the EDGE_IRREDUCIBLE_LOOP flag, even if I recompute that flag
> via mark_irreducible_loops.  So maybe set LOOPS_NEED_FIXUP every time we
> remove an edge?

I did some measurements on a GCC regtest.  The edge removal was triggered
~13000 times, but for only ~4000 out of that was "loop_exit_edge_p
(bb->loop_father, e)" true -- setting LOOPS_NEED_FIXUP unconditionally would be
maybe too expensive thus.

[Bug middle-end/67005] [5/6 Regression] ICE: in verify_loop_structure, at cfgloop.c:1647 (loop with header n not in loop tree)

[mpolacek at gcc dot gnu.org]

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

--- Comment #8 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
Author: mpolacek
Date: Thu Aug 27 17:07:35 2015
New Revision: 227268

URL: https://gcc.gnu.org/viewcvs?rev=227268&root=gcc&view=rev
Log:
        PR middle-end/67005
        * tree-ssa-dce.c (remove_dead_stmt): Also schedule fixup if removing
        an entry into an irreducible region.

        * gcc.dg/torture/pr67005.c: New test.

Added:
    trunk/gcc/testsuite/gcc.dg/torture/pr67005.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-ssa-dce.c

[Bug middle-end/67005] [5/6 Regression] ICE: in verify_loop_structure, at cfgloop.c:1647 (loop with header n not in loop tree)

[mpolacek at gcc dot gnu.org]

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

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #9 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
Fixed.

[Bug middle-end/67005] [5/6 Regression] ICE: in verify_loop_structure, at cfgloop.c:1647 (loop with header n not in loop tree)

[rguenth at gcc dot gnu.org]

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

--- Comment #10 from Richard Biener <rguenth at gcc dot gnu.org> ---
Can you please backport to GCC 5 as well?

[Bug middle-end/67005] [5/6 Regression] ICE: in verify_loop_structure, at cfgloop.c:1647 (loop with header n not in loop tree)

[mpolacek at gcc dot gnu.org]

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

--- Comment #11 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #10)
> Can you please backport to GCC 5 as well?

Sure.

[Bug middle-end/67005] [5/6 Regression] ICE: in verify_loop_structure, at cfgloop.c:1647 (loop with header n not in loop tree)

[mpolacek at gcc dot gnu.org]

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

--- Comment #12 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
Author: mpolacek
Date: Mon Aug 31 12:46:14 2015
New Revision: 227340

URL: https://gcc.gnu.org/viewcvs?rev=227340&root=gcc&view=rev
Log:
        PR middle-end/67005
        * tree-ssa-dce.c (remove_dead_stmt): Also schedule fixup if removing
        an entry into an irreducible region.

        * gcc.dg/torture/pr67005.c: New test.

Added:
    branches/gcc-5-branch/gcc/testsuite/gcc.dg/torture/pr67005.c
Modified:
    branches/gcc-5-branch/gcc/ChangeLog
    branches/gcc-5-branch/gcc/testsuite/ChangeLog
    branches/gcc-5-branch/gcc/tree-ssa-dce.c