This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch] Stay in cfglayout mode until after cse2


>> > 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
away.

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.

Paolo


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]