-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Add support for LLVM's wasm-ld #11862
base: master
Are you sure you want to change the base?
Conversation
Oh wow! I have recently decided to switch to Meson for one of my projects, and this patch is exactly what I need, having run into some issues just a few minutes ago (specifically with As a bystander, I have a couple question: I normally use Edit: Another question: I normally also use (I will note that when I ask “is there a difference”, I mean to ask if there is something relevant for the sake of this pull request, i.e. whether it would be better to reconsider those options.) Edit 2: Also, I think it might be worthwhile to specify |
@zamfofex EDIT: didn't read your first question propertly. As per About the Regarding wasm32, it would be a good future-proof measure to change, at least, the name of the cross files, even for Emscripten. I'll rename and add the strip binary to both cross files, I completely missed it for some reason :) |
521acae
to
39ddfed
Compare
This is exactly what I was looking for! +1 This PR adds support for compiling WebAssembly System Interface (WASI) programs using Meson. WASI is a modular system interface for WebAssembly, designed as a portable target for the compilation of high-level languages like C, allowing them to run on all different types of systems (including embedded systems and servers) without needing to recompile. |
Can you add a release notes snippet in docs/markdown/snippets? ## Added support for the LLVM WASI linker
... |
@@ -923,7 +923,37 @@ | |||
raise mesonlib.MesonBugException(f'win_subsystem: {value} not handled in lld linker. This should not be possible.') | |||
|
|||
|
|||
class WASMDynamicLinker(GnuLikeDynamicLinkerMixin, PosixDynamicLinkerMixin, DynamicLinker): | |||
class LLVMWASMDynamicLinker(GnuLikeDynamicLinkerMixin, PosixDynamicLinkerMixin, DynamicLinker): |
Check warning
Code scanning / CodeQL
Conflicting attributes in base classes
install_rpath: str) -> T.Tuple[T.List[str], T.Set[bytes]]: | ||
return ([], set()) | ||
|
||
class EmscriptenWASMDynamicLinker(GnuLikeDynamicLinkerMixin, PosixDynamicLinkerMixin, DynamicLinker): |
Check warning
Code scanning / CodeQL
Conflicting attributes in base classes
39ddfed
to
bd8ad9c
Compare
@tristan957 Added, thanks for pointing out. |
bd8ad9c
to
41eee7a
Compare
I have targeted this for 1.3.0. Hopefully someone can review it before then |
I'll try to keep it free of conflicts meanwhile. Thanks! |
The current conflicts are due to some big refactoring I did a short while ago to delay expensive imports. Once that is resolved there probably should not be any more conflicts. It's basically just imports though, so it should be cheap to rebase and resolve them. The reason it's being targeted for 1.3.0, for context, is because we're currently in the release candidate stage for the pending release of 1.2.0, which means that the Hopefully we'll be un-freezing for continued development in about a week, fingers crossed that we don't need an rc4... |
Wanna rebase this PR @GuilleX7? |
Sure, I'll rebase and ping you when it's ready. @tristan957 |
if os.environ.get('WASI') != '1': | ||
return True |
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.
Seems like you could remove this?
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.
Well, I'm not sure how to proceed here. The idea is to avoid running these tests when the LLVM installation doesn't ship a proper WASI libc, which is the case for out-of-the-box Clang installations. My original idea was to test the LLVM installation instead, but I couldn't get it done properly, so I ended up adding this. I'd be really grateful if you can lend me a hand here.
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.
How does one know if the LLVM installation ships a proper WASI libc? Can you query such information with llvm-config for example?
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 guess a pragmatic approach would be to perform a sanity check: look for Clang and then have it compile a single file with a main function using the proper flags to target wasm32-wasi. I remember I tried to do something similar to https://github.com/mesonbuild/meson/blob/master/run_project_tests.py#L965, but for some reason I failed. I'll try to do it again, maybe I just didn't spend too much time on it.
Edit: since standard LLVM installations can target wasm32 by default, the only difference is WASI libc missing, which has to be downloaded manually by the user. I don't think llvm-config could be helpful at all in this case.
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.
Yes, llvm-config
would have no way to know if there is a WASI sysroot, that is not its purpose. I think you could run clang -print-resource-dir
and check if that has a lib/wasi
subdirectory.
You don't need the extra_files and VERSION in your tests imo. Those are being tested elsewhere. |
I'll get rid of them. Thanks! |
41eee7a
to
bbccc59
Compare
It seems that meson only supports emscripten as a compiler to wasm at present, wasi-sdk support is not completed yet, so this only adds ci build scripts for emscripten. References: * <mesonbuild/meson@f41bdae> * <mesonbuild/meson#11862>
It seems that meson only supports emscripten as a compiler to wasm at present, wasi-sdk support is not completed yet, so this only adds ci build scripts for emscripten. References: * <mesonbuild/meson@f41bdae> * <mesonbuild/meson#11862>
It seems that meson only supports emscripten as a compiler to wasm at present, wasi-sdk support is not completed yet, so this only adds ci build scripts for emscripten. References: * <mesonbuild/meson@f41bdae> * <mesonbuild/meson#11862>
bbccc59
to
e52c9fb
Compare
Hmm, I don’t know what changed, but this code doesn’t seem to be working anymore. Presumably something changed from LLVM’s side on how they handle some options in LLD. I was able to fix this code with the following patch: diff --git a/mesonbuild/linkers/detect.py b/mesonbuild/linkers/detect.py
index 1094f5e..5b9ea86 100644
--- a/mesonbuild/linkers/detect.py
+++ b/mesonbuild/linkers/detect.py
@@ -161,9 +161,9 @@ def guess_nix_linker(env: 'Environment', compiler: T.List[str], comp_class: T.Ty
lld_cls = linkers.LLVMLD64DynamicLinker
else:
if isinstance(comp_class.LINKER_PREFIX, str):
- cmd = compiler + override + [comp_class.LINKER_PREFIX]
+ cmd = compiler + override + [comp_class.LINKER_PREFIX + '-h']
else:
- cmd = compiler + override + comp_class.LINKER_PREFIX
+ cmd = compiler + override + comp_class.LINKER_PREFIX + ['-h']
_, _, newerr = Popen_safe(cmd)
if 'wasm-ld' in newerr:
lld_cls = linkers.LLVMWASMDynamicLinker |
What is needed to bring this PR forward? The Emscripten toolchain isn't useful if you're building WebAssembly for a non-browser target like Wasmtime, and currently Meson is a blocker in being able to cross-compile some projects this way. |
I was hoping another maintainer who is more familiar with wasm would be available to review this... :( Maybe @jpakkane can review this as the person that initially implemented emscripten support? |
I have a lot of Wasm experience. I've looked through this PR and (other than the comment I made) I have one major concern. WASI has a distinction between I believe they should most likely be distinct systems in Meson, although I've used Meson for the first time today and I may be entirely wrong here. |
@whitequark Based on your description that sounds like we should consider them two different systems. Or two different subsystems? not sure. |
Having "wasi" as the system and "wasip1"/"wasip2" as the subsystem sounds reasonable to me from first principles. The programming model in C/C++ is essentially the same, so being able to compare just the system seems useful, but for deployment the subsystems would be fairly different as they cannot execute on the same set of Wasm engines. |
It's been a long time since some WASM support was added to Meson (see f41bdae). However, there has been support for the Emscripten toolchain only, which ships a stand-alone compiler and linker. Since they moved to LLVM, Emscripten uses its own compiler (emcc) and a custom LLVM's wasm-ld, which is called directly from the compiler, hiding it from the end user.
LLVM's Clang comes with stable, out-of-the-box support for WASM and WASI since version 8.0.0 (see https://releases.llvm.org/8.0.0/docs/ReleaseNotes.html#changes-to-the-webassembly-target). While Meson is moreless able to interact with wasm-ld, it tries to generate options for a generic GNU lld.ld linker, which are invalid for wasm-ld. See #6662, #6847, #10187, #10116, #7489, which would be fixed with this PR.
The main purpose of this PR is to add support for directly using LLVM's Clang + wasm-ld toolchain. The main changes are listed below:
Emscripten does not cover the entire WASM ecosystem anymore:
WASMDynamicLinker
has been renamed toEmscriptenWASMDynamicLinker
wasm
are no longer appended a.js
suffix. Those generated for theemscripten
system still are. This is not a breaking change, sinceemscripten
is registered as a stable system in the reference table, butwasm
is not. While we can keep the original behaviour, it is "more" correct, IMO, to add.wasm
suffix to the unknown WASM system.wasm
has been renamed toemscripten
, following the same logic as abovewasm.txt
has been renamed towasm-emscripten.txt
, following the same logic as aboveSupport for freestanding WASM and WASM/WASI has been added:
LLVMWASMDynamicLinker
has been added. It is detected by passing some invalid parameters to Clang. Clang will try, by default, to link with the standard WASI libc, even ifwasm32-unknown-unknown
is specified. Therefore, two scenarios can happen after askingclang --target=wasm32-unknown-unknown -Wl,
: (1) If the Clang installation contains a valid WASI sysroot and can find the WASI libc, wasm-ld will fail to resolve the main symbol, or (2) the Clang installation doesn't contain a valid WASI sysroot and the linker will complain about the libc and the startup files not being resolved. In either case, the name of the linker (wasm-ld
) appears, which we can rely on to pick LLVMWASMDynamicLinker.wasi
orwasm
system are appended the.wasm
suffix(Error messages from wasm-ld as mentioned above)
WASI
environment variable is set to1
. This is done by thecross/wasm-wasi.json
, since not every LLVM installation can handle WASI.I really tried my best here, but since I have little knowledge on this project, I might be missing something. I'll be glad to answer any question, and I'll be open to any suggestion/changes.