[patch] Stay in cfglayout mode until after cse2

Paolo Bonzini paolo.bonzini@lu.unisi.ch
Mon Mar 26 08:37:00 GMT 2007


>>                /* Otherwise we have some return, switch or computed
>>                   jump.  In the 99% case, there should not have been a
>>                   fallthru edge.  */
>> +              if (NEXT_INSN (bb_end_insn) == 0
>> +                  || !BARRIER_P (NEXT_INSN (bb_end_insn)))
>> +                emit_barrier_after (bb_end_insn);
>>               continue;
> 
> This is wrong. You would emit the barrier as a non-insn after the end
> of the basic block, which you can't do in cfglayout mode. If the
> barrier would go anywhere, it would be in bb->il.rtl->footer.  But in
> this case, emiting a barrier is just wrong. Why did you need this?

I don't really recall this exactly, it will take some more
investigation.  It doesn't seem to be so wrong to force the
presence of a barrier when going *out* of cfglayout mode if
there's no fallthrough edge.  The presence of barriers is
probably the biggest difference between cfglayout and cfgrtl
mode, isn't it?


>> +static void
>> +update_cfg_for_uncondjump (rtx insn, bool fallthru)
> 
> No comment before the function, so I have no idea what you are trying
> to do here.

Will fix.  It takes a conditional jump INSN that was simplified,
and the CFG in order to keep only one edge.  FALLTHRU is
true if the insn is now of the form (set (pc) (pc)), false
if it is of the form (set (pc) (label_ref XYZ)).

>> +{
>> +  basic_block bb = BLOCK_FOR_INSN (insn);
>> +
>> +  if (current_ir_type () == IR_RTL_CFGLAYOUT)
>> +    {
>> +      /* For a (set (pc) (label_ref FOO)), we need to delete the insn
>> *after*
>> +        removing the dead edges.  Otherwise, we always keep the old
>> fallthrough
>> +        edge, which is wrong.  */
> 
> Why is that?
> Just for my understanding: What are we trying to do here?  Is this
> something we have to do if you've turned a conditional jump into a
> non-conditional one?  In that case the CFG should be updated in place
> with the proper cfg manipulation functions.

delete_insn_and_edges amounts basically to delete_insn +
purge_dead_edges.  In the case of a conditional jump having
having been simplfied to (set (pc) (label_ref FOO)), p_d_e
will see two edges and no jump -- it will then keep the old
fallthru edge, which is wrong as it should keep the *other* edge.

Instead, if I call purge_dead_edges before, it deletes the
old fallthru edge and leaves the other one.  Then I delete
the insn and make the sole remaining edge a fallthru one.

It might be that this is a latent bug in delete_insn_and_edges.

> Note that unconditional jumns may be omited in cfglayout mode.

Exactly.  After purge_dead_edges, in fact, I have delete_insn
to delete the now-unconditional jump.

> I don't know if this will make us miss
> opportunities to combine insns with unconditional jumps, if some silly
> target wants to be able to do that...

No, that should not be a problem.

Paolo



More information about the Gcc-patches mailing list