This is the mail archive of the
mailing list for the GCC project.
Re: [RFA][PATCH]Fix 59019
- From: Jeff Law <law at redhat dot com>
- To: Steven Bosscher <stevenb dot gcc at gmail dot com>
- Cc: gcc-patches <gcc-patches at gcc dot gnu dot org>
- Date: Sun, 17 Nov 2013 21:36:59 -0700
- Subject: Re: [RFA][PATCH]Fix 59019
- Authentication-results: sourceware.org; auth=none
- References: <528866B9 dot 3070008 at redhat dot com> <CABu31nNtJJuRJeTBii7cye0aj680Gyaqzjsih7MfMgyHDkXRfg at mail dot gmail dot com>
On 11/17/13 04:28, Steven Bosscher wrote:
I took const1_rtx from control_flow_insn_p. That's ultimately what we
need to be consistent with.
TRAP_CONDITION (PATTERN (i3)) == const1_rtx
But shouldn't the check be on const_true_rtx? Or does combine put a
+ basic_block bb = BLOCK_FOR_INSN (i3);
+ rtx last = get_last_bb_insn (bb);
This won't work, get_last_bb_insn() is intended to be used only in
cfgrtl mode and "combine" works in cfglayout mode. If you use it in
cfglayout mode on a block that ends in a tablejump, you get back the
JUMP_TABLE_DATA insn that is in BB_FOOTER and there is no NEXT_INSN
path from BB_END to any insns in the footer.
rtx last = BB_END (bb);
Any dead jump tables will be dealt with later.
My first iteration split the block and let cfgcleanup take care of the
rest. That seemed wasteful when all we really need to do is zap the
+ /* First remove all the insns after the trap. */
+ if (i3 != last)
+ delete_insn_chain (NEXT_INSN (i3), last, true);
+ /* And ensure there's no outgoing edges anymore. */
+ while (EDGE_COUNT (bb->succs) > 0)
+ remove_edge (EDGE_SUCC (bb, 0));
Alternatively, you could do "split_block (bb, i3);" and let cfgcleanup
deal with the new, unreachable basic block.
Umm, no. Failure to emit the barrier will result in a checking failure.
Been there, done that.
+ /* And ensure cfglayout knows this block does not fall through. */
+ emit_barrier_after_bb (bb);
Bah... Emitting the barrier is necessary here because
fixup_reorder_chain doesn't handle cases where a basic block is a dead
end. That is actually a bug in fixup_reorder_chain: Other passes could
create dead ends in the CFG in cfglayout mode and not emit a barrier
into BB_FOOTER, and fixup_reorder_chain wouldn't be able to handle
that (resulting in verify_flow_info failure).
Would you mind if I try spend some time making conditional traps be
control flow insns? It should make all of this a little bit less ugly.
And I have no fish to fry at all :-) Give me a week or two please, to
see if I can figure out those issues you've been running into.
Feel free. I'm not terribly concerned about this issue right now.
To trigger use the test in 59019 with an itanic cross compiler and
comment out these two lines from gimple-ssa-isolate-paths.c:
TREE_THIS_VOLATILE (op) = 1;
TREE_SIDE_EFFECTS (op) = 1;