Skip to content

GH-135904: Add tests for the JIT build process #136766

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

brandtbucher
Copy link
Member

@brandtbucher brandtbucher commented Jul 18, 2025

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 generates redundant moves on 32-bit platforms 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.

@brandtbucher brandtbucher self-assigned this Jul 18, 2025
@brandtbucher brandtbucher added the tests Tests in the Lib/test dir label Jul 18, 2025
@brandtbucher brandtbucher requested a review from diegorusso as a code owner July 18, 2025 19:51
@brandtbucher brandtbucher added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 18, 2025
@bedevere-bot
Copy link

🤖 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.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants