[patch] Stay in cfglayout mode until after cse2
Mon Mar 26 13:06:00 GMT 2007
>> > 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.
> But cfg_layout_finalize already forces a barrier after non-fallthrough
> edges, so you still shouldn't need this.
It only does this when it creates a non-fallthrough edge itself
in force_nonfallthru, AFAICT. It seems like a barrier is not
necessary before combine, but is necessary afterwards because
of some simplification. If it really bothers you, I'll look for
a reduced testcase.
> Also, afaics you add the barrier while still in cfglayout mode, and
> you shouldn't have to do that.
You're right that I should have added it in the footer, probably.
>> 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.
> Then the proper thing to do would be to update the CFG in place, i.e.
> the place that turns the conditional jump into an unconditional one.
> You would only have to remove the redundant edge, and put the
> EDGE_FALLTHRU flag on the other edge.
I guess you mean that I should *not* use purge_dead_edges or something
like that. This part however does not bother me much. If I add an
assertion to combine that it is executed in cfglayout mode, I can
inline it in the callers, and the troublesome comment would probably go
I'll look for a testcase for the other part.
>> It might be that this is a latent bug in delete_insn_and_edges.
> I don't know. If you don't tell it which edge to keep, it'll keep the
> wrong edge, I suppose.
Maybe, yes. Or maybe that's what delete_insn_and_edges is supposed
to do, because it's not called delete_insn_because_in_cfglayout_mode_it_is_redundant.
More information about the Gcc-patches