Skip to content

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ronaldoussoren
Copy link
Contributor

@ronaldoussoren ronaldoussoren commented Feb 20, 2024

Updates to the build system to support fat builds (--enable-universalsdk) on macOS with the experimental JIT.

Current status: "it compiles, therefore it works"

@bedevere-app bedevere-app bot mentioned this pull request Feb 20, 2024
@@ -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",
Copy link
Contributor Author

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())
Copy link
Contributor Author

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
Copy link
Contributor Author

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")
Copy link
Contributor Author

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.

@brandtbucher
Copy link
Member

Thanks for working on this! Juggling a few tasks right now, but I'll try this out later this week.

@brandtbucher
Copy link
Member

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")

@savannahostrowski
Copy link
Member

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

@ned-deily
Copy link
Member

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.

@ned-deily ned-deily self-requested a review November 19, 2024 17:20
@ronaldoussoren
Copy link
Contributor Author

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

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.

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.

4 participants