GH-135904: Add tests for the JIT build process by brandtbucher · Pull Request #136766 · python/cpython

brandtbucher · 2025-07-18T19:51:57Z

This is overdue, but it's especially valuable now that we're doing more subtle transformations on the machine code at JIT build time. This has a number of benefits:

When we add a new optimization at this level, we can directly see the impact on example code in the PR diff. And if it doesn't show up, we can add a test for it.
It gives us visibility into the quality of the generated code (without having the full stencils checked in yet). For example, making this PR helped me realize that there are three obvious quality issues on platforms I don't look at much:
- On some platforms, system headers were putting unused writable data in our read-only data stencils. This is both wrong and wasteful. I fixed this issue as part of this PR (we just optimistically remove this data, and raise if it's actually used).
- The JIT still inserts frame pointer pushes/pops on Intel-based Macs. I'll fix this in a follow-up PR.
- LLVM has a bug that inserts unnecessary spills and reloads of C local variables on 32-bit Windows at the beginning and end of every stencil. I've filed an issue upstream: musttail doesn't optimize sibling calls llvm/llvm-project#147813

The new test is in Lib/test/test_jit_stencils.py. This test uses Tools/jit/test/test_executor_cases.c.h to generate a few small test stencils, and compares them to the expected output in the corresponding Tools/jit/test/test_jit_stencils-*.h file.

Most of the changes in Tools/jit/_targets.py are just cleanup to make the generated stencils simpler and more consistent, or to fix the writable-data bug I mentioned above.

Issue: Optimize the JIT's low-level assembly control flow #135904

…nto jit-tests

bedevere-bot · 2025-07-18T20:38:09Z

🤖 New build scheduled with the buildbot fleet by @brandtbucher for commit 867a686 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F136766%2Fmerge

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

brandtbucher · 2025-07-21T19:28:31Z

!buildbot aarch64 RHEL8 LTO

bedevere-bot · 2025-07-21T19:28:34Z

🤖 New build scheduled with the buildbot fleet by @brandtbucher for commit 7c9c3ff 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F136766%2Fmerge

The command will test the builders whose names match following regular expression: aarch64 RHEL8 LTO

The builders matched are:

aarch64 RHEL8 LTO PR
aarch64 RHEL8 LTO + PGO PR

Lib/test/test_jit_stencils.py

tomasr8 · 2025-07-21T21:08:19Z

Lib/test/test_jit_stencils.py

+class TestJITStencils(unittest.TestCase):
+
+    def _build_jit_stencils(self, target: str) -> str:
+        with tempfile.TemporaryDirectory() as work:


I think os_helper.temp_dir is a bit more common in tests but either is fine :)

Hm, I'll take a look!

Is it, like, better? Haha.

Couldn't tell you 😅 but it seems quite common. Though both are used so it probably doesn't matter :)

savannahostrowski · 2025-07-21T23:00:28Z

Tools/jit/build.py

+    parser.add_argument(
+        "-i",
+        "--input-file",
+        help="where to find the generated executor cases",


Do you think it's worth calling out that this is used to pass in the dummy file in our tests and/or that this could be useful for custom builds/debugging? Maybe not in help text but in a comment?

I think I left a similar comment at the top of the PR. Is this the case of regenerating the file while in development so then be able to pass it?

diegorusso

This is not fully related to this PR but sometime I change this line https://github.com/python/cpython/pull/136766/files#diff-700c6057d24addce74e8787edb5f5556f718299f6ef1742cbb1d79580cdb535cR199 and add delete=False because I want to inspect the temporary files that have been generated.
Do you think that we should expose this to the user (it would be useful for me)?

diegorusso · 2025-07-22T12:20:32Z

Lib/test/test_jit_stencils.py

+            pyconfig_h = pathlib.Path(sysconfig.get_config_h_filename()).resolve()
+            result, args = test.support.script_helper.run_python_until_end(
+                _TOOLS_JIT_BUILD_PY,
+                "--input-file", _TOOLS_JIT_TEST_TEST_EXECUTOR_CASES_C_H,


This is just for my understanding. is this needed in case we need to regenerate this during development and we need to test it, right?

diegorusso · 2025-07-22T12:44:06Z

Tools/jit/_stencils.py

-    HoleValue.JUMP_TARGET: "state->instruction_starts[instruction->jump_target]",
-    HoleValue.ERROR_TARGET: "state->instruction_starts[instruction->error_target]",
+    # These should all have raised an error if they were actually used:
+    # HoleValue.WRITABLE: "",


I'm not sure I fully understand this. Can you expland please?

diegorusso · 2025-07-22T12:54:41Z

Tools/jit/build.py

+    parser.add_argument(
+        "-i",
+        "--input-file",
+        help="where to find the generated executor cases",


I think I left a similar comment at the top of the PR. Is this the case of regenerating the file while in development so then be able to pass it?

diegorusso · 2025-07-22T13:02:58Z

Most of the changes in Tools/jit/_targets.py are just cleanup to make the generated stencils simpler and more consistent, or to fix the writable-data bug I mentioned above.

It's not a hard push back, but shouldn't be these changes go to a separate PR?

savannahostrowski

Ah, just noticed this was still open and I don't see any reason why it wouldn't still be valuable! I think the main thing outstanding was Diego's comment (#136766 (comment)). Otherwise, I'd be happy with an additional comment about the --input-file being used for tests and merging main back in. @brandtbucher I know you're busy these days; do you need/want help carrying this over the line?

brandtbucher and others added 12 commits July 15, 2025 09:35

Test the JIT stencils build process

7a6d819

Cleanup

a322ad4

Add expected output for Windows

e1eb85d

Add expected output for macOS

f4c05b3

Is it BSS?

00cd7e3

It *is* BSS!

202fb6f

Optimistically strip writable data

4e5554c

Ditto for ELF

9db0563

Ditto for Mach-O, and *way* simplify the parsing!

c4c3ccc

Rework tests

c6fc7bd

Merge branch 'jit-tests' of https://github.com/brandtbucher/cpython i…

558c83b

…nto jit-tests

Cleanup for PR

867a686

brandtbucher self-assigned this Jul 18, 2025

brandtbucher requested a review from savannahostrowski as a code owner July 18, 2025 19:51

brandtbucher added the tests Tests in the Lib/test dir label Jul 18, 2025

brandtbucher requested a review from diegorusso as a code owner July 18, 2025 19:51

brandtbucher added skip news topic-JIT labels Jul 18, 2025

bedevere-app bot added the awaiting core review label Jul 18, 2025

bedevere-app bot mentioned this pull request Jul 18, 2025

Optimize the JIT's low-level assembly control flow #135904

Closed

brandtbucher added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 18, 2025

bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 18, 2025

Check for all of the tools

7c9c3ff

tomasr8 self-requested a review July 21, 2025 19:56

tomasr8 reviewed Jul 21, 2025

View reviewed changes

savannahostrowski reviewed Jul 21, 2025

View reviewed changes

diegorusso reviewed Jul 22, 2025

View reviewed changes

savannahostrowski reviewed Feb 19, 2026

View reviewed changes

Uh oh!

Conversation

brandtbucher commented Jul 18, 2025 • edited by bedevere-app bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bedevere-bot commented Jul 18, 2025

Uh oh!

brandtbucher commented Jul 21, 2025

Uh oh!

bedevere-bot commented Jul 21, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

diegorusso left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

diegorusso commented Jul 22, 2025

Uh oh!

savannahostrowski left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

brandtbucher commented Jul 18, 2025 •

edited by bedevere-app bot

Loading

savannahostrowski left a comment •

edited

Loading