llogiq / bytecount

Counting occurrences of a given byte or UTF-8 characters in a slice of memory – fast
Apache License 2.0
225 stars 26 forks source link

Version 0.6.5 is broken on wasm targets (version 0.6.3 worked) #89

Closed stephanemagnenat closed 1 year ago

stephanemagnenat commented 1 year ago

While compiling bytecount as a dependency of nom_locate v4.2.0 for wasm targets, I get the following error:

   Compiling bytecount v0.6.5
error[E0080]: evaluation of `core::arch::wasm32::i32x4_extract_lane::<4>::{constant#0}` failed
    --> /home/steph/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/../../stdarch/crates/core_arch/src/wasm32/simd128.rs:1212:5
     |
1212 |     static_assert!(N < 4);
     |     ^^^^^^^^^^^^^^^^^^^^^ the evaluated program panicked at 'assertion failed: N < 4', /rustc/cc66ad468955717ab92600c770da8c1601a4ff33/library/core/src/../../stdarch/crates/core_arch/src/wasm32/simd128.rs:1212:5
     |
     = note: this error originates in the macro `assert` which comes from the expansion of the macro `static_assert` (in Nightly builds, run with -Z macro-backtrace for more info)

note: the above error was encountered while instantiating `fn core::arch::wasm32::u32x4_extract_lane::<4>`
  --> /home/steph/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bytecount-0.6.5/src/simd/wasm.rs:62:9
   |
62 |         u32x4_extract_lane::<4>(u32s),
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

For more information about this error, try `rustc --explain E0080`.

The minimal reproducible test case is here: https://github.com/stephanemagnenat/bytecount-89

Just do cargo build --target wasm32-unknown-unknown in bytecount-issue-89/ and you get the same error as above (rustc 1.73.0 (cc66ad468 2023-10-03)).

The test case is trivial: The repository is a newly-created project with cargo and with bytecount added as a dependency.

huitseeker commented 1 year ago

Confirmed (CI run). 0.6.4 worked fine.

llogiq commented 1 year ago

I'm totally sorry! Will push a fix shortly and yank the offending version.

llogiq commented 1 year ago

It took two attempts, but I hope 0.6.7 will work for you.

stephanemagnenat commented 1 year ago

Yes 0.6.7 seems to work, so I'm closing the issue.

However, I feel a bit uncomfortable to have so much code with unsafe and SIMD operations that is not unit-tested. As far as I have seen, wasm32 with SIMD is not tested in CI, am I correct? If so, do you think that at some point the tests could be run for wasm32 with SIMD in the CI? Maybe using the infrastructure already present in ci/miri.sh?

llogiq commented 1 year ago

I'm currently having a look at how to incorporate the tests (which need a slightly different setup) into CI. However, I'm also currently down with the flu, so expect results somewhere within the next two weeks.

stephanemagnenat commented 1 year ago

Thank you! Have a quick recovery!