Make no-opt JIT to pass whole unit test. · Issue #137838 · python/cpython · GitHub

#137800 (comment)

While investigating, make JIT thread-safe for FT build.
IMO, we should separate this task into making the tier 2 interpreter thread-safe (with optimzation disabled) and the optimizer thread-safe.

But we notice that the current no-opt JIT at the default build does not pass the whole unittest.
Let's make no-opt JIT the default build pass, whole unit tests.

I plan to investigate the issue, please let me know if I should take care of something since I am not an expert on this topic.

cc @Fidget-Spinner @savannahostrowski @brandtbucher @diegorusso

Linked PRs

$> PYTHON_UOPS_OPTIMIZE=0 lldb ./python.exe
(lldb) r -m test test_zoneinfo
Process 92170 launched: '/Users/denny.na/oss/cpython/python.exe' (arm64)
Raised RLIMIT_NOFILE: 256 -> 1024
Using random seed: 199945185
0:00:00 load avg: 6.25 Run 1 test sequentially in a single process
0:00:00 load avg: 6.25 [1/1] test_zoneinfo
Process 92170 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = signal SIGABRT
    frame #0: 0x0000000186719388 libsystem_kernel.dylib`__pthread_kill + 8
libsystem_kernel.dylib`__pthread_kill:
->  0x186719388 <+8>:  b.lo   0x1867193a8    ; <+40>
    0x18671938c <+12>: pacibsp
    0x186719390 <+16>: stp    x29, x30, [sp, #-0x10]!
    0x186719394 <+20>: mov    x29, sp
(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason = signal SIGABRT
  * frame #0: 0x0000000186719388 libsystem_kernel.dylib`__pthread_kill + 8
    frame #1: 0x000000018675288c libsystem_pthread.dylib`pthread_kill + 296
    frame #2: 0x000000018665bcf0 libsystem_c.dylib`__abort + 132
    frame #3: 0x000000018664caec libsystem_c.dylib`__stack_chk_fail + 96
    frame #4: 0x00000001001fd774 python.exe`_PyOptimizer_Optimize(frame=<unavailable>, start=<unavailable>, executor_ptr=<unavailable>, chain_depth=<unavailable>) at refcount.h:0:5 [opt]

   1319	    length = prepare_for_execution(buffer, length);
-> 1320	    assert(length <= UOP_MAX_TRACE_LENGTH);

@Fidget-Spinner I didn't take a look at uop_trace deeply inside(which codes are generated, something like that), but simply increased UOP_MAX_TRACE_LENGTH to 1600, specifically failed test code, but it will not be the answer.
Possibly unoptimized traced code emits too much long trace code?

The problem is that prepare_for_execution inserts more exit stubs (_EXIT_TRACE, _DEOPT, _ERROR_N) that overrun the buffer.

Basically the flow is like this:

translate_bytecode_to_trace (uops without exit stubs)
optimize (shortens the uops)
prepare_for_execution (inserts the exit stubs)

The problem is that translate_bytecode_to_trace seems to not be properly reserving enough space in the uop buffer for the number of possible exit stubs needed.

pythongh-137838: Fix JIT trace buffer overrun by pre-reserving exit s…

Revert "pythongh-137838: Fix JIT trace buffer overrun by pre-reservin…

pythongh-137838: Fix JIT trace buffer overrun by increasing possible …

Summary discussion w/ @Fidget-Spinner and CHIPS

Move _PyUOpInstruction buffer[UOP_MAX_TRACE_LENGTH] to _PyThreadStateImpl with doubling the size.
Fix calculation bug that @Fidget-Spinner found(?)

So there's a big problem right now that isn't easily solved: the calculation for side exit stubs to reserve when there's no optimizer and with optimizer is different. Let me explain:

Suppose you have the following sequence of uops, all decomposed from the same bytecode:

_CHECK_VALIDITY
_UOP_1
_UOP_2

The side exit calculation with optimizer on is pretty smart, just insert need two exit stubs, one for _CHECK_VALIDITY if needed, and one for the two _UOPS. The second exit stub can reused for the last two uops together.

Now suppose we have no optimizer, that means we have a lot of redundant _CHECK_VALIDITY in between:

_CHECK_VALIDITY
_UOP_1
_CHECK_VALIDITY
_UOP_2

Note that now, _UOP_1 and _UOP_2 cannot share the same exit stub using our naiive exit stub sharing algo in the prepare_execution function, they must have separate ones.

So the problem is:

When optimizer is on: side exit is per opcode as they will be shared across the uops
When the optimizer is off: side exit is per uop as they cannot be shared across the uops.

There is no easy way to solve it by refining our calculation. I think we should simply reserve more space or project a smaller trace by overestimating.

I think we should simply reserve more space or project a smaller trace by overestimating.

That would be my one suggstion, something like this: 917c6f0 at #138177

gh-137838: Fix JIT trace buffer overrun by increasing possible exit s…

pythongh-137838: Fix JIT trace buffer overrun by increasing possible …

Move _PyUOpInstruction buffer[UOP_MAX_TRACE_LENGTH] to _PyThreadStateImpl with doubling the size.

Plan to work on during the core sprint cc @Fidget-Spinner

pythongh-137838: Move _PyUOpInstruction buffer[UOP_MAX_TRACE_LENGTH] …

gh-137838: Move _PyUOpInstruction buffer to PyInterpreterState (gh-13…

pythongh-137838: Add CI for no-opt JIT

gh-137838: Add CI for no-opt JIT (#139081)

This was completed during the sprint! Thanks for the support, @Fidget-Spinner and @brandtbucher!

Make no-opt JIT to pass whole unit test. · Issue #137838 · python/cpython · GitHub

Repository navigation

Make no-opt JIT to pass whole unit test. #137838

Linked PRs

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Make no-opt JIT to pass whole unit test. · Issue #137838 · python/cpython · GitHub

Repository navigation

Make no-opt JIT to pass whole unit test. #137838

Description

Linked PRs

Activity

corona10 commented on Aug 15, 2025

corona10 commented on Aug 16, 2025

corona10 commented on Aug 16, 2025

Fidget-Spinner commented on Aug 16, 2025

corona10 commented on Aug 27, 2025

Fidget-Spinner commented on Aug 28, 2025

corona10 commented on Aug 28, 2025

corona10 commented on Sep 14, 2025

corona10 commented on Sep 18, 2025

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions