-
Notifications
You must be signed in to change notification settings - Fork 349
Use LLVM 15 by default, add experimental LLVM 16 support #1067
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
Changes required to use LLVM 15 by default, and to support LLVM 16 experimentally, include: CI config: - Bump all LLVM 14 configurations to use LLVM 15 - Dump the LLVM 15 configurations to use LLVM 16 llvmlite: - Always set opaque pointers to false (they are the default in both 15 and 16, unlike 14). - Don't initialize of `ObjCARCOpts` in LLVM 16 and above. This was removed in LLVM 16 by: ``` commit 4153f989bab0f2f300fa8d3001ebeef7b6d9672c Author: Arthur Eubanks <[email protected]> Date: Sun Oct 2 13:20:21 2022 -0700 [ObjCARC] Remove legacy PM versions of optimization passes ``` - Remove the `AggressiveInstCombine` and `PruneEH` passes. These were removed from the legacy pass manager in LLVM 16 by: ``` commit 70dc3b811e4926fa2c88bd3b53b29c46fcba1a90 Author: Arthur Eubanks <[email protected]> Date: Mon Oct 31 14:50:38 2022 -0700 [AggressiveInstCombine] Remove legacy PM pass ``` and ``` commit 46fc75ab28b78a730ea21fd7daba6443937bfaac Author: Sebastian Peryt <[email protected]> Date: Mon Sep 26 18:31:32 2022 -0700 [NFC][2/n] Remove PrunePH pass ``` - Modify `reserveAllocationSpace` in the memory manager to use `Align` for the type of alignments in LLVM 16 - this mirrors an upstream change. - Remove LLVM 14-specific code paths (and one vestigial LLVM < 9 path). - Update the function attributes test to recognize the new form of memory attributes, - `memory(<action>)` as opposed to individual attributes like `readonly`. See: https://releases.llvm.org/16.0.0/docs/LangRef.html#function-attributes
sklam
left a comment
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.
Code changes are good. Just need to schedule time for this to be merged as we don't want to affect the Scipy demo this week that is relying on dev build of llvmlite.
|
Just noticed that this doesn't bump the llvmdev version in the conda recipe |
|
It should replace the |
|
Ah I thought we might just delete the LLVM 14 version separately as I didn't know if it would be a problem for the build farm setup if the recipe names changed. |
|
Before this can be merged, we also need to synchronize Numba to use opaque type |
Replace with llvmdev recipe for LLVM 15.
|
@sklam many thanks for the review and pointers!
I don't think we want to do this. This PR doesn't have anything to do with llvmdev builds - the llvmdev build was merged in #1036.
This is now done (and I've also done the same with the llvmdev recipe, since the llvmdev 14 recipe is no longer needed).
I don't think we want to do this. I thought that the plan was:
This is because as I understand it, the typed / opaque pointer support in LLVM versions is:
We still unconditionally disable opaque pointers in llvmlite with this PR, so Numba should be unaffected for now, and we are in a position to be able to switch to Opaque Pointers smoothly on an LLVM version (15) that supports both, without having to worry about experimental-related issues (Opaque Pointers on 14) or bitrot (Typed Pointers on 16). |
|
Just to add a little to my last comment - I don't think we should be testing Numba with LLVM 16 yet, the CI for LLVM 16 here is only for llvmlite development to ensure future compatibility, with no expectation that Numba works with that configuration yet. |
|
xref: llvmlite Opaque Pointer support is in #1064. |
|
CI is good. I've build this PR on the buildfarm llvmlite dag and tested the artifacts manually against numba main. |
update PATH before starting your build: PATH=/usr/lib/llvm-/bin:$PATH llvm-config |
How do you do that in the context of our RTD configuration? |
|
sorry, I don't know about RTD configuration |
For some background, the problem with RTD configuration is that you can't set environment variables in it as far as I know (from the linked issue in the description of this PR). If I could set an environment variable before starting the build, I'd be able to set |
Changes required to use LLVM 15 by default, and to support LLVM 16 experimentally, include:
CI config:
Docs:
LLVM_CONFIGfrom the config file (see Allow setting open build env vars in.readthedocs.ymlconfig file readthedocs/readthedocs.org#6311). Executingupdate-alternativesto setllvm-config-15as the defaultllvm-configalso fails with a permission error. So I resorted to editing the FFI build to explicitly usellvm-config-15in a post-checkout step.llvmlite itself:
Always set opaque pointers to false (they are the default in both 15 and 16, unlike 14).
Don't initialize of
ObjCARCOptsin LLVM 16 and above. This was removed in LLVM 16 by:Remove the
AggressiveInstCombineandPruneEHpasses. These were removed from the legacy pass manager in LLVM 16 by:and
Modify
reserveAllocationSpacein the memory manager to useAlignfor the type of alignments in LLVM 16 - this mirrors an upstream change.Remove LLVM 14-specific code paths (and one vestigial LLVM < 9 path).
Update the function attributes test to recognize the new form of memory attributes, -
memory(<action>)as opposed to individual attributes likereadonly. See: https://releases.llvm.org/16.0.0/docs/LangRef.html#function-attributes