On Wed, Dec 02, 2015 at 08:19:05PM +0100, Jakub Jelinek wrote:
> 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?
The prologue is put in a new block before the chosen one (as always).
> It would be nice to have it covered by a testcase.
If I knew how to prepare one, that stayed stable for more than about
two weeks, yes :-/
> > + 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.
If the compiler is any good, neither does my code, right? :-)
I think it is more important to have this code readable than a teeny
tiny bit faster. It is all linear (assuming dominator lookups are O(1)),
which isn't too hard to ascertain (yeah, famous last words).
Segher
