-
Notifications
You must be signed in to change notification settings - Fork 24
Update code base to use LLVM 21 #434
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
base: main
Are you sure you want to change the base?
Conversation
|
Still draft as I need to fix polkavm's dwarf handling. |
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.
Great 🙂 The cross compilation section in the compiler book needs an update too (still referencing LLVM_SYS_181_PREFIX), as well as an entry to the CHANGELOG.md (perhaps we should add an "Internal" section to the CHANGELOG 🤔).
| "-DCOMPILER_RT_BUILD_MEMPROF='Off'", | ||
| "-DCOMPILER_RT_BUILD_ORC='Off'", | ||
| "-DCOMPILER_RT_DEFAULT_TARGET_ARCH='aarch64'", | ||
| "-DCOMPILER_RT_DEFAULT_TARGET_ONLY='On'", |
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.
Why does this need to be removed? Doesn't this ensure it only tries to build for aarch64?
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.
Unfortunately, I have no good answer to that question. That define caused compiler-rt to look for some asan headers that in the wrong location.
I was trying to understand logic in cmake files of compiler-rt, but just gave up.
jfyi: In official docs on how to build compiler-rt, this flag is not used.
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.
Ah ok, strange. Is the size of the build artifact affected? Maybe the change is negligible.
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.
I can't follow, this flag tells the build system to only built the runtime for the default target (for the host compiler). How would it change the size of our contract builds?
I have no good answer to that question
Fair. They set this flag in the era compiler llvm builder and I didn't bother to investigate it first hand. It must've something to do with how cross builds work. I'll briefly look into it too.
| "-DCOMPILER_RT_BUILD_MEMPROF='Off'", | ||
| "-DCOMPILER_RT_BUILD_ORC='Off'", | ||
| "-DCOMPILER_RT_DEFAULT_TARGET_ARCH='x86_64'", | ||
| "-DCOMPILER_RT_DEFAULT_TARGET_ONLY='On'", |
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.
Same as previous comment.
Co-authored-by: LJ <81748770+elle-j@users.noreply.github.com>
Good point of pointing out that it should be added to the changelog. But WDYM with "internal" section, he can just add it on top, where all new (unreleased) changes go? |
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.
Nice, thanks! LGTM. But could you please first try all release workflows (LLVM and then resolc) in a fork (e.g. here)?
| inkwell::attributes::AttributeLoc::Param(index as u32), | ||
| self.llvm | ||
| .create_enum_attribute(Attribute::NoCapture as u32, 0), | ||
| .create_enum_attribute(Attribute::Captures as u32, 0), // captures(none) |
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.
This caught my eye. Since NoCapture no longer exist. I guess the comment intents to say that it is still like no capture?
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.
nocapture now is represented as captures(none) in textual LLVM IR
Yes still part of the Unreleased section, was thinking more of a subsection within that called e.g. Internal, for changes that may not necessarily affect the usage of resolc. E.g.: |
|
This change should be mentioned under |
xermicus
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.
I think we should also update rust musl cross.
- Update the rust-musl-cross image to the latest tag on CI and Dockerfile
- Use Rust 1.92
- Update to download solc 0.33 in the Dockerfile
The cumulative LLVM21 musl build fixes: - Update rust-musl-cross and Rust versions - Bring back `DCOMPILER_RT_DEFAULT_TARGET_ONLY` to prevent multilib related issues in the cross build - Use lld in the musl target stage - ~~Do not build the LTO tool shared object (this caused linker errors for some reason)~~ scrapped - that seemed sus to me and I tested in a clean full-stage build. Maybe was an issue with the build cache. --------- Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>
do you mean as a part of this PR or follow up ? |
Done with #466 |
|
Updated changelog |
Co-authored-by: xermicus <cyrill@parity.io>
No description provided.