[1/4] cfgcleanup: keep tail merging out of an unfactored computed goto [PR127246]

Message ID 20261001162056.68977-1-ktkachov@nvidia.com
State New
Headers
Series [1/4] cfgcleanup: keep tail merging out of an unfactored computed goto [PR127246] |

Commit Message

Kyrylo Tkachov Oct. 1, 2026, 4:20 p.m. UTC
  From: Kyrylo Tkachov <ktkachov@nvidia.com>

bbcb0c056be (PR15242) stopped try_crossjump_bb from sharing the tails of
the predecessors of a computed jump unless optimizing for size.
pass_duplicate_computed_gotos copies the jump back into each predecessor
and a shared tail defeats that.  efd8f7507b3 replaced !optimize_size with
optimize_bb_for_size_p (bb), which inverted the test: tails are now shared
in code optimized for speed and kept apart in code optimized for size.

Test optimize_function_for_speed_p (cfun) instead.  PR42621 fixed the
bb-reorder half of the same change in the same way, in the gate of
pass_duplicate_computed_gotos.  As in bbcb0c056be, the guard then applies
to the whole function, also to a block that is too large to be unfactored.
try_head_merge_bb has a copy of the inverted test that cannot change code,
because every successor edge of a computed jump is abnormal.  It stays as
it is.

On the PR127246 testcase with identical handler tails, jump2 no longer
crossjumps the handlers at -O2 only for compgotos to copy the tails back.
At -Os the handler tails are shared again, which makes the aarch64 code of
the PR testcase 54% smaller.

Bootstrapped and regression tested on aarch64-unknown-linux-gnu and
x86_64-pc-linux-gnu.

Ok for trunk?
Thanks,
Kyrill

gcc/ChangeLog:

	PR rtl-optimization/127246
	* cfgcleanup.cc (try_crossjump_bb): Keep predecessors of a computed
	jump apart when the function, not the block, is optimized for speed.

gcc/testsuite/ChangeLog:

	PR rtl-optimization/127246
	* gcc.dg/pr127246-1.c: New test.
	* gcc.dg/pr127246-2.c: New test.

Signed-off-by: Kyrylo Tkachov <ktkachov@nvidia.com>
---
 gcc/cfgcleanup.cc                 |  2 +-
 gcc/testsuite/gcc.dg/pr127246-1.c | 48 +++++++++++++++++++++++++++++++
 gcc/testsuite/gcc.dg/pr127246-2.c | 12 ++++++++
 3 files changed, 61 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.dg/pr127246-1.c
 create mode 100644 gcc/testsuite/gcc.dg/pr127246-2.c
  

Comments

Richard Biener Oct. 2, 2026, 6:47 a.m. UTC | #1
On Thu, Oct 1, 2026 at 6:22 PM <ktkachov@nvidia.com> wrote:
>
> From: Kyrylo Tkachov <ktkachov@nvidia.com>
>
> bbcb0c056be (PR15242) stopped try_crossjump_bb from sharing the tails of
> the predecessors of a computed jump unless optimizing for size.
> pass_duplicate_computed_gotos copies the jump back into each predecessor
> and a shared tail defeats that.  efd8f7507b3 replaced !optimize_size with
> optimize_bb_for_size_p (bb), which inverted the test: tails are now shared
> in code optimized for speed and kept apart in code optimized for size.
>
> Test optimize_function_for_speed_p (cfun) instead.  PR42621 fixed the
> bb-reorder half of the same change in the same way, in the gate of
> pass_duplicate_computed_gotos.  As in bbcb0c056be, the guard then applies
> to the whole function, also to a block that is too large to be unfactored.
> try_head_merge_bb has a copy of the inverted test that cannot change code,
> because every successor edge of a computed jump is abnormal.  It stays as
> it is.
>
> On the PR127246 testcase with identical handler tails, jump2 no longer
> crossjumps the handlers at -O2 only for compgotos to copy the tails back.
> At -Os the handler tails are shared again, which makes the aarch64 code of
> the PR testcase 54% smaller.
>
> Bootstrapped and regression tested on aarch64-unknown-linux-gnu and
> x86_64-pc-linux-gnu.
>
> Ok for trunk?
> Thanks,
> Kyrill
>
> gcc/ChangeLog:
>
>         PR rtl-optimization/127246
>         * cfgcleanup.cc (try_crossjump_bb): Keep predecessors of a computed
>         jump apart when the function, not the block, is optimized for speed.
>
> gcc/testsuite/ChangeLog:
>
>         PR rtl-optimization/127246
>         * gcc.dg/pr127246-1.c: New test.
>         * gcc.dg/pr127246-2.c: New test.
>
> Signed-off-by: Kyrylo Tkachov <ktkachov@nvidia.com>
> ---
>  gcc/cfgcleanup.cc                 |  2 +-
>  gcc/testsuite/gcc.dg/pr127246-1.c | 48 +++++++++++++++++++++++++++++++
>  gcc/testsuite/gcc.dg/pr127246-2.c | 12 ++++++++
>  3 files changed, 61 insertions(+), 1 deletion(-)
>  create mode 100644 gcc/testsuite/gcc.dg/pr127246-1.c
>  create mode 100644 gcc/testsuite/gcc.dg/pr127246-2.c
>
> diff --git a/gcc/cfgcleanup.cc b/gcc/cfgcleanup.cc
> index 1d9ec908dab..7e5a0775e22 100644
> --- a/gcc/cfgcleanup.cc
> +++ b/gcc/cfgcleanup.cc
> @@ -2182,7 +2182,7 @@ try_crossjump_bb (int mode, basic_block bb)
>
>    /* Don't crossjump if this block ends in a computed jump,
>       unless we are optimizing for size.  */
> -  if (optimize_bb_for_size_p (bb)
> +  if (optimize_function_for_speed_p (cfun)

pass_duplicate_computed_gotos::gate has

  return (optimize > 0
          && flag_expensive_optimizations
          && ! optimize_function_for_size_p (fun));

so please use ! optimize_function_for_size_p (cfun) &&
flag_expensive_optimizations
and adjust the comment to mention the reason, duplicating computed gotos.

OK with that change.
Richard.

>        && bb != EXIT_BLOCK_PTR_FOR_FN (cfun)
>        && computed_jump_p (BB_END (bb)))
>      return false;
> diff --git a/gcc/testsuite/gcc.dg/pr127246-1.c b/gcc/testsuite/gcc.dg/pr127246-1.c
> new file mode 100644
> index 00000000000..cfea56d631d
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/pr127246-1.c
> @@ -0,0 +1,48 @@
> +/* PR rtl-optimization/127246 */
> +/* { dg-do compile } */
> +/* { dg-require-effective-target label_values } */
> +/* { dg-require-effective-target indirect_jumps } */
> +/* { dg-options "-O2 -fno-tree-slp-vectorize -fdump-rtl-jump2-details" } */
> +
> +/* pass_duplicate_computed_gotos copies the dispatch back into every handler,
> +   so tail merging must not share the identical handler tails in front of
> +   it.  */
> +
> +struct frame
> +{
> +  const unsigned short *instr_ptr;
> +  long *stackpointer;
> +};
> +
> +#define OPS(X) \
> +  X (0) X (1) X (2) X (3) X (4) X (5) X (6) X (7) X (8) X (9) X (10) \
> +  X (11) X (12) X (13) X (14) X (15) X (16) X (17) X (18) X (19)
> +
> +extern long op (long);
> +
> +#define DISPATCH() goto *targets[*next_instr & 0xff]
> +
> +#define HANDLER(N) \
> +  L_##N: \
> +    frame->instr_ptr = next_instr; \
> +    next_instr += 1; \
> +    frame->stackpointer = stack_pointer; \
> +    stack_pointer[-1] = op (N); \
> +    stack_pointer -= 1; \
> +    DISPATCH ();
> +
> +#define ENTRY(N) &&L_##N,
> +
> +void
> +interp (struct frame *frame)
> +{
> +  static const void *const targets[] = { OPS (ENTRY) };
> +  const unsigned short *next_instr = frame->instr_ptr;
> +  long *stack_pointer = frame->stackpointer;
> +
> +  DISPATCH ();
> +
> +  OPS (HANDLER)
> +}
> +
> +/* { dg-final { scan-rtl-dump-not "Cross jumping" "jump2" } } */
> diff --git a/gcc/testsuite/gcc.dg/pr127246-2.c b/gcc/testsuite/gcc.dg/pr127246-2.c
> new file mode 100644
> index 00000000000..02328f04eab
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/pr127246-2.c
> @@ -0,0 +1,12 @@
> +/* PR rtl-optimization/127246 */
> +/* { dg-do compile } */
> +/* { dg-require-effective-target label_values } */
> +/* { dg-require-effective-target indirect_jumps } */
> +/* { dg-options "-Os -fdump-rtl-jump2-details" } */
> +
> +/* When optimizing for size the computed goto is not unfactored, so tail
> +   merging the handler tails is what we want.  */
> +
> +#include "pr127246-1.c"
> +
> +/* { dg-final { scan-rtl-dump "Cross jumping" "jump2" } } */
> --
> 2.50.1 (Apple Git-155)
>
  
Kyrylo Tkachov Oct. 6, 2026, 8:16 a.m. UTC | #2
> On 2 Oct 2026, at 08:47, Richard Biener <richard.guenther@gmail.com> wrote:
> 
> On Thu, Oct 1, 2026 at 6:22 PM <ktkachov@nvidia.com> wrote:
>> 
>> From: Kyrylo Tkachov <ktkachov@nvidia.com>
>> 
>> bbcb0c056be (PR15242) stopped try_crossjump_bb from sharing the tails of
>> the predecessors of a computed jump unless optimizing for size.
>> pass_duplicate_computed_gotos copies the jump back into each predecessor
>> and a shared tail defeats that.  efd8f7507b3 replaced !optimize_size with
>> optimize_bb_for_size_p (bb), which inverted the test: tails are now shared
>> in code optimized for speed and kept apart in code optimized for size.
>> 
>> Test optimize_function_for_speed_p (cfun) instead.  PR42621 fixed the
>> bb-reorder half of the same change in the same way, in the gate of
>> pass_duplicate_computed_gotos.  As in bbcb0c056be, the guard then applies
>> to the whole function, also to a block that is too large to be unfactored.
>> try_head_merge_bb has a copy of the inverted test that cannot change code,
>> because every successor edge of a computed jump is abnormal.  It stays as
>> it is.
>> 
>> On the PR127246 testcase with identical handler tails, jump2 no longer
>> crossjumps the handlers at -O2 only for compgotos to copy the tails back.
>> At -Os the handler tails are shared again, which makes the aarch64 code of
>> the PR testcase 54% smaller.
>> 
>> Bootstrapped and regression tested on aarch64-unknown-linux-gnu and
>> x86_64-pc-linux-gnu.
>> 
>> Ok for trunk?
>> Thanks,
>> Kyrill
>> 
>> gcc/ChangeLog:
>> 
>>        PR rtl-optimization/127246
>>        * cfgcleanup.cc (try_crossjump_bb): Keep predecessors of a computed
>>        jump apart when the function, not the block, is optimized for speed.
>> 
>> gcc/testsuite/ChangeLog:
>> 
>>        PR rtl-optimization/127246
>>        * gcc.dg/pr127246-1.c: New test.
>>        * gcc.dg/pr127246-2.c: New test.
>> 
>> Signed-off-by: Kyrylo Tkachov <ktkachov@nvidia.com>
>> ---
>> gcc/cfgcleanup.cc                 |  2 +-
>> gcc/testsuite/gcc.dg/pr127246-1.c | 48 +++++++++++++++++++++++++++++++
>> gcc/testsuite/gcc.dg/pr127246-2.c | 12 ++++++++
>> 3 files changed, 61 insertions(+), 1 deletion(-)
>> create mode 100644 gcc/testsuite/gcc.dg/pr127246-1.c
>> create mode 100644 gcc/testsuite/gcc.dg/pr127246-2.c
>> 
>> diff --git a/gcc/cfgcleanup.cc b/gcc/cfgcleanup.cc
>> index 1d9ec908dab..7e5a0775e22 100644
>> --- a/gcc/cfgcleanup.cc
>> +++ b/gcc/cfgcleanup.cc
>> @@ -2182,7 +2182,7 @@ try_crossjump_bb (int mode, basic_block bb)
>> 
>>   /* Don't crossjump if this block ends in a computed jump,
>>      unless we are optimizing for size.  */
>> -  if (optimize_bb_for_size_p (bb)
>> +  if (optimize_function_for_speed_p (cfun)
> 
> pass_duplicate_computed_gotos::gate has
> 
>  return (optimize > 0
>          && flag_expensive_optimizations
>          && ! optimize_function_for_size_p (fun));
> 
> so please use ! optimize_function_for_size_p (cfun) &&
> flag_expensive_optimizations
> and adjust the comment to mention the reason, duplicating computed gotos.
> 
> OK with that change.

Thanks, here’s what I’m pushing.
Kyrill

> Richard.
> 
>>       && bb != EXIT_BLOCK_PTR_FOR_FN (cfun)
>>       && computed_jump_p (BB_END (bb)))
>>     return false;
>> diff --git a/gcc/testsuite/gcc.dg/pr127246-1.c b/gcc/testsuite/gcc.dg/pr127246-1.c
>> new file mode 100644
>> index 00000000000..cfea56d631d
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/pr127246-1.c
>> @@ -0,0 +1,48 @@
>> +/* PR rtl-optimization/127246 */
>> +/* { dg-do compile } */
>> +/* { dg-require-effective-target label_values } */
>> +/* { dg-require-effective-target indirect_jumps } */
>> +/* { dg-options "-O2 -fno-tree-slp-vectorize -fdump-rtl-jump2-details" } */
>> +
>> +/* pass_duplicate_computed_gotos copies the dispatch back into every handler,
>> +   so tail merging must not share the identical handler tails in front of
>> +   it.  */
>> +
>> +struct frame
>> +{
>> +  const unsigned short *instr_ptr;
>> +  long *stackpointer;
>> +};
>> +
>> +#define OPS(X) \
>> +  X (0) X (1) X (2) X (3) X (4) X (5) X (6) X (7) X (8) X (9) X (10) \
>> +  X (11) X (12) X (13) X (14) X (15) X (16) X (17) X (18) X (19)
>> +
>> +extern long op (long);
>> +
>> +#define DISPATCH() goto *targets[*next_instr & 0xff]
>> +
>> +#define HANDLER(N) \
>> +  L_##N: \
>> +    frame->instr_ptr = next_instr; \
>> +    next_instr += 1; \
>> +    frame->stackpointer = stack_pointer; \
>> +    stack_pointer[-1] = op (N); \
>> +    stack_pointer -= 1; \
>> +    DISPATCH ();
>> +
>> +#define ENTRY(N) &&L_##N,
>> +
>> +void
>> +interp (struct frame *frame)
>> +{
>> +  static const void *const targets[] = { OPS (ENTRY) };
>> +  const unsigned short *next_instr = frame->instr_ptr;
>> +  long *stack_pointer = frame->stackpointer;
>> +
>> +  DISPATCH ();
>> +
>> +  OPS (HANDLER)
>> +}
>> +
>> +/* { dg-final { scan-rtl-dump-not "Cross jumping" "jump2" } } */
>> diff --git a/gcc/testsuite/gcc.dg/pr127246-2.c b/gcc/testsuite/gcc.dg/pr127246-2.c
>> new file mode 100644
>> index 00000000000..02328f04eab
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/pr127246-2.c
>> @@ -0,0 +1,12 @@
>> +/* PR rtl-optimization/127246 */
>> +/* { dg-do compile } */
>> +/* { dg-require-effective-target label_values } */
>> +/* { dg-require-effective-target indirect_jumps } */
>> +/* { dg-options "-Os -fdump-rtl-jump2-details" } */
>> +
>> +/* When optimizing for size the computed goto is not unfactored, so tail
>> +   merging the handler tails is what we want.  */
>> +
>> +#include "pr127246-1.c"
>> +
>> +/* { dg-final { scan-rtl-dump "Cross jumping" "jump2" } } */
>> --
>> 2.50.1 (Apple Git-155)
  

Patch

diff --git a/gcc/cfgcleanup.cc b/gcc/cfgcleanup.cc
index 1d9ec908dab..7e5a0775e22 100644
--- a/gcc/cfgcleanup.cc
+++ b/gcc/cfgcleanup.cc
@@ -2182,7 +2182,7 @@  try_crossjump_bb (int mode, basic_block bb)
 
   /* Don't crossjump if this block ends in a computed jump,
      unless we are optimizing for size.  */
-  if (optimize_bb_for_size_p (bb)
+  if (optimize_function_for_speed_p (cfun)
       && bb != EXIT_BLOCK_PTR_FOR_FN (cfun)
       && computed_jump_p (BB_END (bb)))
     return false;
diff --git a/gcc/testsuite/gcc.dg/pr127246-1.c b/gcc/testsuite/gcc.dg/pr127246-1.c
new file mode 100644
index 00000000000..cfea56d631d
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr127246-1.c
@@ -0,0 +1,48 @@ 
+/* PR rtl-optimization/127246 */
+/* { dg-do compile } */
+/* { dg-require-effective-target label_values } */
+/* { dg-require-effective-target indirect_jumps } */
+/* { dg-options "-O2 -fno-tree-slp-vectorize -fdump-rtl-jump2-details" } */
+
+/* pass_duplicate_computed_gotos copies the dispatch back into every handler,
+   so tail merging must not share the identical handler tails in front of
+   it.  */
+
+struct frame
+{
+  const unsigned short *instr_ptr;
+  long *stackpointer;
+};
+
+#define OPS(X) \
+  X (0) X (1) X (2) X (3) X (4) X (5) X (6) X (7) X (8) X (9) X (10) \
+  X (11) X (12) X (13) X (14) X (15) X (16) X (17) X (18) X (19)
+
+extern long op (long);
+
+#define DISPATCH() goto *targets[*next_instr & 0xff]
+
+#define HANDLER(N) \
+  L_##N: \
+    frame->instr_ptr = next_instr; \
+    next_instr += 1; \
+    frame->stackpointer = stack_pointer; \
+    stack_pointer[-1] = op (N); \
+    stack_pointer -= 1; \
+    DISPATCH ();
+
+#define ENTRY(N) &&L_##N,
+
+void
+interp (struct frame *frame)
+{
+  static const void *const targets[] = { OPS (ENTRY) };
+  const unsigned short *next_instr = frame->instr_ptr;
+  long *stack_pointer = frame->stackpointer;
+
+  DISPATCH ();
+
+  OPS (HANDLER)
+}
+
+/* { dg-final { scan-rtl-dump-not "Cross jumping" "jump2" } } */
diff --git a/gcc/testsuite/gcc.dg/pr127246-2.c b/gcc/testsuite/gcc.dg/pr127246-2.c
new file mode 100644
index 00000000000..02328f04eab
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr127246-2.c
@@ -0,0 +1,12 @@ 
+/* PR rtl-optimization/127246 */
+/* { dg-do compile } */
+/* { dg-require-effective-target label_values } */
+/* { dg-require-effective-target indirect_jumps } */
+/* { dg-options "-Os -fdump-rtl-jump2-details" } */
+
+/* When optimizing for size the computed goto is not unfactored, so tail
+   merging the handler tails is what we want.  */
+
+#include "pr127246-1.c"
+
+/* { dg-final { scan-rtl-dump "Cross jumping" "jump2" } } */