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

Can not disable sign-ext feature for wasm32 target #109807

Closed
laizy opened this issue Mar 31, 2023 · 15 comments · Fixed by #128511
Closed

Can not disable sign-ext feature for wasm32 target #109807

laizy opened this issue Mar 31, 2023 · 15 comments · Fixed by #128511
Labels
A-codegen Area: Code generation A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. A-target-feature Area: Enabling/disabling target features like AVX, Neon, etc. C-bug Category: This is a bug. O-wasm Target: WASM (WebAssembly), http://webassembly.org/ T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@laizy
Copy link

laizy commented Mar 31, 2023

I tried to build a wasm binary with MVP opcode only, so I disabled the features:

RUSTFLAGS='-C link-arg=-zstack-size=32768 -C target-feature=-mutable-globals,-sign-ext,-multivalue,-simd128'
 cargo build --release --target=wasm32-unknown-unknown

I expected to see no sign-extension opcode in the wasm binary.

Instead, this happened:

$ wasm-tools parse -t target/wasm32-unknown-unknown/release/output.wasm -o hello.wat
$ cat hello.wat | grep i32.ext
                            i32.extend8_s

Meta

rustc --version --verbose:

rustc 1.70.0-nightly (ec2f40c6b 2023-03-30)
binary: rustc
commit-hash: ec2f40c6b04f0e9850dd1f454e8639d319f4ed9b
commit-date: 2023-03-30
host: x86_64-unknown-linux-gnu
release: 1.70.0-nightly
LLVM version: 16.0.0
Backtrace

<backtrace>

@laizy laizy added the C-bug Category: This is a bug. label Mar 31, 2023
@CryZe
Copy link
Contributor

CryZe commented Apr 1, 2023

You can not deactivate default features, because std is prebuilt. You need -Z build-std to build std yourself. Also these features are considered the default by LLVM ever since the recent upgrade to LLVM 16:

https://github.com/llvm/llvm-project/blob/f991f30845bb464a1a8e240b3512bac0258417db/llvm/lib/Target/WebAssembly/WebAssembly.td#L99-L106

This should possibly be mentioned in the Rust 1.70 release notes though?

@nikic nikic added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. O-wasm Target: WASM (WebAssembly), http://webassembly.org/ labels Apr 1, 2023
@nikic
Copy link
Contributor

nikic commented Apr 1, 2023

cc @alexcrichton to confirm that we're fine with inheriting this change to default target features in Rust.

@alexcrichton
Copy link
Member

More context for this change can be found in these locations:

In that sense this is an expected change, but at the same time the change was made with the expectation that most users are running on an engine that implements sign-extension. @laizy could you expand a bit more on your motivation for excluding these sign-extension instructions?

I do agree, though, that this should be mentioned in the detailed release notes. It's unfortunately a bit opaque since not all users of wasm are intimately familiar with proposals and whether or not engines support them, but it's a change nonetheless worth mentioning.

@laizy
Copy link
Author

laizy commented Apr 3, 2023

I have tried to use -Z build-std=core, alloc, still not work:

RUSTFLAGS="-C link-arg=-zstack-size=32768 -C target-feature=-mutable-globals,-sign-ext,-multivalue,-simd128" cargo build -Z build-std=core,alloc --release --target=wasm32-unknown-unknown

@alexcrichton Our blockchain platform has used Wasm as a virtual machine running smart contracts and has been running steadily for many years. The MVP opcodes is fully sufficient for our use case. I hope there are ways to turn off these default extension features to avoid frequent upgrades every time a new feature is introduced.

@alexcrichton
Copy link
Member

Could you share the code used to reproduce this?

These upgrades will not be frequent. This is the first time it's changed in 5 years.

@Noratrieb Noratrieb added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Apr 5, 2023
@shanemadden
Copy link

The use case I have for wanting to build without these extensions isn't critical (servers for a game which don't support the extensions in the old version of nodejs they're running), but a way to successfully disable them to be able to build for such environments would be appreciated!

@alexcrichton
Copy link
Member

It would be helpful to have a reproduction of this to run to test out fixes. I tested something locally and -Ctarget-cpu=mvp was sufficient to disable sign extension instructions. I don't know why -C target-feature=-sign-ext isn't working and that may be an LLVM bug. I can't confirm whether this works for others though without a means of reproduction.

@CryZe
Copy link
Contributor

CryZe commented Apr 15, 2023

I just tried the following:

#[no_mangle]
pub extern "C" fn foo(a: &i8, b: &i8) -> i32 {
    (*a + *b) as _
}

compiled with:
RUSTFLAGS="-C target-feature=-sign-ext" cargo build --target wasm32-unknown-unknown --release -Z build-std=std,panic_abort -Z build-std-features=panic_immediate_abort

to make sure std being precompiled isn't causing problems, and it totally still contains the sign-ext instructions:

(func (;0;) (type 0) (param i32 i32) (result i32)
    local.get 1
    i32.load8_u
    local.get 0
    i32.load8_u
    i32.add
    i32.extend8_s)

So you are right, this is some sort of LLVM bug.

@shanemadden
Copy link

Confirming that -Ctarget-cpu=mvp results in a file without the sign-ext opcodes, while -Ctarget-feature=-sign-ext does not, in my testing as well.

@alexcrichton
Copy link
Member

@sunfishcode would you perhaps know more about this behavior? I would expect that with -sign-ext on the function then even without -mcpu=mvp sign-extension instructions wouldn't get generated, but I'm not sure.

@sunfishcode
Copy link
Member

I wonder if this is a consequence of https://reviews.llvm.org/D59625. It looks like that might be unioning the module-level features along with the per-function features to form the effective feature set to use.

casperlabs-bors-ng bot added a commit to casper-network/casper-node that referenced this issue Jun 2, 2023
4008: Updates toolchain to a more recent 2023-03-25 r=mpapierski a=mpapierski

This bumps a toolchain for contract API to more recent 2023-03-25. This is not the most recent, as the current nightly appears to produce non-mvp WebAssembly opcodes which are not supported on our platform. Apparently this version update also decerases gas cost on average in our tests.

See also upstream compiler issue here: rust-lang/rust#109807

Co-authored-by: Michał Papierski <michal@casperlabs.io>
@kvinwang
Copy link

kvinwang commented Jun 3, 2023

I just tried the following:

#[no_mangle]
pub extern "C" fn foo(a: &i8, b: &i8) -> i32 {
    (*a + *b) as _
}

compiled with: RUSTFLAGS="-C target-feature=-sign-ext" cargo build --target wasm32-unknown-unknown --release -Z build-std=std,panic_abort -Z build-std-features=panic_immediate_abort

to make sure std being precompiled isn't causing problems, and it totally still contains the sign-ext instructions:

(func (;0;) (type 0) (param i32 i32) (result i32)
    local.get 1
    i32.load8_u
    local.get 0
    i32.load8_u
    i32.add
    i32.extend8_s)

So you are right, this is some sort of LLVM bug.

It can also be reproduced with: RUSTFLAGS="-C linker-plugin-lto -C target-cpu=mvp" cargo build --target wasm32-unknown-unknown --release -Z build-std=std,panic_abort -Z build-std-features=panic_immediate_abort

wip-sync pushed a commit to NetBSD/pkgsrc-wip that referenced this issue Jun 3, 2023
Pkgsrc changes:
 * Adjust patches and cargo checksums to new versions.
 * Adjust to not cross-build to 8.0, due to LLVM using c++17,
   so adjust USE_LANGUAGES.

Upstream changes:

Version 1.70.0 (2023-06-01)
==========================

Language
--------
- [Relax ordering rules for `asm!` operands]
  (rust-lang/rust#105798)
- [Properly allow macro expanded `format_args` invocations to uses
  captures] (rust-lang/rust#106505)
- [Lint ambiguous glob re-exports]
  (rust-lang/rust#107880)
- [Perform const and unsafe checking for expressions in `let _ =
  expr` position.]
  (rust-lang/rust#102256)

Compiler
--------
- [Extend -Cdebuginfo with new options and named aliases]
  (rust-lang/rust#109808)
  This provides a smaller version of debuginfo for cases that only
  need line number information (`-Cdebuginfo=line-tables-only`),
  which may eventually become the default for `-Cdebuginfo=1`.
- [Make `unused_allocation` lint against `Box::new` too]
  (rust-lang/rust#104363)
- [Detect uninhabited types early in const eval]
  (rust-lang/rust#109435)
- [Switch to LLD as default linker for {arm,thumb}v4t-none-eabi]
  (rust-lang/rust#109721)
- [Add tier 3 target `loongarch64-unknown-linux-gnu`]
  (rust-lang/rust#96971)
- [Add tier 3 target for `i586-pc-nto-qnx700` (QNX Neutrino RTOS,
  version 7.0)] (rust-lang/rust#109173),
- [Insert alignment checks for pointer dereferences as debug assertions]
  (rust-lang/rust#98112)
  This catches undefined behavior at runtime, and may cause existing
  code to fail.

Refer to Rust's [platform support page][platform-support-doc]
for more information on Rust's tiered platform support.

Libraries
---------
- [Document NonZeroXxx layout guarantees]
  (rust-lang/rust#94786)
- [Windows: make `Command` prefer non-verbatim paths]
  (rust-lang/rust#96391)
- [Implement Default for some alloc/core iterators]
  (rust-lang/rust#99929)
- [Fix handling of trailing bare CR in str::lines]
  (rust-lang/rust#100311)
- [allow negative numeric literals in `concat!`]
  (rust-lang/rust#106844)
- [Add documentation about the memory layout of `Cell`]
  (rust-lang/rust#106921)
- [Use `partial_cmp` to implement tuple `lt`/`le`/`ge`/`gt`]
  (rust-lang/rust#108157)
- [Stabilize `atomic_as_ptr`]
  (rust-lang/rust#108419)
- [Stabilize `nonnull_slice_from_raw_parts`]
  (rust-lang/rust#97506)
- [Partial stabilization of `once_cell`]
  (rust-lang/rust#105587)
- [Stabilize `nonzero_min_max`]
  (rust-lang/rust#106633)
- [Flatten/inline format_args!() and (string and int) literal
  arguments into format_args!()]
  (rust-lang/rust#106824)
- [Stabilize movbe target feature]
  (rust-lang/rust#107711)
- [don't splice from files into pipes in io::copy]
  (rust-lang/rust#108283)
- [Add a builtin unstable `FnPtr` trait that is implemented for
  all function pointers]
  (rust-lang/rust#108080)
  This extends `Debug`, `Pointer`, `Hash`, `PartialEq`, `Eq`,
  `PartialOrd`, and `Ord` implementations for function pointers
  with all ABIs.

Stabilized APIs
---------------

- [`NonZero*::MIN/MAX`]
  (https://doc.rust-lang.org/stable/std/num/struct.NonZeroI8.html#associatedconstant.MIN)
- [`BinaryHeap::retain`]
  (https://doc.rust-lang.org/stable/std/collections/struct.BinaryHeap.html#method.retain)
- [`Default for std::collections::binary_heap::IntoIter`]
  (https://doc.rust-lang.org/stable/std/collections/binary_heap/struct.IntoIter.html)
- [`Default for std::collections::btree_map::{IntoIter, Iter, IterMut}`]
  (https://doc.rust-lang.org/stable/std/collections/btree_map/struct.IntoIter.html)
- [`Default for std::collections::btree_map::{IntoKeys, Keys}`]
  (https://doc.rust-lang.org/stable/std/collections/btree_map/struct.IntoKeys.html)
- [`Default for std::collections::btree_map::{IntoValues, Values}`]
  (https://doc.rust-lang.org/stable/std/collections/btree_map/struct.IntoKeys.html)
- [`Default for std::collections::btree_map::Range`]
  (https://doc.rust-lang.org/stable/std/collections/btree_map/struct.Range.html)
- [`Default for std::collections::btree_set::{IntoIter, Iter}`]
  (https://doc.rust-lang.org/stable/std/collections/btree_set/struct.IntoIter.html)
- [`Default for std::collections::btree_set::Range`]
  (https://doc.rust-lang.org/stable/std/collections/btree_set/struct.Range.html)
- [`Default for std::collections::linked_list::{IntoIter, Iter, IterMut}`]
  (https://doc.rust-lang.org/stable/alloc/collections/linked_list/struct.IntoIter.html)
- [`Default for std::vec::IntoIter`]
  (https://doc.rust-lang.org/stable/alloc/vec/struct.IntoIter.html#impl-Default-for-IntoIter%3CT,+A%3E)
- [`Default for std::iter::Chain`]
  (https://doc.rust-lang.org/stable/std/iter/struct.Chain.html)
- [`Default for std::iter::Cloned`]
  (https://doc.rust-lang.org/stable/std/iter/struct.Cloned.html)
- [`Default for std::iter::Copied`]
  (https://doc.rust-lang.org/stable/std/iter/struct.Copied.html)
- [`Default for std::iter::Enumerate`]
  (https://doc.rust-lang.org/stable/std/iter/struct.Enumerate.html)
- [`Default for std::iter::Flatten`]
  (https://doc.rust-lang.org/stable/std/iter/struct.Flatten.html)
- [`Default for std::iter::Fuse`]
  (https://doc.rust-lang.org/stable/std/iter/struct.Fuse.html)
- [`Default for std::iter::Rev`]
  (https://doc.rust-lang.org/stable/std/iter/struct.Rev.html)
- [`Default for std::slice::Iter`]
  (https://doc.rust-lang.org/stable/std/slice/struct.Iter.html)
- [`Default for std::slice::IterMut`]
  (https://doc.rust-lang.org/stable/std/slice/struct.IterMut.html)
- [`Rc::into_inner`]
  (https://doc.rust-lang.org/stable/alloc/rc/struct.Rc.html#method.into_inner)
- [`Arc::into_inner`]
  (https://doc.rust-lang.org/stable/alloc/sync/struct.Arc.html#method.into_inner)
- [`std::cell::OnceCell`]
  (https://doc.rust-lang.org/stable/std/cell/struct.OnceCell.html)
- [`Option::is_some_and`]
  (https://doc.rust-lang.org/stable/std/option/enum.Option.html#method.is_some_and)
- [`NonNull::slice_from_raw_parts`]
  (https://doc.rust-lang.org/stable/std/ptr/struct.NonNull.html#method.slice_from_raw_parts)
- [`Result::is_ok_and`]
  (https://doc.rust-lang.org/stable/std/result/enum.Result.html#method.is_ok_and)
- [`Result::is_err_and`]
  (https://doc.rust-lang.org/stable/std/result/enum.Result.html#method.is_err_and)
- [`std::sync::atomic::Atomic*::as_ptr`]
  (https://doc.rust-lang.org/stable/std/sync/atomic/struct.AtomicU8.html#method.as_ptr)
- [`std::io::IsTerminal`]
  (https://doc.rust-lang.org/stable/std/io/trait.IsTerminal.html)
- [`std::os::linux::net::SocketAddrExt`]
  (https://doc.rust-lang.org/stable/std/os/linux/net/trait.SocketAddrExt.html)
- [`std::os::unix::net::UnixDatagram::bind_addr`]
  (https://doc.rust-lang.org/stable/std/os/unix/net/struct.UnixDatagram.html#method.bind_addr)
- [`std::os::unix::net::UnixDatagram::connect_addr`]
  (https://doc.rust-lang.org/stable/std/os/unix/net/struct.UnixDatagram.html#method.connect_addr)
- [`std::os::unix::net::UnixDatagram::send_to_addr`]
  (https://doc.rust-lang.org/stable/std/os/unix/net/struct.UnixDatagram.html#method.send_to_addr)
- [`std::os::unix::net::UnixListener::bind_addr`]
  (https://doc.rust-lang.org/stable/std/os/unix/net/struct.UnixListener.html#method.bind_addr)
- [`std::path::Path::as_mut_os_str`]
  (https://doc.rust-lang.org/stable/std/path/struct.Path.html#method.as_mut_os_str)
- [`std::sync::OnceLock`]
  (https://doc.rust-lang.org/stable/std/sync/struct.OnceLock.html)

Cargo
-----

- [Add `CARGO_PKG_README`]
  (rust-lang/cargo#11645)
- [Make `sparse` the default protocol for crates.io]
  (rust-lang/cargo#11791)
- [Accurately show status when downgrading dependencies]
  (rust-lang/cargo#11839)
- [Use registry.default for login/logout]
  (rust-lang/cargo#11949)
- [Stabilize `cargo logout`]
  (rust-lang/cargo#11950)

Misc
----

- [Stabilize rustdoc `--test-run-directory`]
  (rust-lang/rust#103682)

Compatibility Notes
-------------------

- [Prevent stable `libtest` from supporting `-Zunstable-options`]
  (rust-lang/rust#109044)
- [Perform const and unsafe checking for expressions in `let _ =
  expr` position.] (rust-lang/rust#102256)
- [WebAssembly targets enable `sign-ext` and `mutable-globals`
  features in codegen] (rust-lang/rust#109807)
  This may cause incompatibility with older execution environments.
- [Insert alignment checks for pointer dereferences as debug assertions]
  (rust-lang/rust#98112)
  This catches undefined behavior at runtime, and may cause existing
  code to fail.

Internal Changes
----------------

These changes do not affect any public interfaces of Rust, but they represent
significant improvements to the performance or internals of rustc and related
tools.

- [Upgrade to LLVM 16]
  (rust-lang/rust#109474)
- [Use SipHash-1-3 instead of SipHash-2-4 for StableHasher]
  (rust-lang/rust#107925)
doubledup added a commit to Snowfork/snowbridge that referenced this issue Jun 12, 2023
This avoids an issue where the sign-ext feature is enabled in Rust >=
1.70.0.

[cargo-contract issue](use-ink/cargo-contract#1139)
[rust issue](rust-lang/rust#109807)
@stephanemagnenat
Copy link

stephanemagnenat commented Jan 11, 2024

A slightly related issue is that the WebAssembly extensions used by default by the Rust compiler are not documented (besides in the change log, but new users won't look at it), I opened a specific issue for that (#119811) and just want to link it here.

peaBerberian added a commit to canalplus/rx-player that referenced this issue Feb 1, 2024
…to make it work on some lg TVs

After testing at large scale our `MULTI_THREAD` feature, we noticed that
some LG and samsung TVs had issues running it.

The problem in question was due to an unrecognized WebAssembly Op Code by
the device linked to the "sign-extensions" proposal
(https://github.com/WebAssembly/sign-extension-ops/blob/master/proposals/sign-extension-ops/Overview.md)
which was not introduced initially with the so-called "WebAssembly MVP"
(initial release of the feature).

It turns out that rustc, the Rust compiler, targets a WebAssembly
version that is a little further than the MVP by default (it seems that
this decision is actually taken by the LLVM project from what I
understand from an answers in
rust-lang/rust#109807) and amongst
the included features is the sign-extensions feature (so the rest of
included feature is totally unclear and I could not find any resource on
the web listing them.

From there, I was able to remove the sign-extensions Op Code from the
built WebAssembly file by doing either one of these two ways:

  1. Through a rustc flag (`-C target-feature=-sign-ext`). It should be
     noted that the inclusion of this flag print a deprecation notice.

     Interestingly, a `-C target-cpu=mvp` flag, which looks like it is
     exactly what we want, is also listed in that page yet it doesn't
     seem to remove the sign-extensions opcode for me.

     Maybe because it takes an already-compiled stdlib and the
     problematic opcode is from there? We could re-build stdlib through
     another flag but this latter one is marked as "unstable" so I
     didn't want to adventure too far into this theory.

  2. By adding the `--signext-lowering` flag to binaryen's wasm-opt tool
     which is documented at:

     > lower sign-ext operations to wasm mvp and disable the sign
     > extension feature

     That solution is nice but I couldn't rely on that flag with the
     binaryen npm module we now rely on (because RxPlayer developers
     most likely have npm installed so it was seen as a simpler
     dependency in the project than binaries brought by a binaryen
     project that has to be installed separately).

     This means that to add this feature, I have to bring back the full
     `binaryen` package as a dependency of the RxPlayer which isn't
     nice.

Even with its drawbacks, I chose to go with the second solution here
because I was afraid due to both the deprecation notices and the fact
that another rustc flag (-C target=mvp) didn't have the expected effect.

Still, this only fix the issue with the `sign-extensions` opcode, and
not the real larger issue which is to either properly handle all devices
supporting WebAssembly or to be able to detect and fallback when a
device fails to compile it.

This hypotetical scenario (for now, though may be more common in the
future), will be handled in a future PR.

It only acts on that feature. Future WebAssembly builds could
break if other new features are not handled by the device.
peaBerberian added a commit to canalplus/rx-player that referenced this issue Feb 1, 2024
…to make it work on some lg TVs

After testing at large scale our `MULTI_THREAD` feature, we noticed that
some LG and samsung TVs had issues running it.

The problem in question was due to an unrecognized WebAssembly Op Code by
the device linked to the "sign-extensions" proposal
(https://github.com/WebAssembly/sign-extension-ops/blob/master/proposals/sign-extension-ops/Overview.md)
which was not introduced initially with the so-called "WebAssembly MVP"
(the initial release of WebAssembly).

It turns out that rustc (the Rust compiler) targets a WebAssembly
version that is a little further than the MVP by default (it seems that
this decision is actually taken by the LLVM project from what I
understand from an answers in
rust-lang/rust#109807) and amongst
the included features is the sign-extensions feature (so the rest of
included feature is totally unclear and I could not find any resource on
the web listing them.

From there, I was able to remove the sign-extensions Op Code from the
built WebAssembly file by doing either one of these two ways:

  1. Through a rustc flag (`-C target-feature=-sign-ext`). It should be
     noted that the inclusion of this flag prints a warning notice
     indicating that the flag is "unstable"

     Interestingly, a `-C target-cpu=mvp` flag, which looks like it is
     exactly what we want, is also listed in that page yet it doesn't
     seem to remove the sign-extensions opcode for me.

     Maybe because it takes an already-compiled stdlib and the
     problematic opcode is from there? We could re-build stdlib through
     another flag but this latter one is marked as "unstable" so I
     didn't want to adventure too far into this theory and I didn't
     either understand why the other flag would have an effect in that
     case.

  2. By adding the `--signext-lowering` flag to binaryen's wasm-opt tool
     which is documented at:

     > lower sign-ext operations to wasm mvp and disable the sign
     > extension feature

     That solution is nice but I couldn't rely on that flag with the
     [binaryen npm module](https://www.npmjs.com/package/binaryen)
     we now rely on (because RxPlayer developers most likely have
     npm installed so it was seen as a simpler dependency in the
     project than binaries brought by a binaryen project that has
     to be installed separately).

     This means that to add this feature, I have to bring back the full
     `binaryen` package as a dependency of the RxPlayer which isn't
     nice.

Even with its drawbacks, I chose to go with the second solution here
because I was afraid due to its warning notice, the fact that the other
rustc flag (-C target=mvp) didn't have the expected effect and the hint
that this might not work when the feature is in stdlib. We may come back
to this in the future.

Still, this only fix the issue with the `sign-extensions` opcode, and
not the real larger issue which is to either properly handle all devices
supporting WebAssembly or to be able to detect and fallback when a
device fails to compile it.

This hypotetical scenario (for now, though may be more common in the
future), will be handled in a future PR.

It only acts on that feature. Future WebAssembly builds could
break if other new features are not handled by the device.
peaBerberian added a commit to canalplus/rx-player that referenced this issue Feb 2, 2024
…to make it work on some lg TVs

After testing at large scale our `MULTI_THREAD` feature, we noticed that
some LG and samsung TVs had issues running it.

The problem in question was due to an unrecognized WebAssembly Op Code by
the device linked to the "sign-extensions" proposal
(https://github.com/WebAssembly/sign-extension-ops/blob/master/proposals/sign-extension-ops/Overview.md)
which was not introduced initially with the so-called "WebAssembly MVP"
(the initial release of WebAssembly).

It turns out that rustc (the Rust compiler) targets a WebAssembly
version that is a little further than the MVP by default (it seems that
this decision is actually taken by the LLVM project from what I
understand from an answers in
rust-lang/rust#109807) and amongst
the included features is the sign-extensions feature (so the rest of
included feature is totally unclear and I could not find any resource on
the web listing them.

From there, I was able to remove the sign-extensions Op Code from the
built WebAssembly file by doing either one of these two ways:

  1. Through a rustc flag (`-C target-feature=-sign-ext`). It should be
     noted that the inclusion of this flag prints a warning notice
     indicating that the flag is "unstable"

     Interestingly, a `-C target-cpu=mvp` flag, which looks like it is
     exactly what we want, is also listed in that page yet it doesn't
     seem to remove the sign-extensions opcode for me.

     Maybe because it takes an already-compiled stdlib and the
     problematic opcode is from there? We could re-build stdlib through
     another flag but this latter one is marked as "unstable" so I
     didn't want to adventure too far into this theory and I didn't
     either understand why the other flag would have an effect in that
     case.

  2. By adding the `--signext-lowering` flag to binaryen's wasm-opt tool
     which is documented at:

     > lower sign-ext operations to wasm mvp and disable the sign
     > extension feature

     That solution is nice but I couldn't rely on that flag with the
     [binaryen npm module](https://www.npmjs.com/package/binaryen)
     we now rely on (because RxPlayer developers most likely have
     npm installed so it was seen as a simpler dependency in the
     project than binaries brought by a binaryen project that has
     to be installed separately).

     This means that to add this feature, I have to bring back the full
     `binaryen` package as a dependency of the RxPlayer which isn't
     nice.

Even with its drawbacks, I chose to go with the second solution here
because I was afraid due to its warning notice, the fact that the other
rustc flag (-C target=mvp) didn't have the expected effect and the hint
that this might not work when the feature is in stdlib. We may come back
to this in the future.

Still, this only fix the issue with the `sign-extensions` opcode, and
not the real larger issue which is to either properly handle all devices
supporting WebAssembly or to be able to detect and fallback when a
device fails to compile it.

This hypotetical scenario (for now, though may be more common in the
future), will be handled in a future PR.

It only acts on that feature. Future WebAssembly builds could
break if other new features are not handled by the device.
peaBerberian added a commit to canalplus/rx-player that referenced this issue Feb 5, 2024
…to make it work on some lg TVs

After testing at large scale our `MULTI_THREAD` feature, we noticed that
some LG and samsung TVs had issues running it.

The problem in question was due to an unrecognized WebAssembly Op Code by
the device linked to the "sign-extensions" proposal
(https://github.com/WebAssembly/sign-extension-ops/blob/master/proposals/sign-extension-ops/Overview.md)
which was not introduced initially with the so-called "WebAssembly MVP"
(the initial release of WebAssembly).

It turns out that rustc (the Rust compiler) targets a WebAssembly
version that is a little further than the MVP by default (it seems that
this decision is actually taken by the LLVM project from what I
understand from an answers in
rust-lang/rust#109807) and amongst
the included features is the sign-extensions feature (so the rest of
included feature is totally unclear and I could not find any resource on
the web listing them.

From there, I was able to remove the sign-extensions Op Code from the
built WebAssembly file by doing either one of these two ways:

  1. Through a rustc flag (`-C target-feature=-sign-ext`). It should be
     noted that the inclusion of this flag prints a warning notice
     indicating that the flag is "unstable"

     Interestingly, a `-C target-cpu=mvp` flag, which looks like it is
     exactly what we want, is also listed in that page yet it doesn't
     seem to remove the sign-extensions opcode for me.

     Maybe because it takes an already-compiled stdlib and the
     problematic opcode is from there? We could re-build stdlib through
     another flag but this latter one is marked as "unstable" so I
     didn't want to adventure too far into this theory and I didn't
     either understand why the other flag would have an effect in that
     case.

  2. By adding the `--signext-lowering` flag to binaryen's wasm-opt tool
     which is documented at:

     > lower sign-ext operations to wasm mvp and disable the sign
     > extension feature

     That solution is nice but I couldn't rely on that flag with the
     [binaryen npm module](https://www.npmjs.com/package/binaryen)
     we now rely on (because RxPlayer developers most likely have
     npm installed so it was seen as a simpler dependency in the
     project than binaries brought by a binaryen project that has
     to be installed separately).

     This means that to add this feature, I have to bring back the full
     `binaryen` package as a dependency of the RxPlayer which isn't
     nice.

Even with its drawbacks, I chose to go with the second solution here
because I was afraid due to its warning notice, the fact that the other
rustc flag (-C target=mvp) didn't have the expected effect and the hint
that this might not work when the feature is in stdlib. We may come back
to this in the future.

Still, this only fix the issue with the `sign-extensions` opcode, and
not the real larger issue which is to either properly handle all devices
supporting WebAssembly or to be able to detect and fallback when a
device fails to compile it.

This hypotetical scenario (for now, though may be more common in the
future), will be handled in a future PR.

It only acts on that feature. Future WebAssembly builds could
break if other new features are not handled by the device.
peaBerberian added a commit to canalplus/rx-player that referenced this issue Feb 5, 2024
…to make it work on some lg TVs

After testing at large scale our `MULTI_THREAD` feature, we noticed that
some LG and samsung TVs had issues running it.

The problem in question was due to an unrecognized WebAssembly Op Code by
the device linked to the "sign-extensions" proposal
(https://github.com/WebAssembly/sign-extension-ops/blob/master/proposals/sign-extension-ops/Overview.md)
which was not introduced initially with the so-called "WebAssembly MVP"
(the initial release of WebAssembly).

It turns out that rustc (the Rust compiler) targets a WebAssembly
version that is a little further than the MVP by default (it seems that
this decision is actually taken by the LLVM project from what I
understand from an answers in
rust-lang/rust#109807) and amongst
the included features is the sign-extensions feature (so the rest of
included feature is totally unclear and I could not find any resource on
the web listing them.

From there, I was able to remove the sign-extensions Op Code from the
built WebAssembly file by doing either one of these two ways:

  1. Through a rustc flag (`-C target-feature=-sign-ext`). It should be
     noted that the inclusion of this flag prints a warning notice
     indicating that the flag is "unstable"

     Interestingly, a `-C target-cpu=mvp` flag, which looks like it is
     exactly what we want, is also listed in that page yet it doesn't
     seem to remove the sign-extensions opcode for me.

     Maybe because it takes an already-compiled stdlib and the
     problematic opcode is from there? We could re-build stdlib through
     another flag but this latter one is marked as "unstable" so I
     didn't want to adventure too far into this theory and I didn't
     either understand why the other flag would have an effect in that
     case.

  2. By adding the `--signext-lowering` flag to binaryen's wasm-opt tool
     which is documented at:

     > lower sign-ext operations to wasm mvp and disable the sign
     > extension feature

     That solution is nice but I couldn't rely on that flag with the
     [binaryen npm module](https://www.npmjs.com/package/binaryen)
     we now rely on (because RxPlayer developers most likely have
     npm installed so it was seen as a simpler dependency in the
     project than binaries brought by a binaryen project that has
     to be installed separately).

     This means that to add this feature, I have to bring back the full
     `binaryen` package as a dependency of the RxPlayer which isn't
     nice.

Even with its drawbacks, I chose to go with the second solution here
because I was afraid due to its warning notice, the fact that the other
rustc flag (-C target=mvp) didn't have the expected effect and the hint
that this might not work when the feature is in stdlib. We may come back
to this in the future.

Still, this only fix the issue with the `sign-extensions` opcode, and
not the real larger issue which is to either properly handle all devices
supporting WebAssembly or to be able to detect and fallback when a
device fails to compile it.

This hypotetical scenario (for now, though may be more common in the
future), will be handled in a future PR.

It only acts on that feature. Future WebAssembly builds could
break if other new features are not handled by the device.
poplexity added a commit to poplexity/rust-contracts-builder that referenced this issue Apr 12, 2024
Recent rust versions include this instruction which is not part of WASM 1.0/mvp, see rust-lang/rust#109807 for more info.
@alexcrichton
Copy link
Member

As a heads up I've added documentation about features to #128511 which I've currently got as flagged to close this issue. The documented solution for targetting MVP wasm is:

$ export RUSTFLAG=-Ctarget-cpu=mvp
$ cargo +nightly build -Zbuild-std=panic_abort,std --target wasm32-unknown-unknown

which should solve the original OP of this issue. Other solutions in this issue such as using -Ctarget-feature=-sign-ext or that in conjunction with -Clto or others don't work for one reason or another. Using the mvp target-cpu, however, plus recompiling the standard library with -Zbuild-std, does work.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Aug 22, 2024
…jieyouxu

Document WebAssembly target feature expectations

This commit is a result of the discussion on rust-lang#128475 and incorporates parts of rust-lang#109807 as well. This is all done as a new page of documentation for the `wasm32-unknown-unknown` target which previously did not exist. This new page goes into details about the preexisting target and additionally documents the expectations for WebAssembly features and code generation.

The tl;dr is that LLVM will enable features over time after most engines have had support for awhile. Compiling without features requires `-Ctarget-cpu=mvp` to rustc plus `-Zbuild-std` to Cargo.

Closes rust-lang#109807
Closes rust-lang#119811
Closes rust-lang#128475
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Aug 22, 2024
…jieyouxu

Document WebAssembly target feature expectations

This commit is a result of the discussion on rust-lang#128475 and incorporates parts of rust-lang#109807 as well. This is all done as a new page of documentation for the `wasm32-unknown-unknown` target which previously did not exist. This new page goes into details about the preexisting target and additionally documents the expectations for WebAssembly features and code generation.

The tl;dr is that LLVM will enable features over time after most engines have had support for awhile. Compiling without features requires `-Ctarget-cpu=mvp` to rustc plus `-Zbuild-std` to Cargo.

Closes rust-lang#109807
Closes rust-lang#119811
Closes rust-lang#128475
tgross35 added a commit to tgross35/rust that referenced this issue Aug 23, 2024
…jieyouxu

Document WebAssembly target feature expectations

This commit is a result of the discussion on rust-lang#128475 and incorporates parts of rust-lang#109807 as well. This is all done as a new page of documentation for the `wasm32-unknown-unknown` target which previously did not exist. This new page goes into details about the preexisting target and additionally documents the expectations for WebAssembly features and code generation.

The tl;dr is that LLVM will enable features over time after most engines have had support for awhile. Compiling without features requires `-Ctarget-cpu=mvp` to rustc plus `-Zbuild-std` to Cargo.

Closes rust-lang#109807
Closes rust-lang#119811
Closes rust-lang#128475
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Aug 23, 2024
…jieyouxu

Document WebAssembly target feature expectations

This commit is a result of the discussion on rust-lang#128475 and incorporates parts of rust-lang#109807 as well. This is all done as a new page of documentation for the `wasm32-unknown-unknown` target which previously did not exist. This new page goes into details about the preexisting target and additionally documents the expectations for WebAssembly features and code generation.

The tl;dr is that LLVM will enable features over time after most engines have had support for awhile. Compiling without features requires `-Ctarget-cpu=mvp` to rustc plus `-Zbuild-std` to Cargo.

Closes rust-lang#109807
Closes rust-lang#119811
Closes rust-lang#128475
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Aug 23, 2024
…jieyouxu

Document WebAssembly target feature expectations

This commit is a result of the discussion on rust-lang#128475 and incorporates parts of rust-lang#109807 as well. This is all done as a new page of documentation for the `wasm32-unknown-unknown` target which previously did not exist. This new page goes into details about the preexisting target and additionally documents the expectations for WebAssembly features and code generation.

The tl;dr is that LLVM will enable features over time after most engines have had support for awhile. Compiling without features requires `-Ctarget-cpu=mvp` to rustc plus `-Zbuild-std` to Cargo.

Closes rust-lang#109807
Closes rust-lang#119811
Closes rust-lang#128475
@bors bors closed this as completed in e65a48e Aug 23, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Aug 23, 2024
Rollup merge of rust-lang#128511 - alexcrichton:doc-wasm-features, r=jieyouxu

Document WebAssembly target feature expectations

This commit is a result of the discussion on rust-lang#128475 and incorporates parts of rust-lang#109807 as well. This is all done as a new page of documentation for the `wasm32-unknown-unknown` target which previously did not exist. This new page goes into details about the preexisting target and additionally documents the expectations for WebAssembly features and code generation.

The tl;dr is that LLVM will enable features over time after most engines have had support for awhile. Compiling without features requires `-Ctarget-cpu=mvp` to rustc plus `-Zbuild-std` to Cargo.

Closes rust-lang#109807
Closes rust-lang#119811
Closes rust-lang#128475
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-codegen Area: Code generation A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. A-target-feature Area: Enabling/disabling target features like AVX, Neon, etc. C-bug Category: This is a bug. O-wasm Target: WASM (WebAssembly), http://webassembly.org/ T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.