On Mon, Nov 25, 2013 at 7:25 PM, Jeff Law <[email protected]> wrote:
> On 11/22/13 08:56, Richard Biener wrote:
>>
>>
>>> So the issue here is we can create irreducible regions & new nested
>>> loops. Does just setting the header,latch fields for the current loop
>>> handle those cases?
>>
>>
>> Yes.
>
> Fixed via the attached patch.
>
> Bootstrapped and regression tested on x86_64-unknown-linux-gnu. Also tested
> by removing the requirement that the loop header contain a multi-way branch,
> bootstrapping and testing that compiler as well. Installed on the trunk.
>
> Jeff
>
>
>
> * tree-ssa-threadupdate.c (thread_through_all_blocks): Selectively
> invalidate loop information.
>
>
>
> diff --git a/gcc/tree-ssa-threadupdate.c b/gcc/tree-ssa-threadupdate.c
> index ee0c838..1a52e47 100644
> --- a/gcc/tree-ssa-threadupdate.c
> +++ b/gcc/tree-ssa-threadupdate.c
> @@ -1579,7 +1579,6 @@ thread_through_all_blocks (bool may_peel_loop_headers)
> bitmap_iterator bi;
> bitmap threaded_blocks;
> struct loop *loop;
> - bool totally_clobbered_loops = false;
>
> /* We must know about loops in order to preserve them. */
> gcc_assert (current_loops != NULL);
> @@ -1675,9 +1674,15 @@ thread_through_all_blocks (bool
> may_peel_loop_headers)
> /* Our path is still valid, thread it. */
> if (e->aux)
> {
> - totally_clobbered_loops
> - |= thread_block ((*path)[0]->e->dest, false);
> + struct loop *loop = (*path)[0]->e->dest->loop_father;
> +
> + retval |= thread_block ((*path)[0]->e->dest, false);
> e->aux = NULL;
> +
> + /* This jump thread likely totally scrambled this loop.
> + So arrange for it to be fixed up. */
> + loop->header = NULL;
> + loop->latch = NULL;
But only necessary if this threading returned true, no? Also
how "likely" did it scramble the loop? I see that thread_block_1
already cancels loops in some cases so I wonder what case
it misses so that you need to cancel the loop unconditonally here.
Do you have a testcase that ICEs if I remove the cancelling above?
Thanks,
Richard.
> }
> }
> else
> @@ -1700,32 +1705,7 @@ thread_through_all_blocks (bool
> may_peel_loop_headers)
> threaded_blocks = NULL;
> paths.release ();
>
> - /* If we made changes to the CFG that might have totally messed
> - up the loop structure, then drop the old loop structure and
> - rebuild. */
> - if (totally_clobbered_loops)
> - {
> - /* Release the current loop structures, they are totally
> - clobbered at this point. */
> - loop_optimizer_finalize ();
> - current_loops = NULL;
> -
> - /* Similarly for dominance information. */
> - free_dominance_info (CDI_DOMINATORS);
> - free_dominance_info (CDI_POST_DOMINATORS);
> -
> - /* Before we can rebuild the loop structures, we need dominators,
> - which requires no unreachable code. So remove unreachable code.
> */
> - delete_unreachable_blocks ();
> -
> - /* Now rebuild the loop structures. */
> - cfun->curr_properties &= ~PROP_loops;
> - loop_optimizer_init (AVOID_CFG_MODIFICATIONS);
> - cfun->curr_properties |= PROP_loops;
> - retval = 1;
> - }
> -
> - if (retval && current_loops)
> + if (retval)
> loops_state_set (LOOPS_NEED_FIXUP);
>
> return retval;
>
