-
-
Notifications
You must be signed in to change notification settings - Fork 31.8k
gh-114809: Support fat builds on macOS with the experimental JIT #115742
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
base: main
Are you sure you want to change the base?
Conversation
@@ -106,6 +108,7 @@ async def _compile( | |||
o = tempdir / f"{opname}.o" | |||
args = [ | |||
f"--target={self.triple}", | |||
"-isysroot", "/Users/ronald/Applications/Xcode-beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will be removed in the final version, and likely needs a different issue and patch: I installed llvm using a download on llvm's GitHub, and use Xcode to build python (not Command Line Tools). Without this addition "clang-16" cannot find the SDK to use.
if ( | ||
not self.force | ||
and jit_stencils.exists() | ||
and jit_stencils.read_text().startswith(digest) | ||
): | ||
return | ||
stencil_groups = asyncio.run(self._build_stencils()) | ||
stencil_groups = ASYNCIO_RUNNER.run(self._build_stencils()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if this is the right way to enable running multiple builds, I'm a very light asyncio user. Without this change the second run fails due to running with a different runloop.
args.target.build(pathlib.Path.cwd(), comment=comment) | ||
|
||
else: | ||
# Multiple triples specified, assume this is a macOS multiarchitecture build |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This bit is not ideal, multiple files are created and that likely breaks dependency handling in the Makefile.
It should be possible to publish everything in a single header file, but that requires more significant changes to the builder.
with open("jit_stencils.h", "w") as fp: | ||
for idx, target in enumerate(args.target): | ||
cpu, _, _ = target.triple.partition("-") | ||
fp.write(f"#{'if' if idx == 0 else 'elif'} defined(__{cpu}__)\n") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works for x86_64 and arm64, but might break for ancient systems running PPC and PPC64. I don't care for those.
Thanks for working on this! Juggling a few tasks right now, but I'll try this out later this week. |
Something like this fixes CI for me, and generalizes this to all platforms: diff --git a/Tools/jit/_targets.py b/Tools/jit/_targets.py
index 37b3e596bd..fd82eda697 100644
--- a/Tools/jit/_targets.py
+++ b/Tools/jit/_targets.py
@@ -37,6 +37,7 @@
@dataclasses.dataclass
class _Target(typing.Generic[_S, _R]):
triple: str
+ condition: str
_: dataclasses.KW_ONLY
alignment: int = 1
prefix: str = ""
@@ -108,7 +109,6 @@ async def _compile(
o = tempdir / f"{opname}.o"
args = [
f"--target={self.triple}",
- "-isysroot", "/Users/ronald/Applications/Xcode-beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk",
"-DPy_BUILD_CORE",
"-D_DEBUG" if self.debug else "-DNDEBUG",
f"-D_JIT_OPCODE={opname}",
@@ -383,15 +383,21 @@ def _handle_relocation(
def get_target(host: str) -> _COFF | _ELF | _MachO:
"""Build a _Target for the given host "triple" and options."""
if re.fullmatch(r"aarch64-apple-darwin.*", host):
- return _MachO(host, alignment=8, prefix="_")
+ condition = "defined(__aarch64__) && defined(__APPLE__)"
+ return _MachO(host, condition, alignment=8, prefix="_")
if re.fullmatch(r"aarch64-.*-linux-gnu", host):
- return _ELF(host, alignment=8)
+ condition = "defined(__aarch64__) && defined(__linux__)"
+ return _ELF(host, condition, alignment=8)
if re.fullmatch(r"i686-pc-windows-msvc", host):
- return _COFF(host, prefix="_")
+ condition = "defined(_M_IX86)"
+ return _COFF(host, condition, prefix="_")
if re.fullmatch(r"x86_64-apple-darwin.*", host):
- return _MachO(host, prefix="_")
+ condition = "defined(__x86_64__) && defined(__APPLE__)"
+ return _MachO(host, condition, prefix="_")
if re.fullmatch(r"x86_64-pc-windows-msvc", host):
- return _COFF(host)
+ condition = "defined(_M_X64)"
+ return _COFF(host, condition)
if re.fullmatch(r"x86_64-.*-linux-gnu", host):
- return _ELF(host)
+ condition = "defined(__x86_64__) && defined(__linux__)"
+ return _ELF(host, condition)
raise ValueError(host)
diff --git a/Tools/jit/build.py b/Tools/jit/build.py
index 800d8e31b0..7872004a90 100644
--- a/Tools/jit/build.py
+++ b/Tools/jit/build.py
@@ -42,8 +42,7 @@
with open("jit_stencils.h", "w") as fp:
for idx, target in enumerate(args.target):
- cpu, _, _ = target.triple.partition("-")
- fp.write(f"#{'if' if idx == 0 else 'elif'} defined(__{cpu}__)\n")
+ fp.write(f"#{'if' if idx == 0 else 'elif'} {target.condition}\n")
fp.write(f'# include "jit_stencils-{target.triple}.h"\n')
fp.write("#else\n") |
@ronaldoussoren Hey there! Are you planning on carrying this forward? I have the branch pulled down locally and have patched things up with the main, etc. I'm happy to carry it forward if you're working on other things. Just let me know! |
While preparing for the 3.14.0a2 release, I realized too late that this issue had not yet been resolved thus preventing supporting the JIT in the python.org macOS installer build. My apologies for that. I'm looking at the issue now. If anyone does have any updates for it, it would be great to get them into the draft PR soon so we can re-target this for 3.14.0a3. |
Feel free to carry it forward. I'm afraid I won't have time to work on this until at least the end of the year. |
Updates to the build system to support fat builds (
--enable-universalsdk
) on macOS with the experimental JIT.Current status: "it compiles, therefore it works"