On Fri, Feb 26, 2016 at 8:50 AM, Jeff Law <l...@redhat.com> wrote:
> On 02/25/2016 03:00 AM, Richard Biener wrote:
>>
>>
>> So I fail to see how only successor edges are relevant. Isn't the
>> important
>> case to catch whether we remove an edge marked EDGE_IRREDUCIBLE_LOOP?
>> Even if the BB persists we might have exposed a new loop here.
>>
>> Note that it is not safe to look at {BB,EDGE}_IRREDUCIBLE_LOOP if the loop
>> state does not have LOOPS_HAVE_MARKED_IRREDUCIBLE_REGIONS set
>> (the flags may be stale or missing). So it might be that we can't rely on
>> non-loop passes modifying the CFG to handle this optimistically.
>>
>> Thus, how about (my main point) moving this to remove_edge instead, like
>
> Yea. That works. The !loops_state_satisfies_p check will almost certainly
> cause us to trigger loop cleanups more often, but I think it's the
> right/safe thing to do to catch cases where we haven't go the
> IRREDUCIBLE_LOOP flags set.
>
>
> Bootstrapped and regression tested on x86_64-linux-gnu. OK for the trunk?
Ok with a comment added before that check.
Richard.
> Thanks,
> Jeff
>
>
> PR tree-optimization/69740
> * cfghooks.c (remove_edge): Request loop fixups if we delete
> an edge that might turn an irreducible loop into a natural
> loop.
>
> PR tree-optimization/69740
> * gcc.c-torture/compile/pr69740-1.c: New test.
> * gcc.c-torture/compile/pr69740-2.c: New test.
>
> diff --git a/gcc/cfghooks.c b/gcc/cfghooks.c
> index bbb1017..f80e455 100644
> --- a/gcc/cfghooks.c
> +++ b/gcc/cfghooks.c
> @@ -408,7 +408,14 @@ void
> remove_edge (edge e)
> {
> if (current_loops != NULL)
> - rescan_loop_exit (e, false, true);
> + {
> + rescan_loop_exit (e, false, true);
> +
> + if (!loops_state_satisfies_p (LOOPS_HAVE_MARKED_IRREDUCIBLE_REGIONS)
> + || (e->flags & EDGE_IRREDUCIBLE_LOOP)
> + || (e->dest->flags & BB_IRREDUCIBLE_LOOP))
> + loops_state_set (LOOPS_NEED_FIXUP);
> + }
>
> /* This is probably not needed, but it doesn't hurt. */
> /* FIXME: This should be called via a remove_edge hook. */
> diff --git a/gcc/testsuite/gcc.c-torture/compile/pr69740-1.c
> b/gcc/testsuite/gcc.c-torture/compile/pr69740-1.c
> new file mode 100644
> index 0000000..ac867d8
> --- /dev/null
> +++ b/gcc/testsuite/gcc.c-torture/compile/pr69740-1.c
> @@ -0,0 +1,12 @@
> +char a;
> +short b;
> +void fn1() {
> + if (b)
> + ;
> + else {
> + int c[1] = {0};
> + l1:;
> + }
> + if (a)
> + goto l1;
> +}
> diff --git a/gcc/testsuite/gcc.c-torture/compile/pr69740-2.c
> b/gcc/testsuite/gcc.c-torture/compile/pr69740-2.c
> new file mode 100644
> index 0000000..a89c9a0
> --- /dev/null
> +++ b/gcc/testsuite/gcc.c-torture/compile/pr69740-2.c
> @@ -0,0 +1,19 @@
> +inline int foo(int *p1, int p2) {
> + int z = *p1;
> + while (z > p2)
> + p2 = 2;
> + return z;
> +}
> +int main() {
> + int i;
> + for (;;) {
> + int j, k;
> + i = foo(&k, 7);
> + if (k)
> + j = i;
> + else
> + k = j;
> + if (2 != j)
> + __builtin_abort();
> + }
> +}
>
