On Mon, Nov 25, 2013 at 7:25 PM, Jeff Law <l...@redhat.com> 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;
>

Reply via email to