Hi!

On Wed, Dec 16, 2020 at 02:35:32PM +0100, Jakub Jelinek wrote:
> As mentioned in the PR, shrink-wrapping disqualifies for prologue
> placement basic blocks that have EDGE_CROSSING incoming edge.
> I don't see why that is necessary, those edges seem to be redirected
> just fine, both on x86_64 and powerpc64.

This used to not work, as mentioned in the original patch submission:
https://patchwork.ozlabs.org/project/gcc/patch/52f14532eb742ac8d878a185a46a88da7b0326eb.1442588483.git.seg...@kernel.crashing.org/
I wonder what changed?

So, the situation is there are multiple incoming edges to where we want
to place the prologue.  Because we have to have one single edge to put
the prologue on (that is how the infrastructure we have works, it is not
anything fundamental), we create a new block that just jumps to the
block that needs the prologue, use that new edge for where we place the
prologue, and then redirect all edges to the first block to the new
block.  (We cannot in all case simply fall through.)

> In the former case, they
> are usually conditional jumps that patch_jump_insn can handle just fine,
> after all, they were previously crossing and will be crossing after
> the redirection too, just to a different label.  And in the powerpc64
> case, it is a simple_jump instead that again seems to be handled by
> patch_jump_insn just fine.

So what changed in the last five years that makes redirecting
EDGE_CROSSING jumps now always work?  Does that also apply to
EDGE_COMPLEX, btw?  Knowing this would make at least me a lot less
nervous about this patch :-)

> Sure, redirecting an edge that was previously not crossing to be crossing or
> vice versa can fail, but that is not what shrink-wrapping needs.

It used to ICE when not disallowing EDGE_CROSSING.

> Also tested in GCC 8 with this patch and don't see ICEs there either
> (though, of course, I'm not suggesting we should backport this to release
> branches).

The patch looks fine to me of course, modulo that worry.


Segher

Reply via email to