[Bug middle-end/53695] [4.8 Regression] ICE: in dfs_enumerate_from, at cfganal.c:1221 with -O2 -ftracer and labels/gotos

["rguenther at suse dot de" <gcc-bugzilla@gcc.gnu.org>]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53695

--- Comment #13 from rguenther at suse dot de <rguenther at suse dot de> 2012-08-23 08:07:18 UTC ---
On Thu, 23 Aug 2012, rguenther at suse dot de wrote:

> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53695
> 
> --- Comment #11 from rguenther at suse dot de <rguenther at suse dot de> 2012-08-23 07:36:46 UTC ---
> On Wed, 22 Aug 2012, steven at gcc dot gnu.org wrote:
> 
> > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53695
> > 
> > --- Comment #9 from Steven Bosscher <steven at gcc dot gnu.org> 2012-08-22 21:33:18 UTC ---
> > I think the right fix for this bug is to use disambiguate_multiple_latches in
> > the loop updating code (fix_loop_structure), but I'm not sure where to put it.
> 
> Not sure - we can handle multiple latches just fine (loop->latch will
> be NULL).  But I see the loop state does not reflect that.  Maybe
> 
> Index: gcc/cfgloopmanip.c
> ===================================================================
> --- gcc/cfgloopmanip.c  (revision 190613)
> +++ gcc/cfgloopmanip.c  (working copy)
> @@ -1715,6 +1716,9 @@ fix_loop_structure (bitmap changed_bbs)
>         }
>      }
> 
> +  if (!loop_state_satisfies_p (LOOPS_MAY_HAVE_MULTIPLE_LATCHES))
> +    disambiguate_loops_with_multiple_latches ();
> +
>    if (loops_state_satisfies_p (LOOPS_HAVE_PREHEADERS))
>      create_preheaders (CP_SIMPLE_PREHEADERS);
> 
> which matches the order in which loop_optimizer_init calls it.

Doesn't fix the testcase.

We fail during verify_loop_structure and the loop state _does_ have
LOOPS_MAY_HAVE_MULTIPLE_LATCHES set.

Now, for the testcase we at this point just have a single loop
left (we don't recognize the loop with the abnormal path from
header to latch).  Btw, I'm looking at the reduced testcase in
the patch (yes, that's a slightly different situation but simpler
to analyze and fails the same way).  So what's wrong here is
indeed loop->num_nodes (we don't account the other "loop" to
loop 1 and we do not properly mark the loop as having multiple
latches).

Already the input to tracer is "wrong" in that we have "lost"
a loop, the one with abnormal path from latch to header (which
we _do_ reject during loop discovery!).  So what happens is
that we turn this "loop" into one that would have been recognized,
swap header and latch and thus get the abnormal edge to a tolerated
place (header to latch).  That inconsistency is what my patch tries
to address (another way would be to simply allow EH/abnormal
latch -> header edges as well).

So, tracer transforms

 <bb2>
   |
 <bb3>
  | ^
  | |(ab)
  v |
 <bb4> (loop1 header)
  | |                  \
 <bb5> (loop1 latch)    v BB6 -> BB1

by duplicating bb3 to

     <bb2>
       |
     <bb7> (duplicate of bb3)
       |
   ---<bb4> (loop1 header)
(ab) |  / \  \
  |  | <bb5> (loop1 latch)
 <bb3>

but it does not add bb3 to loop1, nor zero out its latch
and setting may-have-multiple-latches.  The cfghook
only takes care of updating the loop structure with
respect to the _new_ basic block but does not consider
that the old one magically becomes part of a loop.

But IMHO the bug is either that we don't consider it
part of the loop before this transform or that we do consider
it part of the loop after the transform!  Which is what
my patch tries to address.

Richard.