Repository navigation
GH-136895: Update JIT builds to use LLVM 20 - #140329
savannahostrowski merged 43 commits into
Conversation
Sorry, something went wrong.
With LLVM 20, individual files are greater than the 100MiB single file limit for items checked into git. Therefore, this PR pulls down binaries from GitHub releases, as `.tar.xz` files to additionally maximize compression ratio. Currently this is somewhat of a first draft, as there are things like hash checking needed to be done.
This reverts commit e6450de.
markshannon
left a comment
There was a problem hiding this comment.
A bit of C pedantry. Otherwise looks good.
Sorry, something went wrong.
|
When you're done making the requested changes, leave the comment: I have made the requested changes; please review again. |
Sorry, something went wrong.
brandtbucher
left a comment
There was a problem hiding this comment.
Awesome! Here's a partial review of everything except the stuff in Tools (I'd like to dig a bit more into the changes to the flags and relocations and see how they affect the code quality).
Sorry, something went wrong.
| /* Generate the trampoline (14 bytes, padded to 16): | ||
| 0: ff 25 00 00 00 00 jmp *(%rip) | ||
| 6: XX XX XX XX XX XX XX XX (64-bit target address) | ||
|
|
||
| Reference: https://wiki.osdev.org/X86-64_Instruction_Encoding#FF (JMP r/m64) | ||
| */ | ||
| trampoline[0] = 0xFF; | ||
| trampoline[1] = 0x25; | ||
| *(uint32_t *)(trampoline + 2) = 0; | ||
| *(uint64_t *)(trampoline + 6) = value; |
There was a problem hiding this comment.
@savannahostrowski... just want to make sure it's not lost on you how badass you are hand-writing x86-64 machine code byte-by-byte like this. 🔥
Sorry, something went wrong.
markshannon
left a comment
There was a problem hiding this comment.
Looks good
Sorry, something went wrong.
emmatyping
left a comment
There was a problem hiding this comment.
This looks great! Very excited for this
Sorry, something went wrong.
|
I am going to merge this since it has multiple reviews and is blocking some other work that we'd like to do. @brandtbucher let's chat offline about any other changes we'd like to make here re. flags/relocations, happy to follow up! |
Sorry, something went wrong.
Co-authored-by: Emma Harper Smith <emma@emmatyping.dev>
|
Could this be backported to 3.14? We would really like to get rid of LLVM 19 in openSUSE/Factory. |
Sorry, something went wrong.
This comment was marked as outdated.
|
GH-146420 is a backport of this pull request to the 3.14 branch. |
Sorry, something went wrong.
Alright, this took me longer than expected for two reasons:
Bumping to LLVM 20 required some changes to the infrastructure we use to grab LLVM as a dependency on Windows. As of 20, LLVM now contains files that exceed GitHub's allowable size, so checking these directly into cpython-bin-deps' tree was not an option. Instead, this PR adds the ability to pull from release artifacts (see https://github.com/python/cpython-bin-deps/releases/tag/llvm-20.1.8.0). This also makes the process of bumping LLVM for Windows a little less hairy. Thank you, @emmatyping, for much of this code, and thank you, @zware, for publishing the release artifact for me 🙏 !
For macOS x86_64 debug builds, external symbol references generated by LLVM 20 can exceed the ±2GB PC-relative addressing range that patch_32r requires. This manifested as assertion failures in free-threading tests (see https://github.com/savannahostrowski/cpython/actions/runs/18438327725/job/52537027963?pr=10 as an example). After going down a rabbit hole, I believe the correct fix here is to implement x86_64 trampolines (similar to our existing aarch64 implementation) to handle out-of-range symbols...and so I've done that. AFAICT, trampolines are only needed on macOS right now, as the other target platforms have different relocation mechanisms.