Skip to content

bug: lance-linalg SIMD docs omit the panics and name the wrong popcount algorithm #8861

Description

@LuciferYang

Two documentation defects in lance-linalg, both about SIMD entry points

The six From<&[T]> conversions panic and do not say so

#8593 added an always-on assert! to every slice-to-register conversion in rust/lance-linalg/src/simd/, so a short slice now panics instead of reading out of bounds. None of the six carries a # Panics section:

impl file required length
f32x8 simd/f32.rs:149 8
f32x16 simd/f32.rs:534 16
f64x4 simd/f64.rs:46 4
f64x8 simd/f64.rs:395 8
i32x8 simd/i32.rs:51 8
u8x16 simd/u8.rs:86 16

These are public API on a published crate. From is the conversion a caller reaches for without reading the body, and the signature is infallible, so nothing in the rendered docs warns that f32x8::from(&slice[..]) aborts the thread when the slice is short. The From<&[T; N]> impls next to them take an array reference and cannot be short, so they are correctly silent.

This is the documentation half of #8633, which I closed as a duplicate of #8593 after #8593 landed the asserts. The asserts shipped; the docs did not.

hamming_batch_avx2's doc names an algorithm the code does not use

rust/lance-linalg/src/distance/hamming.rs:515 reads:

/// AVX2 popcount using lookup table (Harley-Seal / PSHUFB method).

The body builds a 16-entry nibble popcount table, splits each byte with _mm256_and_si256 and _mm256_srli_epi16, looks both nibbles up with two _mm256_shuffle_epi8 calls, adds them, and sums bytes with _mm256_sad_epu8. That is the PSHUFB nibble-lookup popcount. Harley-Seal is a different algorithm: a carry-save-adder tree that folds several input words into fewer weighted words before any popcount runs, and there is no such tree here. Each 256-bit chunk is popcounted on its own.

Naming both is not a harmless alias. A reader who knows Harley-Seal will go looking for the CSA tree, and a reader who does not will come away thinking they have seen one.

Suggested fix

Add a # Panics section to each of the six conversions, stating the required length, and drop the wrong algorithm name from the AVX2 doc. Both are comment-only.

Two adjacent gaps I am deliberately not folding in, so they do not get lost: dot_u8.rs, l2_u8.rs, and cosine_u8.rs have zero # Panics and zero # Safety sections between them, though each has always-on length checks and unsafe kernels whose raw loads depend on the lengths matching. That wants one change across all three files, and it has to wait for #8638's fix to land first so that all three are in the same state.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions