Skip to content

GH-114809: Add support for macOS multi-arch builds with the JIT enabled #131751

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

Merged
merged 27 commits into from
Apr 30, 2025

Conversation

savannahostrowski
Copy link
Member

@savannahostrowski savannahostrowski commented Mar 26, 2025

This work patches up #115742.

@savannahostrowski
Copy link
Member Author

savannahostrowski commented Mar 26, 2025

Hmmmmm, will investigate why everything is failing tomorrow.

"It worked on my machine", a tale as old as time.
"I typo'd and left out a single character causing every job to fail", a tale as old as time

Copy link
Member

@brandtbucher brandtbucher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, looks good! Just the few things we discussed offline:

  • Maybe trying to get this in CI.
  • Indentation nitpick for preprocessor directives in jit_stencils.h.
  • The possibility of just using the second code path in build.py for single-arch builds too.

savannahostrowski and others added 2 commits April 11, 2025 12:44
Co-authored-by: Brandt Bucher <brandtbucher@gmail.com>
@ned-deily
Copy link
Member

@savannahostrowski Thanks for working on this! I'll review and test this in the next day or two.

@brandtbucher
Copy link
Member

Thanks @ned-deily! It would be great if we could have the JIT built-but-off (--enable-experimental-jit=yes-off) in the 3.14 betas like we talked about at the sprint, and this is an important step towards that... since I think the release builds are Universal2, right?

The remaining tests seem to be failing because the external introspection and remote exec features doesn't work on fat builds, and this PR switches the JIT CI over to Universal2 to make sure we don't break releases. The existing tests don't skip themselves on these builds, and are just failing.

@pablogsal, is it intended that fat builds aren't supported by the internal introspection and remote exec features? The error message we're running into here says 32-bit Mach-O binaries are not supported, which leads me to think this is just an oversight (both parts of this particular universal build are 64 bits). If I understand correctly, this also means that Universal2 macOS release builds don't support these features, which seems like a shame.

I think the correct thing to do here is for the search_section_in_file functions to decode FAT_MAGIC and FAT_CIGAM headers correctly, since the included architectures are enumerated there. Or, just assume that they're 64-bit, since we don't even support any 32-bit macOS platforms anyways.

@brandtbucher
Copy link
Member

@savannahostrowski, I spoke to @pablogsal and he'll look into the universal issue separately. Let's roll back the CI change and land that later after the feature is fixed for universal builds.

@pablogsal
Copy link
Member

I think I can get something today today or tomorrow if you want to wait until then

@brandtbucher
Copy link
Member

Yep, we're fine waiting.

@pablogsal
Copy link
Member

pablogsal commented Apr 25, 2025

Fastest gun in the west folks: #132892

@pablogsal
Copy link
Member

pablogsal commented Apr 25, 2025

The other test needs #132852 to reuse the implementation

@pablogsal
Copy link
Member

Both are landed. Rebasing should fix the CI

Copy link
Member

@ned-deily ned-deily left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. With these additions, I was able to produce a python.org-style macOS universal2 installer containing both traditional (GIT-enabled) and free-treaded release builds both with optional JIT (yes-off). Installer smoke tests of all four options on both Apple Silicon (running current macOS 15.4) and Intel Macs (with macOS 11.7.10) did not show any obvious regressions.

The only unexpected issue was the JIT build's use of pthread_jit_write_protect_np (added in #126195). That function is only available as of macOS 11, which is when Apple Silicon Mac support was introduced. The macOS installer build supports a range of operating system releases through the use of Apple's preferred method of weak-linking and run-time testing for a feature's availability. We've tried to cover a very broad range of releases and architecture, occasionally upping the lower bound. For 3.13, we have been supporting down to macOS 10.13 but, for various reasons, have been planning to bump that to at least 10.15 for 3.14 and it likely wouldn't affect too many users if we went to macOS 11 (which was the next feature release after 10.15) instead. For b1, I'll plan to target macOS 11+ and make a final recommendation as part of a macOS support policy PEP during the beta phase. There is more than one approach we could take should we decide such support is needed but my initial reaction is that it won't be. (Also, note that this is only an issue when building to support a range of operating systems with one universal binary. Building with the jit enabled for, say, just macOS 10.15 with a macOS 10.15 SDK should work as is.)

@brandtbucher
Copy link
Member

brandtbucher commented Apr 29, 2025

Thanks @ned-deily!

LGTM. With these additions, I was able to produce a python.org-style macOS universal2 installer containing both traditional (GIT-enabled) and free-treaded release builds both with optional JIT (yes-off). Installer smoke tests of all four options on both Apple Silicon (running current macOS 15.4) and Intel Macs (with macOS 11.7.10) did not show any obvious regressions.

One thing I forgot to mention is that I'm planning on making it an error in 3.14 to pass both --enable-experimental-jit and --disable-gil to ./configure (it doesn't do what you think; it builds the JIT, but never actually uses it at runtime).

So we would only pass --enable-experimental-jit=yes-off to the traditional GIL-enabled build. Is that an issue at all?

The only unexpected issue was the JIT build's use of pthread_jit_write_protect_np (added in #126195). That function is only available as of macOS 11, which is when Apple Silicon Mac support was introduced. The macOS installer build supports a range of operating system releases through the use of Apple's preferred method of weak-linking and run-time testing for a feature's availability. We've tried to cover a very broad range of releases and architecture, occasionally upping the lower bound. For 3.13, we have been supporting down to macOS 10.13 but, for various reasons, have been planning to bump that to at least 10.15 for 3.14 and it likely wouldn't affect too many users if we went to macOS 11 (which was the next feature release after 10.15) instead. For b1, I'll plan to target macOS 11+ and make a final recommendation as part of a macOS support policy PEP during the beta phase. There is more than one approach we could take should we decide such support is needed but my initial reaction is that it won't be. (Also, note that this is only an issue when building to support a range of operating systems with one universal binary. Building with the jit enabled for, say, just macOS 10.15 with a macOS 10.15 SDK should work as is.)

Thanks for the explanation. If it turns out to be too much trouble, we can use the "older" mprotect calls to do the same thing. Apparently MAP_JIT/pthread_jit_write_protect_np is faster if you're constantly flipping permissions on the JIT pages, but that's not something we do in 3.14, so it's not a huge loss to go back if needed.

@savannahostrowski
Copy link
Member Author

savannahostrowski commented Apr 30, 2025

Thanks @ned-deily for the review and @pablogsal for the quick turnaround on the test fixes!

(This is also my last CPython contribution as Savannah Ostrowski, as I'm getting married this weekend, so it feels doubly special 🥹)

@savannahostrowski savannahostrowski merged commit 26c0248 into python:main Apr 30, 2025
67 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants