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]: Fix PR29201 - [4.2 Regression] ICE in create_recovery_block, at haifa-sched.c:3692 at -O3


Maxim Kuvyrkov <mkuvyrkov@ispras.ru> 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  <mkuvyrkov@ispras.ru>
> >>
> >> 	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:
> 
> -------------------------------------------------
> (parallel
>    (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.

Ian


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