-
Notifications
You must be signed in to change notification settings - Fork 191
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
Update LLVM to 18.1.2 and add wasm-component-ld #402
Update LLVM to 18.1.2 and add wasm-component-ld #402
Conversation
This commit has two intertwined changes within it. First the LLVM submodule is updated to the 18.1.2 release branch. This alone would cause CI and tests to fail due to differing behavior for the `wasm32-wasip2` target. To fix these test failures the `wasm-component-ld` tool is added to the build. This tool is a Rust-written tool and installed via `cargo install` as part of the build at a pinned version written in the `Makefile`. This linker, used for components, is then used for the `wasm32-wasip2` target. Tests and CI are then updated to skip the need to have `wasm-tools` or the adapter for WASI when making components since that's now the job of `wasm-component-ld`. This then necessitated some changes to tests too.
96c6351
to
e98143e
Compare
@sbc100 or @sunfishcode did y'all have further thoughts on this? |
I'm little wary about this new linker which is outside of llvm and written in rust, but that could also be my lack of familiarity with it. I think it would be great if it could one day be part of llvm or wasm-ld itself. Since I have not really been involved with the preview2 or component model stuff myself so I'll leave to you all to decide what is best here. I'll defer to @sunfishcode for the final lgtm. |
ping @sunfishcode, do you have thoughts on this? |
I think this change makes sense for now, and we'll see how it goes. In the future when the wasip2 support in wasi-libc is more complete, and we don't need adapters or adapter linking in the common case, then the components should be more like simple containers, so it should be easier to add component support to wasm-ld if we want to. |
This commit has two intertwined changes within it. First the LLVM submodule is updated to the 18.1.2 release branch. This alone would cause CI and tests to fail due to differing behavior for the
wasm32-wasip2
target. To fix these test failures thewasm-component-ld
tool is added to the build. This tool is a Rust-written tool and installed viacargo install
as part of the build at a pinned version written in theMakefile
. This linker, used for components, is then used for thewasm32-wasip2
target.Tests and CI are then updated to skip the need to have
wasm-tools
or the adapter for WASI when making components since that's now the job ofwasm-component-ld
. This then necessitated some changes to tests too.