Support LLVM14 #316
Support LLVM14 #316
Conversation
nbaksalyar
force-pushed
the
llvm14
branch
2 times, most recently
from
4e25fc5 to
28fd922
Compare
|
llvm-sys v140 has been published now, so this PR is ready to be reviewed. |
README.md
Outdated
| It is needed to compile BPF bytecode. | ||
| - **LLVM 14 or 13** | ||
| It is needed to compile BPF bytecode. If you're using Rust 1.60 or later, LLVM 14 is required. | ||
| If you need to use LLVM 13, use the feature `llvm13-0`. |
There was a problem hiding this comment.
You seem to have reverted this again to llvm13-0?
There was a problem hiding this comment.
Ah, that's awkward. Made this change on another machine and forgot to synchronise. Sorry about that!
CI configuration needs to be updated too as it seems.
There was a problem hiding this comment.
CI configuration needs to be updated too as it seems.
@rsdy I think there are several ways make it work - which one would you recommend to go for?
- Update the Docker images (needs to be done at some point anyway but now it's a bit more complicated - e.g.,
llvm14isn't packaged for Fedora 35 so you have to use F36 beta). - Change build commands in the CI workflows to
cargo build --features=llvm13. - Make
llvm13the default version/feature for now (I'm favouring this one).
There was a problem hiding this comment.
@nbaksalyar Sorry for the slow response on this, I've been away from the Internet. Currently bumping images for 1.
README.md
Outdated
| @@ -235,7 +242,8 @@ There are two LLVM versions involved in compiling BPF programs: | |||
|
|
|||
| | Rust version | LLVM version of the rustc | Valid LLVM version of system | | |||
| |:-------------|:-------------------------:|:-----------------------------| | |||
| | 1.56 ~ | LLVM 13 | LLVM 13 and newer | | |||
| | 1.56 ~ 1.60 | LLVM 13 | LLVM 13 and newer | | |||
There was a problem hiding this comment.
Shouldn't it be "1.56 ~ 1.59"?
I've reproduced #315 using Rust 1.60 + system LLVM 13.0.1, and Rust 1.59 + system LLVM 13.0.1 works well.
Signed-off-by: Nikita Baksalyar <[email protected]>
* Use latest Docker images for builds. * Re-enable Alpine build because it now supports llvm13. Signed-off-by: Nikita Baksalyar <[email protected]>
rq_disk was removed from the request struct. Use request.q.disk instead. Signed-off-by: Nikita Baksalyar <[email protected]>
Signed-off-by: Nikita Baksalyar <[email protected]>
|
Hi @rsdy. I've made some progress this weekend on this issue, but there's more to do. Do you want incremental PRs along the way to get a some improvements earlier rather than later, or do you want one big PR that handles it all? Also, wanted to inform you in advance it will require a PR to build-images to get merged first and for those images to be updated. Looking at previous comments I suspect you expected that though 😄 |
|
@DerekStrickland Thanks for picking this up! I'm happy to merge PRs as they come in assuming that they keep the repo in a consistent state, e.g. if it breaks I think the best approach might be for you to re-open a different PR with this branch, targeting main, so I can close this and move work over to your fork. Does that make sense? |
|
Yes it does. I've got a little bit of clean up to do and the work week is back upon us. I'll get some incremental updates over to you as soon as I can starting with the build-images changes that will be required. |
|
Excited about this pr! Thanks for all of the work on it. |
|
Thanks @AlexGatz. The build-images PR got merged yesterday but failed in the foniod CI while passing in my fork. I'm investigating what is up with that before proceeding to updating RedBPF itself. I'll keep you all posted as I progress. |
|
LLVM 15 is released! |
This is WIP until
llvm-sysis updated to version 140.Fixes #312.