-
Notifications
You must be signed in to change notification settings - Fork 13.7k
fix(compiler/rustc_target): set correct linker flags for wasm32v1-none
#145539
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: master
Are you sure you want to change the base?
fix(compiler/rustc_target): set correct linker flags for wasm32v1-none
#145539
Conversation
These commits modify compiler targets. |
// were made to WebAssembly starting with LLVM 20.1.0: | ||
// https://releases.llvm.org/20.1.0/docs/ReleaseNotes.html#changes-to-the-webassembly-backend | ||
"--features=+mutable-globals", | ||
"--no-check-features", |
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.
Disabling feature checking doesn't seem right to me. Why is that necessary?
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.
https://github.com/llvm/llvm-project/blob/llvmorg-20.1.8/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp#L249-L257
because if there are 0 functions in the LLVM IR module, then AnyDefinedFuncs = false
and this will result in all features from wasm32-unknown-unknown
being enabled
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.
hmm, you're right in sense. because it will break -C target-feature=+...
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.
it seems that the correct fix is to somehow set the fallback to cpu="mvp"
instead of cpu=""
in wasm-ld
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.
Maybe adding a --mllvm=-mcpu=<target_cpu>
argument at
self.link_arg(&format!("--lto-{opt_level}")); |
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.
it works
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.
although it seems that this is problem with -C target-feature=+...
// were made to WebAssembly starting with LLVM 20.1.0: | ||
// https://releases.llvm.org/20.1.0/docs/ReleaseNotes.html#changes-to-the-webassembly-backend | ||
"--features=+mutable-globals", | ||
"--no-check-features", | ||
"--mllvm=-mcpu=mvp", |
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.
Doing it at the place I indicated (the WasmLd implementation) has the advantage that it works for all wasm targets and also when the user passes -Ctarget-cpu
to override the target default.
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, that's probably good idea, but we need WasmTM->CPU
to be set from the command line.
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.
wasm-ld can't notice any difference between setting this cli arg in the target spec and setting it in the WasmLd implementation I linked to.
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, you're right. I checked in the debugger and it's also worth adding --mllvm=-mattr=+mutable-globals
to change WasmTM->TargetFS
(features set).
Resolves #145491
r? @alexcrichton
https://github.com/llvm/llvm-project/blob/llvmorg-20.1.8/lld/wasm/Writer.cpp#L573-L691
https://releases.llvm.org/20.1.0/docs/ReleaseNotes.html#changes-to-the-webassembly-backend