-
-
Notifications
You must be signed in to change notification settings - Fork 31.9k
gh-118306: Update JIT compilation to use LLVM 18 #118307
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
Conversation
It looks like |
Thanks for taking on this project... it ended up being quite a bit more work for you than I originally thought! I'd like to benchmark this and peek around at the generated code to make sure we don't see any significant performance degradations. I'm expecting a small slowdown on
Yup, see GH-118275. |
Also, thanks for digging up the related LLVM issues! |
Co-authored-by: Brandt Bucher <[email protected]>
Co-authored-by: Brandt Bucher <[email protected]>
I think we might be missing some tests or checks for |
Hm, I wonder if Fedora uses a patched LLVM with different defaults or something... |
Can you try adding |
Ah! Okay, that did work for the Codespace. Also, checked in the local dev container as well. Thanks for the tip! |
Nice, looks like it's also a slight performance improvement (if anything). |
Doc failure isn't us: #118401 |
Free-threading failures look like flukes too. Gonna re-run once the JIT tests finish. |
This PR updates the LLVM version used for JIT compilation from 16 to 18.
Main changes, other than the basic
16
-->18
string updates 😊 :Known Issues:
--with-lto
option had to be removed for emulated and native Linux in CI due to an open issue with llvm-18 on Ubuntu 22.04. The issue is thatLLVMgold.so
is currently missing from binaries (see this failed CI run as an example). The current workaround folks are using is to build without LTO. See The llvm-18 Ubuntu Jammy (22.04) LTS binary packages don't include LLVMgold.so llvm/llvm-project#87553 for more details. Not ideal but @brandtbucher I'll let you be the judge as to whether this is acceptable in the short-term. I've left an inline comment in the workflow yml about this.-mcmodel=large
seems to be unsupported in LLVM 18 so I had to remove this from the build args in_targets.py
. See [Driver] Reject unsupported -mcmodel= llvm/llvm-project#70262 for more details.