Re: [PATCH] Fix RTL checking failure in fix_crossing_unconditional_branches

[Ian Lance Taylor <ian at airs dot com>]

Eric Botcazou <ebotcazou@libertysurf.fr> writes:

> I'm the only one who tests with RTL checking enabled or...?

Yes.  You are the only one.

> Index: bb-reorder.c
> ===================================================================
> RCS file: /cvs/gcc/gcc/gcc/bb-reorder.c,v
> retrieving revision 1.114
> diff -u -p -r1.114 bb-reorder.c
> --- bb-reorder.c	5 Oct 2005 19:11:42 -0000	1.114
> +++ bb-reorder.c	21 Oct 2005 19:37:27 -0000
> @@ -1736,7 +1736,7 @@ fix_crossing_unconditional_branches (voi
>  	      /* Make sure every instruction in the new jump sequence has
>  		 its basic block set to be cur_bb.  */
>  	      
> -	      for (cur_insn = indirect_jump_sequence; cur_insn;
> +	      for (cur_insn = indirect_jump_sequence; !BARRIER_P (cur_insn);
>  		   cur_insn = NEXT_INSN (cur_insn))
>  		{
>  		  BLOCK_FOR_INSN (cur_insn) = cur_bb;

My only discomfort is that this is a sequence generated from the MD
file.  It is hard to understand why anybody would handle indirect_jump
as anything other than a simple (set (pc) (match_operand)).  But
people might do some wacky stuff.  What if we just do

  if (!BARRIER_P (cur_insn))
    BLOCK_FOR_INSN (cur_insn) = cur_bb;

That should be safe as it is done all over the place.  That patch is
approved if it fixes the problem.  Actually it qualifies as obvious.


Looking at this I wonder if it would just be simpler to let BARRIERs
have a BLOCK_FOR_INSN field.  I don't see any good reason that they
don't.  BARRIERs used to be smaller than other insns, but that stopped
being true here:
    http://gcc.gnu.org/ml/gcc/2002-08/msg00884.html
Letting them have a BLOCK_FOR_INSN field would permit us to remove a
bunch of !BARRIER_P tests.

Ian