I have tested the following patch to fix another case where threading plus CFG cleanup messes up loops, identifying a previous loop with a new one and thus messing up nb_iterations_upper_bound. I believe preserving existing loops as much as possible is desirable (and CFG cleanup goes a long way protecting loop headers).
Thus the following patch makes it impossible that CFG cleanup accidentially makes a loop header the header of a larger/different loop by ensuring we only have a single entry edge (aka preheaders). While it would be nice to have this as a CFG property throughout threading breaks it easily and until we fix that (and maybe other places) the simpler fix is to make CFG cleanup re-instantiate preheaders where required. Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk. As this is a regression I plan to backport this so comments are still welcome. Thanks, Richard. 2017-04-28 Richard Biener <rguent...@suse.de> PR tree-optimization/80549 * tree-cfgcleanup.c (mfb_keep_latches): New helper. (cleanup_tree_cfg_noloop): Create forwarders to known loop headers if they do not have a preheader. * gcc.dg/torture/pr80549.c: New testcase. Index: gcc/tree-cfgcleanup.c =================================================================== *** gcc/tree-cfgcleanup.c (revision 247368) --- gcc/tree-cfgcleanup.c (working copy) *************** cleanup_tree_cfg_1 (void) *** 739,744 **** --- 739,749 ---- return retval; } + static bool + mfb_keep_latches (edge e) + { + return ! dominated_by_p (CDI_DOMINATORS, e->src, e->dest); + } /* Remove unreachable blocks and other miscellaneous clean up work. Return true if the flowgraph was modified, false otherwise. */ *************** cleanup_tree_cfg_noloop (void) *** 766,771 **** --- 771,834 ---- changed = false; } + /* Ensure that we have single entries into loop headers. Otherwise + if one of the entries is becoming a latch due to CFG cleanup + (from formerly being part of an irreducible region) then we mess + up loop fixup and associate the old loop with a different region + which makes niter upper bounds invalid. See for example PR80549. + This needs to be done before we remove trivially dead edges as + we need to capture the dominance state before the pending transform. */ + if (current_loops) + { + loop_p loop; + unsigned i; + FOR_EACH_VEC_ELT (*get_loops (cfun), i, loop) + if (loop && loop->header) + { + basic_block bb = loop->header; + edge_iterator ei; + edge e; + bool found_latch = false; + bool any_abnormal = false; + unsigned n = 0; + /* We are only interested in preserving existing loops, but + we need to check whether they are still real and of course + if we need to add a preheader at all. */ + FOR_EACH_EDGE (e, ei, bb->preds) + { + if (e->flags & EDGE_ABNORMAL) + { + any_abnormal = true; + break; + } + if (dominated_by_p (CDI_DOMINATORS, e->src, bb)) + { + found_latch = true; + continue; + } + n++; + } + /* If we have more than one entry to the loop header + create a forwarder. */ + if (found_latch && ! any_abnormal && n > 1) + { + edge fallthru = make_forwarder_block (bb, mfb_keep_latches, + NULL); + loop->header = fallthru->dest; + if (! loops_state_satisfies_p (LOOPS_NEED_FIXUP)) + { + /* The loop updating from the CFG hook is incomplete + when we have multiple latches, fixup manually. */ + remove_bb_from_loops (fallthru->src); + loop_p cloop = loop; + FOR_EACH_EDGE (e, ei, fallthru->src->preds) + cloop = find_common_loop (cloop, e->src->loop_father); + add_bb_to_loop (fallthru->src, cloop); + } + } + } + } + changed |= cleanup_tree_cfg_1 (); gcc_assert (dom_info_available_p (CDI_DOMINATORS)); Index: gcc/testsuite/gcc.dg/torture/pr80549.c =================================================================== *** gcc/testsuite/gcc.dg/torture/pr80549.c (nonexistent) --- gcc/testsuite/gcc.dg/torture/pr80549.c (working copy) *************** *** 0 **** --- 1,33 ---- + /* { dg-do run } */ + + signed char a, b; + int c; + short d; + void fn1(int p1) + { + short e = 4; + int f; + d = 0; + for (; d <= 0; d++) + e = 0; + if (e) + goto L1; + L2: + if (p1) { + a = 9; + for (; a; ++a) { + f = 5; + for (; f != 32; ++f) + c = 8; + L1: + if (b) + goto L2; + } + } + } + + int main() + { + fn1(1); + return 0; + }