This is the mail archive of the
mailing list for the GCC project.
Re: [PATCH]: Fix PR29201 - [4.2 Regression] ICE in create_recovery_block, at haifa-sched.c:3692 at -O3
- From: Ian Lance Taylor <iant at google dot com>
- To: Maxim Kuvyrkov <mkuvyrkov at ispras dot ru>
- Cc: Vladimir Makarov <vmakarov at redhat dot com>, gcc-patches at gcc dot gnu dot org, fang at csl dot cornell dot edu
- Date: 03 Oct 2006 10:27:34 -0700
- Subject: Re: [PATCH]: Fix PR29201 - [4.2 Regression] ICE in create_recovery_block, at haifa-sched.c:3692 at -O3
- References: <4520109E.email@example.com> <firstname.lastname@example.org> <4522980B.email@example.com>
Maxim Kuvyrkov <firstname.lastname@example.org> writes:
> >> The bug due to lack of handling of table jumps during speculative
> >> scheduling on ia64. There is assert that fails when a basic block
> >> that doesn't have a fallthrough successor is not immediately followed
> >> by a barrier. The table jumps have that barrier but a little later.
> >> The fix simply searches for that barrier.
> >> 2006-10-01 Maxim Kuvyrkov <email@example.com>
> >> PR target/29201
> >> * haifa-sched.c (get_barrier): New static function.
> >> (create_recovery_block): Use it.
> >> (extend_bb): Clear BLOCK_FOR_INSN for note that is out of the block.
> > All of this code is kind of ugly, but I don't particularly like this
> > solution. I think that you should not change extend_bb nor add
> The proposed change for extend_bb () merely does two things:
> 1. Clear BLOCK_FOR_INSN field of the note that is forced to be outside
> the block. This is good because this is right.
> 2. Add a comment. This is good unless the comment is wrong.
I don't see why we should be relying on having insns that are not in
any basic block. I don't think you need to clean that up, but I don't
think we should rely on it. So I would prefer to see a change to
create_recovery_block that doesn't rely on that. Under that
assumption, the change to extend_bb, while probably correct given the
way the code works today, is a separate, unrelated, patch.
> This is how rtl of the table jump looks:
> (set pc (reg))
> (use (label 40))) ;; Last insn in the bb.
> (note) ;; Out of any bb. Accidentally generated by extend_bb ().
> (label 40) ;; Out of any bb
> (addr_vec) ;; Out of any bb. The insn type is JUMP_INSN.
> (barrier) ;; Out of any bb
> The fix you've proposed will fail the same way as the present code.
> We must skip not only notes but also labels and jumps.
Yeah, tablejumps are a special case. We could do something like this:
barrier = next_nonnote_insn (BB_END (before_recovery));
if (LABEL_P (barrier) && JUMP_TABLE_DATA_P (NEXT_INSN (barrier)))
barrier = NEXT_INSN (NEXT_INSN (barrier));
gcc_assert (BARRIER_P (barrier));
Or we could wrap that up in a little function.
I think this code is a bit wacky because it is trying to use the CFG
and the straight line insn ordering at the same time.