Skip to content
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

expose wasm intrinsics using target_family = "wasm" #1241

Merged
merged 2 commits into from
Oct 31, 2021

Conversation

devsnek
Copy link
Contributor

@devsnek devsnek commented Oct 30, 2021

I think it should be idiomatic to use target_family = "wasm" instead of target_arch = "wasmNN", so we encourage the ecosystem to write code that works for both. This makes it easier to do that by exporting a wasm in addition to wasm32 and wasm64.

@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Amanieu (or someone else) soon.

Please see the contribution instructions for more information.

@Amanieu Amanieu merged commit ea14173 into rust-lang:master Oct 31, 2021
@devsnek devsnek deleted the patch-1 branch October 31, 2021 18:35
@devsnek
Copy link
Contributor Author

devsnek commented Oct 31, 2021

thanks!

@alexcrichton
Copy link
Member

Personally I don't think that this should happen. Even x86/x86_64 don't have precedent for this which is my main personal rationale, but otherwise this is also a new insta-stable module (which I'm not sure was intended?)

@Amanieu
Copy link
Member

Amanieu commented Nov 1, 2021

Oh whoops, I missed the fact that this was marked as insta-stable.

@bjorn3
Copy link
Member

bjorn3 commented Nov 1, 2021

Even x86/x86_64 don't have precedent for this which is my main personal rationale

x86 and x86_64 have a different instruction set. wasm32 and wasm64 use the exact same instruction set. AFAIK it is even possible to get a mix of the two by having more than one linear memory. LLVM currently doesn't support this AFAIK, but it could by using different address space's for each linear memory.

@ehuss
Copy link
Contributor

ehuss commented Nov 16, 2021

This is causing a problem for wasm32-unknown-emscripten which is in the unix family, but lines above try to import the wasm32 module using target_arch = "wasm32". Should the module include wasm32` in the cfg? Or should this PR be reverted?

@CryZe
Copy link
Contributor

CryZe commented Nov 16, 2021

It sounds more like emscripten should be in that family instead to me. It looks like emscripten accidentally overwrites the families here instead of appending to the wasm base's families: https://github.com/rust-lang/rust/blob/master/compiler/rustc_target/src/spec/wasm32_unknown_emscripten.rs#L40

@alexcrichton
Copy link
Member

@ehuss looks like the failure came through at rust-lang/rust#90382 (comment), I added another commit to add wasm to the families of emscripten and if that fails I'll dig further and actually build it locally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants