Re: [PATCH] Fix up maybe_cleanup_end_of_block (PR middle-end/46499)

["H.J. Lu" <hjl.tools@gmail.com>]

On Mon, Nov 22, 2010 at 2:00 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> On the attached testcases jumpif* emits:
>   (jump_insn 8 7 9 (set (pc)
>           (label_ref 0)) pr46499-1.c:26 -1
>        (nil))
>
>   (barrier 9 8 10)
>
>   (jump_insn 10 9 11 (set (pc)
>           (label_ref 0)) pr46499-1.c:26 -1
>        (nil))
>
>   (barrier 11 10 0)
> where the label_ref in both cases is the non-fallthru label.  This is
> because ssa_name & ssa_name is the condition and both ssa_name's
> are found using TER to be 0 (and the -fno-* options ensure it is not
> optimized earlier).
> maybe_cleanup_end_of_block is called to find out when the sequence ends
> with an unconditional jump to the non-fallthru label, with no fallthru
> code_label afterwards, which means all the sequence does (assuming no
> side-effects in it but GIMPLE should ensure that) is that unconditional
> jump and attempts to remove any other jumps from the sequence.
> Apparently it was expecting just conditional jumps to the non-fallthru
> label, for unconditional ones as in this example the following
> barrier needs to be removed too, otherwise we end up with a barrier
> in a middle of a bb and we either ICE on it, or miscompile it.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2010-11-22  Jakub Jelinek  <jakub@redhat.com>
>
>        PR middle-end/46499
>        * cfgexpand.c (maybe_cleanup_end_of_block): Remove also BARRIERs
>        following unconditional jumps.
>
>        * gcc.dg/pr46499-1.c: New test.
>        * gcc.dg/pr46499-2.c: New test.
>
> --- gcc/cfgexpand.c.jj  2010-11-10 13:14:53.000000000 +0100
> +++ gcc/cfgexpand.c     2010-11-22 14:45:49.000000000 +0100
> @@ -1694,7 +1694,14 @@ maybe_cleanup_end_of_block (edge e, rtx
>        {
>          insn = PREV_INSN (insn);
>          if (JUMP_P (NEXT_INSN (insn)))
> -           delete_insn (NEXT_INSN (insn));
> +           {
> +             if (!any_condjump_p (insn))
                                                 ^^^^^^ Shouldn't it
be any_condjump_p (NEXT_INSN (insn))
> +               {
> +                 gcc_assert (BARRIER_P (NEXT_INSN (NEXT_INSN (insn))));
> +                 delete_insn (NEXT_INSN (NEXT_INSN (insn)));
> +               }
> +             delete_insn (NEXT_INSN (insn));
> +           }
>        }
>     }
>  }


-- 
H.J.