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.
