Skip to content

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

Closed
Closed
Make no-opt JIT to pass whole unit test.#137838
Assignees
corona10
Labels
interpreter-core(Objects, Python, Grammar, and Parser dirs)sprinttopic-JIT

Description

@corona10
corona10
opened on Aug 15, 2025

#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

Activity

  1. corona10
    self-assigned this
    on Aug 15, 2025
  2. corona10
    added
    topic-JIT
    on Aug 15, 2025

  3. corona10 commented on Aug 15, 2025

    @corona10
    corona10
    on Aug 15, 2025
    MemberAuthor
    $> 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]

  4. corona10 commented on Aug 16, 2025

    @corona10
    corona10
    on Aug 16, 2025
    MemberAuthor
       1319	    length = prepare_for_execution(buffer, length);
-> 1320	    assert(length <= UOP_MAX_TRACE_LENGTH);

  5. corona10 commented on Aug 16, 2025

    @corona10
    corona10
    on Aug 16, 2025
    MemberAuthor

    @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?

  6. Fidget-Spinner commented on Aug 16, 2025

    @Fidget-Spinner
    Fidget-Spinner
    on Aug 16, 2025
    Member

    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:

    1. translate_bytecode_to_trace (uops without exit stubs)
    2. optimize (shortens the uops)
    3. 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.

  7. picnixz
    added
    interpreter-core(Objects, Python, Grammar, and Parser dirs)
    on Aug 16, 2025
  8. corona10
    added 3 commits that reference this issue on Aug 27, 2025

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

    0776a59

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

    dbc5d9a

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

    31a5157

  9. corona10 commented on Aug 27, 2025

    @corona10
    corona10
    on Aug 27, 2025
    MemberAuthor

    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(?)

  10. Fidget-Spinner commented on Aug 28, 2025

    @Fidget-Spinner
    Fidget-Spinner
    on Aug 28, 2025
    Member

    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.

  11. corona10 commented on Aug 28, 2025

    @corona10
    corona10
    on Aug 28, 2025
    MemberAuthor

    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

  12. corona10
    added a commit that references this issue on Sep 9, 2025

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

    5edfe55
  13. lkollar
    added a commit that references this issue on Sep 9, 2025

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

    5239077

  14. corona10 commented on Sep 14, 2025

    @corona10
    corona10
    on Sep 14, 2025
    MemberAuthor

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

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

  15. corona10
    added 5 commits that reference this issue on Sep 15, 2025

    pythongh-137838: Move _PyUOpInstruction buffer[UOP_MAX_TRACE_LENGTH] …

    fe3c1af

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

    d873fb4

    pythongh-137838: Add CI for no-opt JIT

    71dbbd1

    pythongh-137838: Add CI for no-opt JIT

    74b95b7

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

    8247e1b
  16. corona10
    closed this as completedon Sep 18, 2025

  17. corona10 commented on Sep 18, 2025

    @corona10
    corona10
    on Sep 18, 2025
    MemberAuthor

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

  18. corona10
    added
    sprint
    on Sep 18, 2025
  19. ezio-melotti
    added this to Sprinton Sep 18, 2025
  20. github-project-automation
    moved this to Todo in Sprinton Sep 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

interpreter-core(Objects, Python, Grammar, and Parser dirs)sprinttopic-JIT

Projects

  • Sprint
    Status
    Todo

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions