On Thu, 7 Mar 2024, Jakub Jelinek wrote:
> Hi!
>
> The following testcase ICEs, because fix_crossing_unconditional_branches
> thinks that asm goto is an unconditional jump and removes it, replacing it
> with unconditional jump to one of the labels.
> This doesn't happen on x86 because the function in question isn't invoked
> there at all:
> /* If the architecture does not have unconditional branches that
> can span all of memory, convert crossing unconditional branches
> into indirect jumps. Since adding an indirect jump also adds
> a new register usage, update the register usage information as
> well. */
> if (!HAS_LONG_UNCOND_BRANCH)
> fix_crossing_unconditional_branches ();
> I think for the asm goto case, for the non-fallthru edge if any we should
> handle it like any other fallthru (and fix_crossing_unconditional_branches
> doesn't really deal with those, it only looks at explicit branches at the
> end of bbs and we are in cfglayout mode at that point) and for the labels
> we just pass the labels as immediates to the assembly and it is up to the
> user to figure out how to store them/branch to them or whatever they want to
> do.
> So, the following patch fixes this by not treating asm goto as a simple
> unconditional jump.
>
> I really think that on the !HAS_LONG_UNCOND_BRANCH targets we have a bug
> somewhere else, where outofcfglayout or whatever should actually create
> those indirect jumps on the crossing edges instead of adding normal
> unconditional jumps, I see e.g. in
> __attribute__((cold)) int bar (char *);
> __attribute__((hot)) int baz (char *);
> void qux (int x) { if (__builtin_expect (!x, 1)) goto l1; bar (""); goto l1;
> l1: baz (""); }
> void corge (int x) { if (__builtin_expect (!x, 0)) goto l1; baz (""); l2:
> return; l1: bar (""); goto l2; }
> with -O2 -freorder-blocks-and-partition on aarch64 before/after this patch
> just b .L? jumps which I believe are +-32MB, so if .text is larger than
> 32MB, it could fail to link, but this patch doesn't address that.
>
> Bootstrapped/regtested on x86_64-linux, i686-linux and aarch64-linux, ok for
> trunk?
OK.
Thanks,
Richard.
> 2024-03-07 Jakub Jelinek <ja...@redhat.com>
>
> PR rtl-optimization/110079
> * bb-reorder.cc (fix_crossing_unconditional_branches): Don't adjust
> asm goto.
>
> * gcc.dg/pr110079.c: New test.
>
> --- gcc/bb-reorder.cc.jj 2024-01-03 11:51:32.000000000 +0100
> +++ gcc/bb-reorder.cc 2024-03-06 18:34:29.468016144 +0100
> @@ -2266,7 +2266,8 @@ fix_crossing_unconditional_branches (voi
> /* Make sure the jump is not already an indirect or table jump. */
>
> if (!computed_jump_p (last_insn)
> - && !tablejump_p (last_insn, NULL, NULL))
> + && !tablejump_p (last_insn, NULL, NULL)
> + && !asm_noperands (PATTERN (last_insn)))
> {
> /* We have found a "crossing" unconditional branch. Now
> we must convert it to an indirect jump. First create
> --- gcc/testsuite/gcc.dg/pr110079.c.jj 2024-03-06 18:42:47.175250069
> +0100
> +++ gcc/testsuite/gcc.dg/pr110079.c 2024-03-06 18:44:47.008620726 +0100
> @@ -0,0 +1,43 @@
> +/* PR rtl-optimization/110079 */
> +/* { dg-do compile { target lra } } */
> +/* { dg-options "-O2" } */
> +/* { dg-additional-options "-freorder-blocks-and-partition" { target
> freorder } } */
> +
> +int a;
> +__attribute__((cold)) int bar (char *);
> +__attribute__((hot)) int baz (char *);
> +
> +void
> +foo (void)
> +{
> +l1:
> + while (a)
> + ;
> + bar ("");
> + asm goto ("" : : : : l2);
> + asm ("");
> +l2:
> + goto l1;
> +}
> +
> +void
> +qux (void)
> +{
> + asm goto ("" : : : : l1);
> + bar ("");
> + goto l1;
> +l1:
> + baz ("");
> +}
> +
> +void
> +corge (void)
> +{
> + asm goto ("" : : : : l1);
> + baz ("");
> +l2:
> + return;
> +l1:
> + bar ("");
> + goto l2;
> +}
>
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
