This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH, PR68715] Add missing single_pred_p test in scop_detection::merge_sese
- From: Tom de Vries <Tom_deVries at mentor dot com>
- To: Richard Biener <rguenther at suse dot de>
- Cc: Sebastian Pop <sebpop at gmail dot com>, GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Wed, 16 Mar 2016 11:16:27 +0100
- Subject: Re: [PATCH, PR68715] Add missing single_pred_p test in scop_detection::merge_sese
- Authentication-results: sourceware.org; auth=none
- References: <56E9120A dot 7040404 at mentor dot com> <alpine dot LSU dot 2 dot 11 dot 1603160946280 dot 15580 at t29 dot fhfr dot qr>
On 16/03/16 09:53, Richard Biener wrote:
Hmm, it looks like for all what this function does this effectively
pessimizes scop merging and it would be easier to split 'exit'
in case its destination is unsuitable (not trivially empty).
Agreed.
The
/* For now we just want to bail out when exit does not post-dominate
entry.
TODO: We might just add a basic_block at the exit to make exit
post-dominate entry (the entire region). */
if (!dominated_by_p (CDI_POST_DOMINATORS, get_entry_bb (combined),
get_exit_bb (combined))
|| !dominated_by_p (CDI_DOMINATORS, get_exit_bb (combined),
comment also suggests that splitting the get_nearest_pdom_with_single_exit
edge and including the new BB in the combined region would also always fix
the dominance relation (though I don't see how it could do that and
the comment looks wrong and by construction the check should never
trigger).
I've replaced the entire condition with two asserts:
...
diff --git {a,b}/gcc/graphite-scop-detection.c
index 7615842..762a248 100644
--- a/gcc/graphite-scop-detection.c
+++ b/gcc/graphite-scop-detection.c
@@ -820,14 +820,10 @@ scop_detection::merge_sese
/* For now we just want to bail out when exit does not post-dominate
entry.
TODO: We might just add a basic_block at the exit to make exit
post-dominate entry (the entire region). */
- if (!dominated_by_p (CDI_POST_DOMINATORS, get_entry_bb (combined),
- get_exit_bb (combined))
- || !dominated_by_p (CDI_DOMINATORS, get_exit_bb (combined),
- get_entry_bb (combined)))
- {
- DEBUG_PRINT (dp << "[scop-detection-fail] cannot merge "
- "seses.\n");
- return invalid_sese;
- }
+ gcc_assert (dominated_by_p (CDI_POST_DOMINATORS,
+ get_entry_bb (combined),
+ get_exit_bb (combined)));
+ gcc_assert (dominated_by_p (CDI_DOMINATORS,
+ get_exit_bb (combined),
+ get_entry_bb (combined)));
/* FIXME: We should remove this piece of code once
canonicalize_loop_closed_ssa has been removed, because that function
...
and ran graphite.exp.
I ran into an ICE for pr68693.f90, for the second assert. The exit bb
has two incoming edges from within the sese, and one from outside. Given
that the exit bb is empty, we could split the exit edge and redirect the
edge from outside the sese to the new bb, which would fix the sese.
Perhaps that is what is meant in the comment, I'm not sure.
Thanks,
- Tom