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;
+ }
