Bug 119628 - Need better mechanisms to manage register saves in callee for tail calls (inc. preserve_none for x86_64?)
Summary: Need better mechanisms to manage register saves in callee for tail calls (inc...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 15.0
: P3 enhancement
Target Milestone: 16.0
Assignee: H.J. Lu
URL:
Keywords: missed-optimization, patch, tail-call
Depends on:
Blocks:
 
Reported: 2025-04-04 15:48 UTC by ak
Modified: 2025-06-29 02:26 UTC (History)
7 users (show)

See Also:
Host:
Target: x86_64
Build:
Known to work:
Known to fail:
Last reconfirmed: 2025-04-09 00:00:00


Attachments
toy byte code interpreter (417 bytes, text/plain)
2025-04-04 15:48 UTC, ak
Details
A patch (2.60 KB, patch)
2025-04-13 15:56 UTC, H.J. Lu
Details | Diff
A tested patch (7.69 KB, patch)
2025-04-14 23:32 UTC, H.J. Lu
Details | Diff
An untested patch (12.32 KB, patch)
2025-04-15 16:29 UTC, H.J. Lu
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description ak 2025-04-04 15:48:52 UTC
Created attachment 60997 [details]
toy byte code interpreter

To compile the test case use

One use case for the musttail feature is to write threaded interpreters with individual small functions each implementing an byte code and calling the next function in the byte code program using musttail. 

This is a replacement for an older code style that put all these byte code handlers into a large function and called them using indirect goto.

See the attached test case as an example.

This works fine for small functions that fit into the callee scratch registers in the x86-64 ABI. But when you have more complex functions that need more registers the individual functions starting saving/restoring the registers that are supposed to be callee saved (this is simulated using inline asm in the test case, thanks the Andrew Pinski for that trick)

You can see that in the case if you make the SAVE_REGS/DONT_SAVE_REGS empty, there are lots of extra push/pops on each opcode.

Now this can be changed by modifying the calling convention as it's done in the unmodified test case. The original caller of the byte code can save all and the rest of the tail called byte code functions none. LLVM has preserve_none/most/all for this and it is used in the field for this.

When the tail called functions are not called through pointers gcc has -fipa-ra for static functions, which should take care of it. But unfortunately this only works for direct calls because for indirects the IPA cgraph RTL mechanism doesn't work.

gcc has no_callee_saved_registers/no_caller_saved_registers which was originally developed for a different use case (fast interrupt handlers in OS) but can modify the callee registers saving. The main drawback of them is that they require -mgeneral-regs-only (as they were designed for an OS), which makes it impossible to use floating point in the interpreter code. While this works for the toy example it's probably a show stopper for real interpreters.

Another problem with them is that they don't affect the caller unlike the LLVM attributes. Luckily for the tail call case the shrink wrapping code takes care of this, although it's a problem if the byte code functions are called non tail for some reason (e.g. in the first function of the interpreter), a well as for other use cases (e.g. to use them to optimize calling of general cold functions)

gcc should:
- support no_callee_saved_registers/no_caller_saved_registers without -mgeneral-regs-only (there might be already bugs for this, but I'm filing it separately to track the particular use case)
- figure out how -fipa-ra can be made to work for indirects? (maybe with some type based analysis)
- Make the attributes affect the caller
- Do we need an equivalent of preserve_most
- Once no_callee/caller_saved_registers work similar to clang perhaps they should be aliased for compatibility.
Comment 1 H.J. Lu 2025-04-04 23:36:01 UTC
no_calle(e|r)_saved_registers=gpr(16|32)?
Comment 2 ak 2025-04-05 15:59:28 UTC
The existing attributes could just handle this case?
Comment 3 H.J. Lu 2025-04-05 18:08:01 UTC
(In reply to ak from comment #2)
> The existing attributes could just handle this case?

Caller needs to know what registers are saved by callee.  But caller doesn't
know what ISAs are used by callee.
Comment 4 H.J. Lu 2025-04-09 16:31:00 UTC
Another question, do we need to handle vector registers, xmm0-xmm16 vs xmm0-xmm31?
Comment 5 H.J. Lu 2025-04-13 15:56:17 UTC
Created attachment 61093 [details]
A patch

Please try this.
Comment 6 H.J. Lu 2025-04-13 16:34:28 UTC
no_caller_saved_registers only works with XMM and ZMM, not YMM, since YMM load
will clear the upper 256 bits of ZMM.
Comment 7 H.J. Lu 2025-04-13 22:37:12 UTC
Comment on attachment 61093 [details]
A patch

The current patches are at

https://gitlab.com/x86-gcc/gcc/-/tree/users/hjl/saved/master?ref_type=heads
Comment 8 H.J. Lu 2025-04-14 23:32:45 UTC
Created attachment 61120 [details]
A tested patch
Comment 9 Ken Jin 2025-04-15 07:12:53 UTC
I tried this out with CPython's interpreter that uses tail calls with the patch at https://gitlab.com/x86-gcc/gcc/-/tree/users/hjl/saved/master?ref_type=heads applied.

I get a roughly 10% speedup on the pystones benchmark:

Without preserve_none
This machine benchmarks at 912722 pystones/second

With preserve_none
This machine benchmarks at 1.02601e+06 pystones/second

(Higher is better).

I noticed it's still about 10% slower than clang-20 though. It's shuffling registers a lot at calls to external functions compared to Clang. Please see https://github.com/llvm/llvm-project/pull/88333. On GCC I get this with the patch applied:

a.out:	file format elf64-x86-64

Disassembly of section .text:

0000000000000000 <entry>:
       0: 55                           	pushq	%rbp
       1: 48 89 e5                     	movq	%rsp, %rbp
       4: 48 89 fb                     	movq	%rdi, %rbx
       7: 49 89 f4                     	movq	%rsi, %r12
       a: 49 89 d5                     	movq	%rdx, %r13
       d: 49 89 ce                     	movq	%rcx, %r14
      10: e8 00 00 00 00               	callq	0x15 <entry+0x15>
      15: 4c 89 f1                     	movq	%r14, %rcx
      18: 4c 89 ea                     	movq	%r13, %rdx
      1b: 4c 89 e6                     	movq	%r12, %rsi
      1e: 48 89 df                     	movq	%rbx, %rdi
      21: 5d                           	popq	%rbp
      22: e9 00 00 00 00               	jmp	0x27 <entry+0x27>
Comment 10 Drea Pinski 2025-04-15 07:34:20 UTC
(In reply to Ken Jin from comment #9)
> 0000000000000000 <entry>:
>        0: 55                           	pushq	%rbp
>        1: 48 89 e5                     	movq	%rsp, %rbp
>        4: 48 89 fb                     	movq	%rdi, %rbx
>        7: 49 89 f4                     	movq	%rsi, %r12
>        a: 49 89 d5                     	movq	%rdx, %r13
>        d: 49 89 ce                     	movq	%rcx, %r14
>       10: e8 00 00 00 00               	callq	0x15 <entry+0x15>
>       15: 4c 89 f1                     	movq	%r14, %rcx
>       18: 4c 89 ea                     	movq	%r13, %rdx
>       1b: 4c 89 e6                     	movq	%r12, %rsi
>       1e: 48 89 df                     	movq	%rbx, %rdi
>       21: 5d                           	popq	%rbp
>       22: e9 00 00 00 00               	jmp	0x27 <entry+0x27>

Note I am not sure if the move are the cause of the slow down though. Because on most recent (like over 10 years old now) Intel and AMD processors moves are handled during renaming and don't take up an issue/exec slot. I am thinking there are other things going on.
Comment 11 H.J. Lu 2025-04-15 13:04:02 UTC
(In reply to Ken Jin from comment #9)
> I tried this out with CPython's interpreter that uses tail calls with the
> patch at
> https://gitlab.com/x86-gcc/gcc/-/tree/users/hjl/saved/master?ref_type=heads
> applied.
> 
> I get a roughly 10% speedup on the pystones benchmark:
> 
> Without preserve_none
> This machine benchmarks at 912722 pystones/second
> 
> With preserve_none
> This machine benchmarks at 1.02601e+06 pystones/second
> 
> (Higher is better).
> 
> I noticed it's still about 10% slower than clang-20 though. It's shuffling
> registers a lot at calls to external functions compared to Clang. Please see
> https://github.com/llvm/llvm-project/pull/88333. On GCC I get this with the
> patch applied:
> 
> a.out:	file format elf64-x86-64
> 
> Disassembly of section .text:
> 
> 0000000000000000 <entry>:
>        0: 55                           	pushq	%rbp
>        1: 48 89 e5                     	movq	%rsp, %rbp
>        4: 48 89 fb                     	movq	%rdi, %rbx
>        7: 49 89 f4                     	movq	%rsi, %r12
>        a: 49 89 d5                     	movq	%rdx, %r13
>        d: 49 89 ce                     	movq	%rcx, %r14
>       10: e8 00 00 00 00               	callq	0x15 <entry+0x15>
>       15: 4c 89 f1                     	movq	%r14, %rcx
>       18: 4c 89 ea                     	movq	%r13, %rdx
>       1b: 4c 89 e6                     	movq	%r12, %rsi
>       1e: 48 89 df                     	movq	%rbx, %rdi
>       21: 5d                           	popq	%rbp
>       22: e9 00 00 00 00               	jmp	0x27 <entry+0x27>

Unlike Clang's preserve_none, preserve_none in my patch is an alias of
no_callee_saved_registers, which doesn't use caller-saved registers for
parameter passing.
Comment 12 H.J. Lu 2025-04-15 16:29:04 UTC
Created attachment 61124 [details]
An untested patch

I got

[hjl@gnu-tgl-3 no-callee]$ cat x.c
extern void boring(void);

extern void (continuation)(void *, void *, void *, void *)
  __attribute__((preserve_none));

__attribute__((preserve_none))
void entry(void *a, void *b, void *c, void *d)
{
  boring();
  continuation(a, b, c, d);
}
[hjl@gnu-tgl-3 no-callee]$ make x.s
/export/build/gnu/tools-build/gcc-gitlab-test-debug/build-x86_64-linux/gcc/xgcc -B/export/build/gnu/tools-build/gcc-gitlab-test-debug/build-x86_64-linux/gcc/ -O2 -mgeneral-regs-only -mtune-ctrl=prologue_using_move,epilogue_using_move -S x.c
[hjl@gnu-tgl-3 no-callee]$ cat x.s
	.file	"x.c"
	.text
	.p2align 4
	.globl	entry
	.type	entry, @function
entry:
.LFB0:
	.cfi_startproc
	subq	$8, %rsp
	.cfi_def_cfa_offset 16
	call	boring
	addq	$8, %rsp
	.cfi_def_cfa_offset 8
	jmp	continuation
	.cfi_endproc
.LFE0:
	.size	entry, .-entry
	.ident	"GCC: (GNU) 15.0.1 20250415 (experimental)"
	.section	.note.GNU-stack,"",@progbits
[hjl@gnu-tgl-3 no-callee]$
Comment 13 H.J. Lu 2025-04-15 16:29:40 UTC
(In reply to Ken Jin from comment #9)
> I tried this out with CPython's interpreter that uses tail calls with the
> patch at
> https://gitlab.com/x86-gcc/gcc/-/tree/users/hjl/saved/master?ref_type=heads
> applied.
> 
> I get a roughly 10% speedup on the pystones benchmark:
> 
> Without preserve_none
> This machine benchmarks at 912722 pystones/second
> 
> With preserve_none
> This machine benchmarks at 1.02601e+06 pystones/second
> 
> (Higher is better).
> 
> I noticed it's still about 10% slower than clang-20 though. It's shuffling
> registers a lot at calls to external functions compared to Clang. Please see
> https://github.com/llvm/llvm-project/pull/88333. On GCC I get this with the
> patch applied:
> 
> a.out:	file format elf64-x86-64
> 
> Disassembly of section .text:
> 
> 0000000000000000 <entry>:
>        0: 55                           	pushq	%rbp
>        1: 48 89 e5                     	movq	%rsp, %rbp
>        4: 48 89 fb                     	movq	%rdi, %rbx
>        7: 49 89 f4                     	movq	%rsi, %r12
>        a: 49 89 d5                     	movq	%rdx, %r13
>        d: 49 89 ce                     	movq	%rcx, %r14
>       10: e8 00 00 00 00               	callq	0x15 <entry+0x15>
>       15: 4c 89 f1                     	movq	%r14, %rcx
>       18: 4c 89 ea                     	movq	%r13, %rdx
>       1b: 4c 89 e6                     	movq	%r12, %rsi
>       1e: 48 89 df                     	movq	%rbx, %rdi
>       21: 5d                           	popq	%rbp
>       22: e9 00 00 00 00               	jmp	0x27 <entry+0x27>

Please try my latest patch.
Comment 14 Ken Jin 2025-04-16 02:37:33 UTC
No speedup (within noise) with latest patch over previous patch. So Andrew might be right there on the register shuffling.
However, note that pystones is just one benchmark in Python and not the
full benchmark suite we use (that takes very long to run), so I'm not sure if the results are fully representative. Though I have to apologize for a previous statement I made:

> I noticed it's still about 10% slower than clang-20 though.

This is only with LTO+PGO. With just LTO GCC 15 is faster than Clang 20!
Interestingly, PGO *slows down* GCC 15 with tail calls and preserve_none on CPython. It might instead
be a PGO and musttail missed optimization happening on GCC 15.

===

Results:

GCC15 with tail calls + NO preserve_none + LTO:
This machine benchmarks at 974700 pystones/second

GCC15 with tail calls + latest preserve_none patch + LTO:
This machine benchmarks at 1.02208e+06 pystones/second

GCC15 with tail calls + latest preserve_none patch + LTO + PGO:
This machine benchmarks at 917500 pystones/second

Clang-20 with tail calls + preserve_none + ThinLTO*:
This machine benchmarks at 962309 pystones/second

Clang-20 with tail calls + preserve_none + ThinLTO + PGO:
This machine benchmarks at 1.09019e+06 pystones/second

For reference:

GCC15 with indirect goto + LTO:
This machine benchmarks at 900922 pystones/second

GCC15 with indirect goto + LTO + PGO:
This machine benchmarks at 1.11972e+06 pystones/second

* ThinLTO is default clang policy for CPython

I tried another toy interpreter. From here https://github.com/brandtbucher/bf-dispatch-study/tree/main

Timings for GCC 15 WITHOUT preserve_none:
bf-tail:
3.22user 0.00system 0:03.22elapsed 99%CPU (0avgtext+0avgdata 1440maxresident)k
0inputs+0outputs (0major+119minor)pagefaults 0swaps

Timings for GCC 15 WITH preserve_none:
bf-tail:
3.39user 0.00system 0:03.39elapsed 99%CPU (0avgtext+0avgdata 1440maxresident)k
0inputs+0outputs (0major+121minor)pagefaults 0swaps


Finally, to make sure the results are quieter, I turned off Turbo Boost on my system:

GCC15 with tail calls + NO preserve_none + LTO:
This machine benchmarks at 496950 pystones/second

GCC15 with tail calls + latest preserve_none patch + LTO:
This machine benchmarks at 508273 pystones/second

===

In short, great work H.J!
preserve_none for the complex interpreter (CPython)
is a win (2-3%), for the simpler toy interpreter without any calls
to external functions, it's a loss (5%). This makes sense to me.
Note that for CPython, results without PGO are generally not as accurate,
as we found results varies quite abit without it.
Comment 15 Ken Jin 2025-04-16 09:03:49 UTC
I tested again this time with taskset, turbo boost off, on a quiet system, with PGO. These are the results. They're quite good:

# Indirect goto + LTO + PGO
This machine benchmarks at 576728 pystones/second

# Tail calls, no preserve_none + LTO + PGO*
This machine benchmarks at 539522 pystones/second

# Tail calls, preserve_none + LTO + PGO*
This machine benchmarks at 572234 pystones/second

So roughly a 6-7% gain from preserve_none on the pystones benchmark over no preserve_none. Thanks again H.J. for the patch.

*PGO is disabled for tail calling functions in the bytecode interpreter, but enabled for everything else, as it seems PGO slows down those functions. I used the attributes `no_instrument_function,no_profile_instrument_function` to turn it off for the bytecode functions.

Something strange is going on with PGO for tail calls on my system. However, I can't figure it out right now.

Everything is benchmarked on this branch https://github.com/Fidget-Spinner/cpython/pull/new/Fidget-Spinner:cpython:tail-call-gcc-3
Comment 17 GCC Commits 2025-06-26 04:55:26 UTC
The master branch has been updated by H.J. Lu <hjl@gcc.gnu.org>:

https://gcc.gnu.org/g:9804b23198b39f85a7258be556c5e8aed44b9efc

commit r16-1692-g9804b23198b39f85a7258be556c5e8aed44b9efc
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Sun Apr 13 11:38:24 2025 -0700

    x86: Add preserve_none and update no_caller_saved_registers attributes
    
    Add preserve_none attribute which is similar to no_callee_saved_registers
    attribute, except on x86-64, r12, r13, r14, r15, rdi and rsi registers are
    used for integer parameter passing.  This can be used in an interpreter
    to avoid saving/restoring the registers in functions which process byte
    codes.  It improved the pystones benchmark by 6-7%:
    
    https://gcc.gnu.org/bugzilla/show_bug.cgi?id=119628#c15
    
    Remove -mgeneral-regs-only restriction on no_caller_saved_registers
    attribute.  Only SSE is allowed since SSE XMM register load preserves
    the upper bits in YMM/ZMM register while YMM register load zeros the
    upper 256 bits of ZMM register, and preserving 32 ZMM registers can
    be quite expensive.
    
    gcc/
    
            PR target/119628
            * config/i386/i386-expand.cc (ix86_expand_call): Call
            ix86_type_no_callee_saved_registers_p instead of looking up
            no_callee_saved_registers attribute.
            * config/i386/i386-options.cc (ix86_set_func_type): Look up
            preserve_none attribute.  Check preserve_none attribute for
            interrupt attribute.  Don't check no_caller_saved_registers nor
            no_callee_saved_registers conflicts here.
            (ix86_set_func_type): Check no_callee_saved_registers before
            checking no_caller_saved_registers attribute.
            (ix86_set_current_function): Allow SSE with
            no_caller_saved_registers attribute.
            (ix86_handle_call_saved_registers_attribute): Check preserve_none,
            no_callee_saved_registers and no_caller_saved_registers conflicts.
            (ix86_gnu_attributes): Add preserve_none attribute.
            * config/i386/i386-protos.h (ix86_type_no_callee_saved_registers_p):
            New.
            * config/i386/i386.cc
            (x86_64_preserve_none_int_parameter_registers): New.
            (ix86_using_red_zone): Don't use red-zone when there are no
            caller-saved registers with SSE.
            (ix86_type_no_callee_saved_registers_p): New.
            (ix86_function_ok_for_sibcall): Also check TYPE_PRESERVE_NONE
            and call ix86_type_no_callee_saved_registers_p instead of looking
            up no_callee_saved_registers attribute.
            (ix86_comp_type_attributes): Call
            ix86_type_no_callee_saved_registers_p instead of looking up
            no_callee_saved_registers attribute.  Return 0 if preserve_none
            attribute doesn't match in 64-bit mode.
            (ix86_function_arg_regno_p): For cfun with TYPE_PRESERVE_NONE,
            use x86_64_preserve_none_int_parameter_registers.
            (init_cumulative_args): Set preserve_none_abi.
            (function_arg_64): Use x86_64_preserve_none_int_parameter_registers
            with preserve_none attribute.
            (setup_incoming_varargs_64): Use
            x86_64_preserve_none_int_parameter_registers with preserve_none
            attribute.
            (ix86_save_reg): Treat TYPE_PRESERVE_NONE like
            TYPE_NO_CALLEE_SAVED_REGISTERS.
            (ix86_nsaved_sseregs): Allow saving XMM registers for
            no_caller_saved_registers attribute.
            (ix86_compute_frame_layout): Likewise.
            (x86_this_parameter): Use
            x86_64_preserve_none_int_parameter_registers with preserve_none
            attribute.
            * config/i386/i386.h (ix86_args): Add preserve_none_abi.
            (call_saved_registers_type): Add TYPE_PRESERVE_NONE.
            (machine_function): Change call_saved_registers to 3 bits.
            * doc/extend.texi: Add preserve_none attribute.  Update
            no_caller_saved_registers attribute to remove -mgeneral-regs-only
            restriction.
    
    gcc/testsuite/
    
            PR target/119628
            * gcc.target/i386/no-callee-saved-3.c: Adjust error location.
            * gcc.target/i386/no-callee-saved-19a.c: New test.
            * gcc.target/i386/no-callee-saved-19b.c: Likewise.
            * gcc.target/i386/no-callee-saved-19c.c: Likewise.
            * gcc.target/i386/no-callee-saved-19d.c: Likewise.
            * gcc.target/i386/no-callee-saved-19e.c: Likewise.
            * gcc.target/i386/preserve-none-1.c: Likewise.
            * gcc.target/i386/preserve-none-2.c: Likewise.
            * gcc.target/i386/preserve-none-3.c: Likewise.
            * gcc.target/i386/preserve-none-4.c: Likewise.
            * gcc.target/i386/preserve-none-5.c: Likewise.
            * gcc.target/i386/preserve-none-6.c: Likewise.
            * gcc.target/i386/preserve-none-7.c: Likewise.
            * gcc.target/i386/preserve-none-8.c: Likewise.
            * gcc.target/i386/preserve-none-9.c: Likewise.
            * gcc.target/i386/preserve-none-10.c: Likewise.
            * gcc.target/i386/preserve-none-11.c: Likewise.
            * gcc.target/i386/preserve-none-12.c: Likewise.
            * gcc.target/i386/preserve-none-13.c: Likewise.
            * gcc.target/i386/preserve-none-14.c: Likewise.
            * gcc.target/i386/preserve-none-15.c: Likewise.
            * gcc.target/i386/preserve-none-16.c: Likewise.
            * gcc.target/i386/preserve-none-17.c: Likewise.
            * gcc.target/i386/preserve-none-18.c: Likewise.
            * gcc.target/i386/preserve-none-19.c: Likewise.
            * gcc.target/i386/preserve-none-20.c: Likewise.
            * gcc.target/i386/preserve-none-21.c: Likewise.
            * gcc.target/i386/preserve-none-22.c: Likewise.
            * gcc.target/i386/preserve-none-23.c: Likewise.
            * gcc.target/i386/preserve-none-24.c: Likewise.
            * gcc.target/i386/preserve-none-25.c: Likewise.
            * gcc.target/i386/preserve-none-26.c: Likewise.
            * gcc.target/i386/preserve-none-27.c: Likewise.
            * gcc.target/i386/preserve-none-28.c: Likewise.
            * gcc.target/i386/preserve-none-29.c: Likewise.
            * gcc.target/i386/preserve-none-30a.c: Likewise.
            * gcc.target/i386/preserve-none-30b.c: Likewise.
    
    Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
Comment 18 H.J. Lu 2025-06-26 04:56:08 UTC
Fixed for GCC 16.
Comment 19 Florian Weimer 2025-06-26 08:02:54 UTC
Note that this implementation of the preserve_none attribute is incompatible with Clang.
Comment 20 H.J. Lu 2025-06-26 08:18:58 UTC
(In reply to Florian Weimer from comment #19)
> Note that this implementation of the preserve_none attribute is incompatible
> with Clang.

This isn't a critical issue since the same compiler should be used
to compile all sources with preserve_none attribute.
Comment 21 H.J. Lu 2025-06-26 09:59:02 UTC
(In reply to Ken Jin from comment #15)
> I tested again this time with taskset, turbo boost off, on a quiet system,
> with PGO. These are the results. They're quite good:
> 
> # Indirect goto + LTO + PGO
> This machine benchmarks at 576728 pystones/second
> 
> # Tail calls, no preserve_none + LTO + PGO*
> This machine benchmarks at 539522 pystones/second
> 
> # Tail calls, preserve_none + LTO + PGO*
> This machine benchmarks at 572234 pystones/second
> 
> So roughly a 6-7% gain from preserve_none on the pystones benchmark over no
> preserve_none. Thanks again H.J. for the patch.
> 
> *PGO is disabled for tail calling functions in the bytecode interpreter, but
> enabled for everything else, as it seems PGO slows down those functions. I
> used the attributes `no_instrument_function,no_profile_instrument_function`
> to turn it off for the bytecode functions.
> 
> Something strange is going on with PGO for tail calls on my system. However,
> I can't figure it out right now.
> 
> Everything is benchmarked on this branch
> https://github.com/Fidget-Spinner/cpython/pull/new/Fidget-Spinner:cpython:
> tail-call-gcc-3

Hi Ken, my patch has been merged into GCC master branch.  Can you give it a try?
Comment 22 Ken Jin 2025-06-26 14:21:04 UTC
Hi H.J,

Thanks a lot for your work on this. I get a crash due to a possible miscompile on the latest GCC commit (7c67f7f8d4c8aadbe8efd733c29d13bfcbb0f50f).

Unfortunately, I cannot create a minimal reproducer right now, but something strange is going at the boundary where a normal function calls a preserve_none function, and where a preserve_none function returns back to the non preserve_none function. An interesting observation: passing `-fno-omit-frame-pointer -mno-omit-leaf-frame-pointer` seems to produce a working binary that doesn't crash.

The call stack at the crash is something like _PyEval_EvalFrameDefault (not tail call, not preserve_none) -> _TAIL_CALL_start_frame (preserve_none) -> (indeterminate number of tail calls, preserve_none) -> _TAIL_CALL_INTERPRETER_EXIT (preserve_none)

_TAIL_CALL_INTERPRETER_EXIT function contains a bare return out of the tail call sequence.
Comment 23 Ken Jin 2025-06-26 15:09:25 UTC
> Hi Ken, my patch has been merged into GCC master branch.  Can you give it a try?

I did a bench, note that this is not 100% what we use in CPython release builds, as I had to pass `-fno-omit-frame-pointer -mno-omit-leaf-frame-pointer` to all my configurations to get the main branch of GCC to not miscompile the current code.

LTO+PGO enabled for all configurations, disabled PGO only around tail call bytecode handlers as it regressed performance for those. Intel Turbo boost off.

NO preserve_none:
Pystone(1.1) time for 1000000 passes = 1.98081
This machine benchmarks at 504844 pystones/second

preserve_none:
Pystone(1.1) time for 1000000 passes = 1.7661
This machine benchmarks at 566219 pystones/second

I also took some benchmarks from the pyperformance benchmark suite that are Python-heavy. Specifically, nbody, spectral_norm, and deltablue.

Mean +- std dev: [NO_preserve_none_nbody] 108 ms +- 2 ms -> [preserve_none_nbody] 95.3 ms +- 2.0 ms: 1.13x faster
Mean +- std dev: [NO_preserve_none_spectralnorm] 95.7 ms +- 0.4 ms -> [preserve_none_spectralnorm] 83.8 ms +- 0.3 ms: 1.14x faster
Mean +- std dev: [NO_preserve_none_deltablue] 3.59 ms +- 0.03 ms -> [preserve_none_deltablue] 3.24 ms +- 0.02 ms: 1.11x faster

So seems like the actual speedup is the ~10% range for preserve_none vs no_preserve_none.

On my system, labels-as-values (indirect goto) performs roughly same as preserve_none + tail calls. However, note that PGO is disabled for the tail call handlers, and CPython has been optimizing for indirect goto style for over 10 years! So the fact the performance matches is actually incredibly good.
Comment 24 H.J. Lu 2025-06-26 21:23:18 UTC
(In reply to Ken Jin from comment #22)
> Hi H.J,
> 
> Thanks a lot for your work on this. I get a crash due to a possible
> miscompile on the latest GCC commit
> (7c67f7f8d4c8aadbe8efd733c29d13bfcbb0f50f).
> 
> Unfortunately, I cannot create a minimal reproducer right now, but something
> strange is going at the boundary where a normal function calls a
> preserve_none function, and where a preserve_none function returns back to
> the non preserve_none function. An interesting observation: passing
> `-fno-omit-frame-pointer -mno-omit-leaf-frame-pointer` seems to produce a
> working binary that doesn't crash.
> 
> The call stack at the crash is something like _PyEval_EvalFrameDefault (not
> tail call, not preserve_none) -> _TAIL_CALL_start_frame (preserve_none) ->
> (indeterminate number of tail calls, preserve_none) ->
> _TAIL_CALL_INTERPRETER_EXIT (preserve_none)
> 
> _TAIL_CALL_INTERPRETER_EXIT function contains a bare return out of the tail
> call sequence.

How do I reproduce it?  Do you have a step-by-step guide?  Thanks.
Comment 25 Sam James 2025-06-27 03:57:33 UTC
Let's carry on in a new bug: PR120840.