On Fri, Sep 11, 2015 at 11:19:40AM +0200, Bernd Schmidt wrote:
> On 09/10/2015 05:14 PM, Segher Boessenkool wrote:
> >This patch rewrites the shrink-wrapping algorithm, allowing non-linear
> >pieces of CFG to be duplicated for use without prologue instead of just
> >linear pieces.
> 
> An example would be good for this kind of patch, also in the comments.

I'll make up something for v2.

> >+  /* If there is more than one predecessor of PRO not dominated by PRO,
> >+     fail.  We don't have to do this (can split the block), but do this
> >+     for now (the original code disallowed this, too).
> 
> Comments shouldn't reference previous versions. Also, a comment 
> describing the why rather than just what is being done would be more 
> helpful.

I'll just remove that second sentence, it just describes what we do not
yet do.

> I'm wondering how your new algorithm prevents the prologue from being 
> placed inside a loop. Can you have a situation where this picks a 
> predecessor that is reachable but not dominated by PRO?

It doesn't prevent it!

The prologue will not be _inside_ the loop: there is one prologue, and it
is executed exactly once for any block needing it.  But the code can copy
part of the first iteration of a loop, if there are early exits.  Example
(from the testsuite, pr39943.c, -Os I think):

(_B_egin, _R_eturn, _S_imple_return; edges without arrowhead are down or
to the right, to simplify the diagram):

Block 3 needs a prologue (it has a call), the rest doesn't:



  B   3<--              B       ->3<--
  |   |   |             |      |  |   |
  |   v   |   becomes   |      |  v   |
  2---4---              2---5--   4---
      |                     |     |
      R                     S     R


by copying bb 4 to bb 5, and inserting the prologue on the edge 5->3.

> Other than that it looks pretty good.

Thanks, and thanks for the review!  I'll send v2 later today.


Segher

Reply via email to