Merge pull request #696 from clarfonthey/lint_simd

Lint SIMD code, fix generic expectation failures

Author: clarfonthey

.github/workflows/rust.yml

@@ -11,7 +11,7 @@ jobs:
     - uses: actions/checkout@v3
     - env:
         TARGET: x86_64-unknown-linux-gnu
-      run: sh ci/miri.sh
+      run: ci/miri.sh
 
   rustfmt_clippy:
     runs-on: ubuntu-latest
@@ -21,7 +21,8 @@ jobs:
       run: curl -fsSL https://github.com/tamasfe/taplo/releases/latest/download/taplo-linux-x86_64.gz | gzip -d - | install -m 755 /dev/stdin /usr/local/bin/taplo
     - env:
         TARGET: i586-unknown-linux-gnu
-      run: sh ci/tools.sh
+        CHANNEL: nightly
+      run: ci/tools.sh
 
   basics:
     runs-on: ubuntu-latest
@@ -42,7 +43,7 @@ jobs:
     - env:
         TARGET: ${{ matrix.target }}
         CHANNEL: ${{ matrix.channel }}
-      run: sh ci/run.sh
+      run: ci/run.sh
     strategy:
       matrix:
         os: [ubuntu-latest]

ci/miri.sh

@@ -1,4 +1,4 @@
-#!/usr/bin/env sh
+#!/usr/bin/env bash
 
 set -ex
 
@@ -36,7 +36,68 @@ if retry rustup component add rustfmt ; then
 fi
 
 if retry rustup component add clippy ; then
-    cargo clippy --all --tests --features serde,rayon -- -D warnings
+    # we want all targets that have dedicated SIMD implementations,
+    # plus one that uses the generic implementation,
+    # since we can't lint cfg-disabled code
+    TARGETS=()
+
+    # associative array of cfgs from rustc
+    declare -A CFG
+    for cfg in $(rustc --print cfg); do
+        # bash is very upset about quotes and equal signs in keys
+        cfg="${cfg//\"/}"
+        cfg="${cfg//=/_}"
+        printf "\n%s\n" "$cfg" >&2
+        CFG["$cfg"]=1
+    done
+
+    HOST_IS_GENERIC=1
+
+    # SSE2
+    if (( CFG[target_feature_sse2] && (CFG[target_arch_x86] || CFG[target_arch_x86_64]) )); then
+        printf "\nHost target supports SSE2\n" >&2
+        TARGETS+=(--target "$(rustc --print host-tuple)")
+        HOST_IS_GENERIC=0
+    elif retry rustup target add x86_64-unknown-linux-gnu; then
+        printf "\nTesting x86_64-unknown-linux-gnu\n" >&2
+        TARGETS+=(--target x86_64-unknown-linux-gnu)
+    fi
+
+    # NEON
+    if (( CFG[target_arch_aarch64] && CFG[target_feature_neon] && CFG[target_endian_little] )); then
+        printf "\nHost target supports NEON\n" >&2
+        TARGETS+=(--target "$(rustc --print host-tuple)")
+        HOST_IS_GENERIC=0
+    elif retry rustup target add aarch64-unknown-linux-gnu; then
+        printf "\nTesting aarch64-unknown-linux-gnu\n" >&2
+        TARGETS+=(--target aarch64-unknown-linux-gnu)
+    fi
+
+    # LSX
+    if (( CFG[target_arch_loongarch64] && CFG[target_feature_lsx] )); then
+        printf "\nHost target supports LSX\n" >&2
+        TARGETS+=(--target "$(rustc --print host-tuple)")
+        HOST_IS_GENERIC=0
+    elif retry rustup target add loongarch64-unknown-linux-gnu; then
+        printf "\nTesting loongarch64-unknown-linux-gnu\n" >&2
+        TARGETS+=(--target loongarch64-unknown-linux-gnu)
+    fi
+
+    # Generic
+    if (( HOST_IS_GENERIC )); then
+        printf "\nHost target support is generic\n" >&2
+        TARGETS+=(--target "$(rustc --print host-tuple)")
+    elif retry rustup target add i586-unknown-linux-gnu; then
+        printf "\nTesting i586-unknown-linux-gnu\n" >&2
+        TARGETS+=(--target i586-unknown-linux-gnu)
+    fi
+
+    cargo clippy --all --tests --features serde,rayon "${TARGETS[@]}" -- -D warnings
+
+    # check nightly too
+    if rustc --version | grep --quiet nightly ; then
+        cargo +nightly clippy --all --tests --features serde,rayon,nightly "${TARGETS[@]}" -- -D warnings
+    fi
 fi
 
 if command -v taplo ; then

ci/run.sh

@@ -24,7 +24,6 @@ pub(crate) type BitMaskWord = GroupWord;
 pub(crate) type NonZeroBitMaskWord = NonZeroGroupWord;
 pub(crate) const BITMASK_STRIDE: usize = 8;
 // We only care about the highest bit of each tag for the mask.
-#[expect(clippy::cast_possible_truncation, clippy::unnecessary_cast)]
 const BITMASK_MASK: BitMaskWord = u64::from_ne_bytes([Tag::DELETED.0; 8]) as GroupWord;
 pub(crate) const BITMASK_ITER_MASK: BitMaskWord = !0;
 
@@ -70,15 +69,13 @@ impl Group {
 
     /// Loads a group of tags starting at the given address.
     #[inline]
-    #[expect(clippy::cast_ptr_alignment)] // unaligned load
     pub(crate) unsafe fn load(ptr: *const Tag) -> Self {
         unsafe { Group(ptr::read_unaligned(ptr.cast())) }
     }
 
     /// Loads a group of tags starting at the given address, which must be
     /// aligned to `mem::align_of::<Group>()`.
     #[inline]
-    #[expect(clippy::cast_ptr_alignment)]
     pub(crate) unsafe fn load_aligned(ptr: *const Tag) -> Self {
         debug_assert_eq!(ptr.align_offset(mem::align_of::<Self>()), 0);
         unsafe { Group(ptr::read(ptr.cast())) }
@@ -87,7 +84,6 @@ impl Group {
     /// Stores the group of tags to the given address, which must be
     /// aligned to `mem::align_of::<Group>()`.
     #[inline]
-    #[expect(clippy::cast_ptr_alignment)]
     pub(crate) unsafe fn store_aligned(self, ptr: *mut Tag) {
         debug_assert_eq!(ptr.align_offset(mem::align_of::<Self>()), 0);
         unsafe {

ci/tools.sh

@@ -27,7 +27,6 @@ impl Group {
     ///
     /// This is guaranteed to be aligned to the group size.
     #[inline]
-    #[expect(clippy::items_after_statements)]
     pub(crate) const fn static_empty() -> &'static [Tag; Group::WIDTH] {
         #[repr(C)]
         struct AlignedTags {
@@ -43,15 +42,13 @@ impl Group {
 
     /// Loads a group of tags starting at the given address.
     #[inline]
-    #[expect(clippy::cast_ptr_alignment)] // unaligned load
     pub(crate) unsafe fn load(ptr: *const Tag) -> Self {
         unsafe { Group(lsx_vld::<0>(ptr.cast())) }
     }
 
     /// Loads a group of tags starting at the given address, which must be
     /// aligned to `mem::align_of::<Group>()`.
     #[inline]
-    #[expect(clippy::cast_ptr_alignment)]
     pub(crate) unsafe fn load_aligned(ptr: *const Tag) -> Self {
         debug_assert_eq!(ptr.align_offset(mem::align_of::<Self>()), 0);
         unsafe { Group(lsx_vld::<0>(ptr.cast())) }
@@ -60,7 +57,6 @@ impl Group {
     /// Stores the group of tags to the given address, which must be
     /// aligned to `mem::align_of::<Group>()`.
     #[inline]
-    #[expect(clippy::cast_ptr_alignment)]
     pub(crate) unsafe fn store_aligned(self, ptr: *mut Tag) {
         debug_assert_eq!(ptr.align_offset(mem::align_of::<Self>()), 0);
         unsafe {

src/control/group/generic.rs

@@ -1,3 +1,10 @@
+// TESTING NOTE:
+//
+// Because this module uses `cfg(..)` to select an implementation, it will not
+// be linted without being run on targets that actually load each of these
+// modules. Be sure to edit `ci/tools.sh` to add in the necessary cfgs if you
+// change these, so that your implementation gets properly linted.
+
 cfg_if! {
     // Use the SSE2 implementation if possible: it allows us to scan 16 buckets
     // at once instead of 8. We don't bother with AVX since it would require

src/control/group/lsx.rs

@@ -13,15 +13,12 @@
 #![cfg_attr(
     feature = "nightly",
     feature(
-        test,
         core_intrinsics,
         dropck_eyepatch,
         min_specialization,
         trivial_clone,
         extend_one,
         allocator_api,
-        slice_ptr_get,
-        maybe_uninit_array_assume_init,
         strict_provenance_lints
     )
 )]