Re: [PATCH] Allow giving up when redirecting a CROSSING_JUMP_P (PR rtl-optimization/87475)

[Richard Biener <richard dot guenther at gmail dot com>]

On Fri, Nov 16, 2018 at 9:55 AM Jakub Jelinek <jakub@redhat.com> wrote:
>
> Hi!
>
> On this testcase on aarch64-linux, we have a bb end like:
> (insn 119 80 120 5 (set (reg:DI 110)
>         (high:DI (label_ref:DI 19))) -1
>      (insn_list:REG_LABEL_OPERAND 19 (nil)))
> (insn 120 119 121 5 (set (reg:DI 109)
>         (lo_sum:DI (reg:DI 110)
>             (label_ref:DI 19))) -1
>      (insn_list:REG_LABEL_OPERAND 19 (expr_list:REG_EQUAL (label_ref:DI 19)
>             (nil))))
> (jump_insn/j 121 120 19 5 (set (pc)
>         (reg:DI 109)) -1
>      (nil)
>  -> 19)
> and try to redirect that CROSSING_JUMP_P to another label.  As it is not
> considered a computed jump (as it has a JUMP_LABEL), but it is hard to
> adjust it (we'd need to generate new insns to compute that label into a
> register and replace the old ones with it (and find them)), redirect_jump
> fails, but patch_jump_insn assumes it must not fail.
>
> I think it is best to allow it to fail, at least until somebody writes code
> to redirect_jump even this kind of calls.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux + tested with
> aarch64-linux cross on the testcase.  Ok for trunk?

LGTM.

Richard.

> 2018-11-16  Jakub Jelinek  <jakub@redhat.com>
>
>         PR rtl-optimization/87475
>         * cfgrtl.c (patch_jump_insn): Allow redirection failure for
>         CROSSING_JUMP_P insns.
>         (cfg_layout_redirect_edge_and_branch): Don't ICE if ret is NULL.
>
>         * g++.dg/opt/pr87475.C: New test.
>
> --- gcc/cfgrtl.c.jj     2018-11-15 09:46:46.383739915 +0100
> +++ gcc/cfgrtl.c        2018-11-15 16:58:08.972980656 +0100
> @@ -1268,11 +1268,13 @@ patch_jump_insn (rtx_insn *insn, rtx_ins
>
>           /* 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.  */
> +            target is exit block on some arches.  Or for crossing
> +            jumps.  */
>           if (!redirect_jump (as_a <rtx_jump_insn *> (insn),
>                               block_label (new_bb), 0))
>             {
> -             gcc_assert (new_bb == EXIT_BLOCK_PTR_FOR_FN (cfun));
> +             gcc_assert (new_bb == EXIT_BLOCK_PTR_FOR_FN (cfun)
> +                         || CROSSING_JUMP_P (insn));
>               return false;
>             }
>         }
> @@ -4460,6 +4462,9 @@ cfg_layout_redirect_edge_and_branch (edg
>    else
>      ret = redirect_branch_edge (e, dest);
>
> +  if (!ret)
> +    return NULL;
> +
>    fixup_partition_crossing (ret);
>    /* We don't want simplejumps in the insn stream during cfglayout.  */
>    gcc_assert (!simplejump_p (BB_END (src)) || CROSSING_JUMP_P (BB_END (src)));
> --- gcc/testsuite/g++.dg/opt/pr87475.C.jj       2018-11-15 17:05:51.793393450 +0100
> +++ gcc/testsuite/g++.dg/opt/pr87475.C  2018-11-15 17:05:34.713673436 +0100
> @@ -0,0 +1,7 @@
> +// PR rtl-optimization/87475
> +// { dg-do compile { target freorder } }
> +// { dg-options "-O2 -freorder-blocks-and-partition -fmodulo-sched" }
> +
> +struct A { A (); ~A (); };
> +int foo (A, A);
> +void bar (bool x) { x ? foo (A (), A ()) : 0; }
>
>         Jakub