https://gcc.gnu.org/bugzilla/show_bug.cgi?id=72772
Richard Biener <rguenth at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |hubicka at gcc dot gnu.org
--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> ---
Thus the following "fixes" it. I'm really not sure at which level we should
try to fix this - ideally passes doing this kind of transform would reset
the upper bound (and niter info). Unfortunately in this case it is really
CFG cleanup ... but the thread pass threading destination is a new header
and not yet identified as such (the thread pass fails to create the new loop
itself, leaving it to loop fixup - in this case CFG cleanup avoids this new
loop by mashing it with the existing one). In fact tree_forwarder_block_p
guards against the transform (but it doesnt' work because BB is not yet
marked as loop header):
if (current_loops)
{
basic_block dest;
/* Protect loop headers. */
if (bb->loop_father->header == bb)
return false;
CFG cleanup runs loop fixup after its job and I'm (not) sure we want to do it
at its start as well to catch this case.
Index: gcc/loop-init.c
===================================================================
--- gcc/loop-init.c (revision 239117)
+++ gcc/loop-init.c (working copy)
@@ -306,6 +311,8 @@ fix_loop_structure (bitmap changed_bbs)
flow_loop_free (loop);
any_deleted = true;
}
+ else if (loop)
+ loop->any_upper_bound = false;
/* If we deleted loops then the cached scalar evolutions refering to
those loops become invalid. */
The alternative is to make sure loops are fixed up before CFG cleanup as
it guards transforms using loop info:
Index: gcc/tree-cfgcleanup.c
===================================================================
--- gcc/tree-cfgcleanup.c (revision 239117)
+++ gcc/tree-cfgcleanup.c (working copy)
@@ -731,59 +731,19 @@ cleanup_tree_cfg_1 (void)
}
-/* Remove unreachable blocks and other miscellaneous clean up work.
- Return true if the flowgraph was modified, false otherwise. */
-
-static bool
-cleanup_tree_cfg_noloop (void)
-{
- bool changed;
-
- timevar_push (TV_TREE_CLEANUP_CFG);
-
- /* Iterate until there are no more cleanups left to do. If any
- iteration changed the flowgraph, set CHANGED to true.
-
- If dominance information is available, there cannot be any unreachable
- blocks. */
- if (!dom_info_available_p (CDI_DOMINATORS))
- {
- changed = delete_unreachable_blocks ();
- calculate_dominance_info (CDI_DOMINATORS);
- }
- else
- {
- checking_verify_dominators (CDI_DOMINATORS);
- changed = false;
- }
-
- changed |= cleanup_tree_cfg_1 ();
-
- gcc_assert (dom_info_available_p (CDI_DOMINATORS));
- compact_blocks ();
-
- checking_verify_flow_info ();
-
- timevar_pop (TV_TREE_CLEANUP_CFG);
-
- if (changed && current_loops)
- loops_state_set (LOOPS_NEED_FIXUP);
-
- return changed;
-}
-
/* Repairs loop structures. */
@@ -803,17 +764,57 @@ repair_loop_structures (void)
timevar_pop (TV_REPAIR_LOOPS);
}
-/* Cleanup cfg and repair loop structures. */
+/* Cleanup the CFG by removing unreachable blocks and doing other
miscellaneous
+ clean up work. Also repair loop structures if necessary.
+
+ Return true if the flowgraph was modified, false otherwise. */
bool
cleanup_tree_cfg (void)
{
- bool changed = cleanup_tree_cfg_noloop ();
+ bool changed;
+
+ timevar_push (TV_TREE_CLEANUP_CFG);
+
+ /* Iterate until there are no more cleanups left to do. If any
+ iteration changed the flowgraph, set CHANGED to true.
+
+ If dominance information is available, there cannot be any unreachable
+ blocks. */
+ if (!dom_info_available_p (CDI_DOMINATORS))
+ {
+ changed = delete_unreachable_blocks ();
+ calculate_dominance_info (CDI_DOMINATORS);
+ }
+ else
+ {
+ checking_verify_dominators (CDI_DOMINATORS);
+ changed = false;
+ }
+ /* Repair loop structures as we rely on loops being correct with
+ guarding several of the transforms. */
if (current_loops != NULL
&& loops_state_satisfies_p (LOOPS_NEED_FIXUP))
repair_loop_structures ();
+ changed |= cleanup_tree_cfg_1 ();
+
+ gcc_assert (dom_info_available_p (CDI_DOMINATORS));
+ compact_blocks ();
+
+ checking_verify_flow_info ();
+
+ timevar_pop (TV_TREE_CLEANUP_CFG);
+
+ if (changed && current_loops != NULL)
+ {
+ /* We are not marking loops for fixup in all cases, thus force it
+ here. */
+ loops_state_set (LOOPS_NEED_FIXUP);
+ repair_loop_structures ();
+ }
+
return changed;
}
I'm leaning towards this one even though it is more expensive :/ (and we
should fix that last comment issue and re-visit the loop-closed SSA update
stuff we do in repair_loop_structures).
