Re: [PATCH] Fix shrink-wrap bug with anticipating into loops (PR67778, PR68634)

[Jakub Jelinek <jakub at redhat dot com>]

On Wed, Dec 02, 2015 at 06:21:47PM +0000, Segher Boessenkool wrote:
> --- a/gcc/shrink-wrap.c
> +++ b/gcc/shrink-wrap.c
> @@ -752,7 +752,11 @@ try_shrink_wrapping (edge *entry_edge, bitmap_head *bb_with,
>  
>    /* If we can move PRO back without having to duplicate more blocks, do so.
>       We can move back to a block PRE if every path from PRE will eventually
> -     need a prologue, that is, PRO is a post-dominator of PRE.  */
> +     need a prologue, that is, PRO is a post-dominator of PRE.  We might
> +     need to duplicate PRE if there is any path from a successor of PRE back
> +     to PRE, so don't allow that either (but self-loops are fine, as are any
> +     other loops entirely dominated by PRE; this in general seems too
> +     expensive to check for, for such an uncommon case).  */

So, what will happen if PRE self-loops?  It would be nice to have it covered
by a testcase.

> +	  bool ok = true;
> +
> +	  if (!can_get_prologue (pre, prologue_clobbered))
> +	    ok = false;
> +
> +	  FOR_EACH_EDGE (e, ei, pre->succs)
> +	    if (e->dest != pre
> +		&& dominated_by_p (CDI_POST_DOMINATORS, e->dest, pre))
> +	      ok = false;

I wonder if it wouldn't be better to:

	if (!can_get_prologue (pre, prologue_clobbered))
	  ok = false;
	else
	  FOR_EACH_EDGE (e, ei, pre->succs)
	    if (e->dest != pre
		&& dominated_by_p (CDI_POST_DOMINATORS, e->dest, pre))
	      {
		ok = false;
		break;
	      }

so that it doesn't walk or continue walking the edges if not needed.

Anyway, that are my comments, I'll defer the rest of the review to somebody
else.

	Jakub