>       PR middle-end/50496
>       * cfgrtl.c (try_redirect_by_replacing_jump): Treat EXIT_BLOCK_PTR case
>       separately before call to redirect_jump(). Add assertion.
>       (patch_jump_insn): Same.

This will definitely disable redirections to the exit block.  Now...

> diff --git a/gcc/cfgrtl.c b/gcc/cfgrtl.c
> index b3f045b..57f561f 100644
> --- a/gcc/cfgrtl.c
> +++ b/gcc/cfgrtl.c
> @@ -846,11 +846,10 @@ try_redirect_by_replacing_jump (edge e, basic_block
> target, bool in_cfglayout) if (dump_file)
>       fprintf (dump_file, "Redirecting jump %i from %i to %i.\n",
>                INSN_UID (insn), e->dest->index, target->index);
> -      if (!redirect_jump (insn, block_label (target), 0))
> -     {
> -       gcc_assert (target == EXIT_BLOCK_PTR);
> -       return NULL;
> -     }
> +      if (target == EXIT_BLOCK_PTR)
> +     return NULL;
> +      if (! redirect_jump (insn, block_label (target), 0))
> +     gcc_unreachable ();
>      }
>
>    /* Cannot do anything for target exit block.  */
> @@ -1030,11 +1029,10 @@ patch_jump_insn (rtx insn, rtx old_label,
> basic_block new_bb) /* If the substitution doesn't succeed, die.  This can
> happen
>            if the back end emitted unrecognizable instructions or if
>            target is exit block on some arches.  */
> -       if (!redirect_jump (insn, block_label (new_bb), 0))
> -         {
> -           gcc_assert (new_bb == EXIT_BLOCK_PTR);
> -           return false;
> -         }
> +       if (new_bb == EXIT_BLOCK_PTR)
> +         return false;
> +       if (! redirect_jump (insn, block_label (new_bb), 0))
> +         gcc_unreachable ();
>       }
>      }
>    return true;

... the code is pretty clear: attempting to redirect to the exit block is a 
valid operation (since its potential failure is expected by the assertion).

The problem is that the interface to redirect_jump/redirect_jump_1 has recently 
been changed:

2011-07-28  Bernd Schmidt  <ber...@codesourcery.com>

[...]

        * jump.c (delete_related_insns): Likewise.
        (jump_to_label_p): New function.
        (redirect_target): New static function.
        (redirect_exp_1): Use it.  Adjust to handle ret_rtx in JUMP_LABELS.
        (redirect_jump_1): Assert that the new label is nonnull.
        (redirect_jump): Likewise.
        (redirect_jump_2): Check for ANY_RETURN_P rather than NULL labels.

[...]

but all the callers haven't apparently been updated.  The model seems to be:

        (dead_or_predicable): Change NEW_DEST arg to DEST_EDGE.  All callers
        changed.  Ensure that the right label is passed to redirect_jump.

@@ -4134,10 +4137,16 @@ dead_or_predicable (basic_block test_bb,
   old_dest = JUMP_LABEL (jump);
   if (other_bb != new_dest)
     {
-      new_label = block_label (new_dest);
+      if (JUMP_P (BB_END (dest_edge->src)))
+       new_dest_label = JUMP_LABEL (BB_END (dest_edge->src));
+      else if (new_dest == EXIT_BLOCK_PTR)
+       new_dest_label = ret_rtx;
+      else
+       new_dest_label = block_label (new_dest);
+
       if (reversep
-         ? ! invert_jump_1 (jump, new_label)
-         : ! redirect_jump_1 (jump, new_label))
+         ? ! invert_jump_1 (jump, new_dest_label)
+         : ! redirect_jump_1 (jump, new_dest_label))
        goto cancel;
     }

so the correct fix is very likely something like:

Index: cfgrtl.c
===================================================================
--- cfgrtl.c    (revision 179844)
+++ cfgrtl.c    (working copy)
@@ -1024,13 +1024,20 @@ patch_jump_insn (rtx insn, rtx old_label

       if (!currently_expanding_to_rtl || JUMP_LABEL (insn) == old_label)
        {
+         rtx new_label;
+
          /* If the insn doesn't go where we think, we're confused.  */
          gcc_assert (JUMP_LABEL (insn) == old_label);

+         if (new_bb == EXIT_BLOCK_PTR)
+           new_label = ret_rtx;
+         else
+           new_label = block_label (new_bb);
+
          /* If the substitution doesn't succeed, die.  This can happen
             if the back end emitted unrecognizable instructions or if
             target is exit block on some arches.  */
-         if (!redirect_jump (insn, block_label (new_bb), 0))
+         if (!redirect_jump (insn, new_label, 0))
            {
              gcc_assert (new_bb == EXIT_BLOCK_PTR);
              return false;


Bernd, should all the callers of redirect_jump/redirect_jump_1 be audited for 
this pattern (there are 3 of them in cfgrtl.c for example)?

-- 
Eric Botcazou

Reply via email to