wasm32: require 'simd128' to be enabled at compile time

It turns out that providing routines with `#[target_feature(enable = "simd128")]` on `wasm32` can fail in some older browsers. The specific problem is not totally clear to me, but it is straight-forward enough to fix (I hope) by just requiring that `simd128` be enabled at compile time in order to include the `wasm32` SIMD modules in this crate. This would not be a great solution if WASM supported runtime CPU feature detection. And the status quo is that `simd128` has to be enabled at compile time anyway for the SIMD code to take effect. So this shouldn't cause any regressions and is likely something we can do long term as well. We can re-evaluate once and if WASM gets support for runtime CPU feature detection. We also add a CI test for `wasm32` *without* the `simd128` target feature enabled. Fixes #144
BurntSushi · Mar 27, 2024 · 30652e7 · 30652e7
1 parent 0d9d383
commit 30652e7
Show file tree

Hide file tree

Showing 8 changed files with 73 additions and 54 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -152,7 +152,40 @@ jobs:
         curl -LO https://github.com/bytecodealliance/wasmtime/releases/download/v$WASMTIME_VERSION/wasmtime-v$WASMTIME_VERSION-x86_64-linux.tar.xz
         tar xvf wasmtime-v$WASMTIME_VERSION-x86_64-linux.tar.xz
         echo `pwd`/wasmtime-v$WASMTIME_VERSION-x86_64-linux >> $GITHUB_PATH
-        echo "CARGO_TARGET_WASM32_WASI_RUNNER=wasmtime run --wasm-features simd --" >> $GITHUB_ENV
+        echo "CARGO_TARGET_WASM32_WASI_RUNNER=wasmtime run --wasm simd --" >> $GITHUB_ENV
+    - name: Basic build
+      run: cargo build --verbose
+    - name: Run tests
+      run: cargo test --verbose
+    - name: Run with only 'alloc' enabled
+      run: cargo test --verbose --no-default-features --features alloc
+    - name: Run tests without any features enabled (core-only)
+      run: cargo test --verbose --no-default-features
+
+  # Setup and run tests on the wasm32-wasi target via wasmtime, but without
+  # simd128 enabled.
+  wasm-no-simd128:
+    runs-on: ubuntu-latest
+    env:
+      # The version of wasmtime to download and install.
+      WASMTIME_VERSION: 15.0.1
+    steps:
+    - name: Checkout repository
+      uses: actions/checkout@v4
+    - name: Install Rust
+      uses: dtolnay/rust-toolchain@master
+      with:
+        toolchain: stable
+    - name: Add wasm32-wasi target
+      run: rustup target add wasm32-wasi
+    - name: Download and install Wasmtime
+      run: |
+        echo "CARGO_BUILD_TARGET=wasm32-wasi" >> $GITHUB_ENV
+        echo "RUSTFLAGS=-Ctarget-feature=-simd128" >> $GITHUB_ENV
+        curl -LO https://github.com/bytecodealliance/wasmtime/releases/download/v$WASMTIME_VERSION/wasmtime-v$WASMTIME_VERSION-x86_64-linux.tar.xz
+        tar xvf wasmtime-v$WASMTIME_VERSION-x86_64-linux.tar.xz
+        echo `pwd`/wasmtime-v$WASMTIME_VERSION-x86_64-linux >> $GITHUB_PATH
+        echo "CARGO_TARGET_WASM32_WASI_RUNNER=wasmtime run --" >> $GITHUB_ENV
     - name: Basic build
       run: cargo build --verbose
     - name: Run tests

diff --git a/src/arch/mod.rs b/src/arch/mod.rs
@@ -10,7 +10,7 @@ pub(crate) mod generic;
 
 #[cfg(target_arch = "aarch64")]
 pub mod aarch64;
-#[cfg(target_arch = "wasm32")]
+#[cfg(all(target_arch = "wasm32", target_feature = "simd128"))]
 pub mod wasm32;
 #[cfg(target_arch = "x86_64")]
 pub mod x86_64;
diff --git a/src/arch/wasm32/memchr.rs b/src/arch/wasm32/memchr.rs
@@ -8,27 +8,14 @@ available for `wasm32` targets.)
 
 macro_rules! defraw {
     ($ty:ident, $find:ident, $start:ident, $end:ident, $($needles:ident),+) => {{
-        #[cfg(target_feature = "simd128")]
-        {
-            use crate::arch::wasm32::simd128::memchr::$ty;
+        use crate::arch::wasm32::simd128::memchr::$ty;
 
-            debug!("chose simd128 for {}", stringify!($ty));
-            debug_assert!($ty::is_available());
-            // SAFETY: We know that wasm memchr is always available whenever
-            // code is compiled for `wasm32` with the `simd128` target feature
-            // enabled.
-            $ty::new_unchecked($($needles),+).$find($start, $end)
-        }
-        #[cfg(not(target_feature = "simd128"))]
-        {
-            use crate::arch::all::memchr::$ty;
-
-            debug!(
-                "no simd128 feature available, using fallback for {}",
-                stringify!($ty),
-            );
-            $ty::new($($needles),+).$find($start, $end)
-        }
+        debug!("chose simd128 for {}", stringify!($ty));
+        debug_assert!($ty::is_available());
+        // SAFETY: We know that wasm memchr is always available whenever
+        // code is compiled for `wasm32` with the `simd128` target feature
+        // enabled.
+        $ty::new_unchecked($($needles),+).$find($start, $end)
     }}
 }
 

diff --git a/src/arch/wasm32/simd128/packedpair.rs b/src/arch/wasm32/simd128/packedpair.rs
@@ -81,14 +81,13 @@ impl Finder {
     /// true then it will always return true.
     #[inline]
     pub fn is_available() -> bool {
-        #[cfg(target_feature = "simd128")]
-        {
-            true
-        }
-        #[cfg(not(target_feature = "simd128"))]
-        {
-            false
-        }
+        // We used to gate on `cfg(target_feature = "simd128")` here, but
+        // we've since required the feature to be enabled at compile time to
+        // even include this module at all. Therefore, it is always enabled
+        // in this context. See the linked issue for why this was changed.
+        //
+        // Ref: https://github.com/BurntSushi/memchr/issues/144
+        true
     }
 
     /// Execute a search using wasm32 v128 vectors and routines.

diff --git a/src/lib.rs b/src/lib.rs
@@ -177,7 +177,7 @@ Originally, this crate was literally just a safe wrapper function around the
 #![cfg_attr(
     not(any(
         all(target_arch = "x86_64", target_feature = "sse2"),
-        target_arch = "wasm32",
+        all(target_arch = "wasm32", target_feature = "simd128"),
         target_arch = "aarch64",
     )),
     allow(dead_code)

diff --git a/src/memchr.rs b/src/memchr.rs
@@ -514,7 +514,7 @@ unsafe fn memchr_raw(
         // nor SSE2 (unusual) are available.
         crate::arch::x86_64::memchr::memchr_raw(needle, start, end)
     }
-    #[cfg(target_arch = "wasm32")]
+    #[cfg(all(target_arch = "wasm32", target_feature = "simd128"))]
     {
         crate::arch::wasm32::memchr::memchr_raw(needle, start, end)
     }
@@ -524,7 +524,7 @@ unsafe fn memchr_raw(
     }
     #[cfg(not(any(
         target_arch = "x86_64",
-        target_arch = "wasm32",
+        all(target_arch = "wasm32", target_feature = "simd128"),
         target_arch = "aarch64"
     )))]
     {
@@ -547,7 +547,7 @@ unsafe fn memrchr_raw(
     {
         crate::arch::x86_64::memchr::memrchr_raw(needle, start, end)
     }
-    #[cfg(target_arch = "wasm32")]
+    #[cfg(all(target_arch = "wasm32", target_feature = "simd128"))]
     {
         crate::arch::wasm32::memchr::memrchr_raw(needle, start, end)
     }
@@ -557,7 +557,7 @@ unsafe fn memrchr_raw(
     }
     #[cfg(not(any(
         target_arch = "x86_64",
-        target_arch = "wasm32",
+        all(target_arch = "wasm32", target_feature = "simd128"),
         target_arch = "aarch64"
     )))]
     {
@@ -581,7 +581,7 @@ unsafe fn memchr2_raw(
     {
         crate::arch::x86_64::memchr::memchr2_raw(needle1, needle2, start, end)
     }
-    #[cfg(target_arch = "wasm32")]
+    #[cfg(all(target_arch = "wasm32", target_feature = "simd128"))]
     {
         crate::arch::wasm32::memchr::memchr2_raw(needle1, needle2, start, end)
     }
@@ -591,7 +591,7 @@ unsafe fn memchr2_raw(
     }
     #[cfg(not(any(
         target_arch = "x86_64",
-        target_arch = "wasm32",
+        all(target_arch = "wasm32", target_feature = "simd128"),
         target_arch = "aarch64"
     )))]
     {
@@ -616,7 +616,7 @@ unsafe fn memrchr2_raw(
     {
         crate::arch::x86_64::memchr::memrchr2_raw(needle1, needle2, start, end)
     }
-    #[cfg(target_arch = "wasm32")]
+    #[cfg(all(target_arch = "wasm32", target_feature = "simd128"))]
     {
         crate::arch::wasm32::memchr::memrchr2_raw(needle1, needle2, start, end)
     }
@@ -628,7 +628,7 @@ unsafe fn memrchr2_raw(
     }
     #[cfg(not(any(
         target_arch = "x86_64",
-        target_arch = "wasm32",
+        all(target_arch = "wasm32", target_feature = "simd128"),
         target_arch = "aarch64"
     )))]
     {
@@ -656,7 +656,7 @@ unsafe fn memchr3_raw(
             needle1, needle2, needle3, start, end,
         )
     }
-    #[cfg(target_arch = "wasm32")]
+    #[cfg(all(target_arch = "wasm32", target_feature = "simd128"))]
     {
         crate::arch::wasm32::memchr::memchr3_raw(
             needle1, needle2, needle3, start, end,
@@ -670,7 +670,7 @@ unsafe fn memchr3_raw(
     }
     #[cfg(not(any(
         target_arch = "x86_64",
-        target_arch = "wasm32",
+        all(target_arch = "wasm32", target_feature = "simd128"),
         target_arch = "aarch64"
     )))]
     {
@@ -698,7 +698,7 @@ unsafe fn memrchr3_raw(
             needle1, needle2, needle3, start, end,
         )
     }
-    #[cfg(target_arch = "wasm32")]
+    #[cfg(all(target_arch = "wasm32", target_feature = "simd128"))]
     {
         crate::arch::wasm32::memchr::memrchr3_raw(
             needle1, needle2, needle3, start, end,
@@ -712,7 +712,7 @@ unsafe fn memrchr3_raw(
     }
     #[cfg(not(any(
         target_arch = "x86_64",
-        target_arch = "wasm32",
+        all(target_arch = "wasm32", target_feature = "simd128"),
         target_arch = "aarch64"
     )))]
     {
@@ -732,7 +732,7 @@ unsafe fn count_raw(needle: u8, start: *const u8, end: *const u8) -> usize {
     {
         crate::arch::x86_64::memchr::count_raw(needle, start, end)
     }
-    #[cfg(target_arch = "wasm32")]
+    #[cfg(all(target_arch = "wasm32", target_feature = "simd128"))]
     {
         crate::arch::wasm32::memchr::count_raw(needle, start, end)
     }
@@ -742,7 +742,7 @@ unsafe fn count_raw(needle: u8, start: *const u8, end: *const u8) -> usize {
     }
     #[cfg(not(any(
         target_arch = "x86_64",
-        target_arch = "wasm32",
+        all(target_arch = "wasm32", target_feature = "simd128"),
         target_arch = "aarch64"
     )))]
     {

diff --git a/src/memmem/searcher.rs b/src/memmem/searcher.rs
@@ -5,7 +5,7 @@ use crate::arch::all::{
 
 #[cfg(target_arch = "aarch64")]
 use crate::arch::aarch64::neon::packedpair as neon;
-#[cfg(target_arch = "wasm32")]
+#[cfg(all(target_arch = "wasm32", target_feature = "simd128"))]
 use crate::arch::wasm32::simd128::packedpair as simd128;
 #[cfg(all(target_arch = "x86_64", target_feature = "sse2"))]
 use crate::arch::x86_64::{
@@ -109,7 +109,7 @@ impl Searcher {
                 Searcher::twoway(needle, rabinkarp, prestrat)
             }
         }
-        #[cfg(target_arch = "wasm32")]
+        #[cfg(all(target_arch = "wasm32", target_feature = "simd128"))]
         {
             if let Some(pp) = simd128::Finder::with_pair(needle, pair) {
                 if do_packed_search(needle) {
@@ -151,7 +151,7 @@ impl Searcher {
         }
         #[cfg(not(any(
             all(target_arch = "x86_64", target_feature = "sse2"),
-            target_arch = "wasm32",
+            all(target_arch = "wasm32", target_feature = "simd128"),
             target_arch = "aarch64"
         )))]
         {
@@ -251,7 +251,7 @@ union SearcherKind {
     sse2: crate::arch::x86_64::sse2::packedpair::Finder,
     #[cfg(all(target_arch = "x86_64", target_feature = "sse2"))]
     avx2: crate::arch::x86_64::avx2::packedpair::Finder,
-    #[cfg(target_arch = "wasm32")]
+    #[cfg(all(target_arch = "wasm32", target_feature = "simd128"))]
     simd128: crate::arch::wasm32::simd128::packedpair::Finder,
     #[cfg(target_arch = "aarch64")]
     neon: crate::arch::aarch64::neon::packedpair::Finder,
@@ -400,7 +400,7 @@ unsafe fn searcher_kind_avx2(
 /// # Safety
 ///
 /// Callers must ensure that the `searcher.kind.simd128` union field is set.
-#[cfg(target_arch = "wasm32")]
+#[cfg(all(target_arch = "wasm32", target_feature = "simd128"))]
 unsafe fn searcher_kind_simd128(
     searcher: &Searcher,
     _prestate: &mut PrefilterState,
@@ -671,7 +671,7 @@ impl Prefilter {
     }
 
     /// Return a prefilter using a wasm32 simd128 vector algorithm.
-    #[cfg(target_arch = "wasm32")]
+    #[cfg(all(target_arch = "wasm32", target_feature = "simd128"))]
     #[inline]
     fn simd128(finder: simd128::Finder, needle: &[u8]) -> Prefilter {
         trace!("building wasm32 simd128 prefilter");
@@ -761,7 +761,7 @@ union PrefilterKind {
     sse2: crate::arch::x86_64::sse2::packedpair::Finder,
     #[cfg(all(target_arch = "x86_64", target_feature = "sse2"))]
     avx2: crate::arch::x86_64::avx2::packedpair::Finder,
-    #[cfg(target_arch = "wasm32")]
+    #[cfg(all(target_arch = "wasm32", target_feature = "simd128"))]
     simd128: crate::arch::wasm32::simd128::packedpair::Finder,
     #[cfg(target_arch = "aarch64")]
     neon: crate::arch::aarch64::neon::packedpair::Finder,
@@ -833,7 +833,7 @@ unsafe fn prefilter_kind_avx2(
 /// # Safety
 ///
 /// Callers must ensure that the `strat.kind.simd128` union field is set.
-#[cfg(target_arch = "wasm32")]
+#[cfg(all(target_arch = "wasm32", target_feature = "simd128"))]
 unsafe fn prefilter_kind_simd128(
     strat: &Prefilter,
     haystack: &[u8],

diff --git a/src/vector.rs b/src/vector.rs
@@ -464,7 +464,7 @@ mod aarch64neon {
     }
 }
 
-#[cfg(target_arch = "wasm32")]
+#[cfg(all(target_arch = "wasm32", target_feature = "simd128"))]
 mod wasm_simd128 {
     use core::arch::wasm32::*;