[Patch] Fix PR41440 bad edge insertion

Michael Matz matz@suse.de
Thu Oct 1 14:56:00 GMT 2009


Hi,

On Thu, 1 Oct 2009, Steven Bosscher wrote:

> >> I don't like this approach at all, for two reasons:
> >>
> >> 1. Garbage rtl generated (a reg and an insn)
> >> 2. Looks like it papers over a problem (insns inserted in  the wrong place).
> >
> > Yes to 1, no to 2.  See bugreport.  The problem is basically that the RTL
> > target expanders are free to emit jumpy sequences even for non-jump
> > instructions.  This normally is not a problem because we use
> > find_many_sub_basicblocks later anyway.  It is a problem in a limited set
> > of situations, namely if something is queued on the edge (we need to emit
> > this before find_many_sub_basicblocks), and that edge is fallthru (due to
> > the last gimple insn being a non-jump) and this last instruction is
> > expanded in such a way that the last RTL instruction is an unexpected jump
> > (due to target expanders).
> 
> Would just clearing the EDGE_FALLTHRU flag not help, if the block ends
> unexpectedly in a jump?

Nope.  The edge inserter (see commit_one_edge_insertion) unconditionally 
inserts before the jump if the block has just one successor and it ends in 
a jump (no matter the fallthru flag).  As this is totally correct if the 
CFG weren't inconsistent in a defined way, I preferred for Andrew not to 
change that function but instead make the CFG consistent enough before 
handing it down.

One other solution would be to add a second edge if we see a "stale" 
condjump but only have one edge.  This would need to be done very careful 
to not disturb the instructions queued on other edges, and in particular 
for the situation that any of these edge in the end has to be split.  

This seems even more fragile (at least I couldn't convince myself that it 
trivially solves all situations without introducing bugs; it becomes a bit 
complicated when you have to assume that there might actually be multiple 
jumps in front of the last one) than just live with the nop move.  It will 
be get rid of fairly soon again.  An emit_nop wouldn't be as good as this 
would live through to the final asm input.

Creating a new pseudo for each block that exhibits the problem also 
worries me a bit, we could reuse the same all the time for one function.  
That way at least the reg allocater wouldn't be slowed down much.


Ciao,
Michael.


More information about the Gcc-patches mailing list