From f158fb583a4e7e157d2615555bb7f985b63fff43 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 19:06:35 +0000 Subject: [PATCH 1/6] feat(fastlanes): resolve BitPacked FastLanes kernels once per array BitPacked arrays are type erased, so decoding went through the fastlanes `unchecked_*` entry points, which re-dispatch on the runtime bit width with a 65-arm match for every 1024-value block (and for every `scalar_at`). Add `BitPackedKernels`, a set of function pointers to the const-width kernel instantiations (`unpack`, `unpack_single`, `unfor_pack`), lazily resolved once per array through a `OnceLock` on `BitPackedData` and shared by every decode path: canonicalization, mapped cast, take, filter, scalar_at, is_constant, between, and the fused FoR decompress. The fused compare kernel resolves its `unpack_cmp` instantiation once per call, since it is generic over the comparison closure. The resolved kernels take slices and check block lengths, so the unsafe runtime-width calls are gone from the decoding logic. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01R9mScTPzB4Vw4PCdSyc862 Signed-off-by: Claude --- Cargo.lock | 1 + Cargo.toml | 1 + encodings/fastlanes/Cargo.toml | 1 + .../bitpacking/array/bitpack_decompress.rs | 39 ++- .../fastlanes/src/bitpacking/array/kernels.rs | 292 ++++++++++++++++++ .../fastlanes/src/bitpacking/array/mod.rs | 39 +++ .../src/bitpacking/array/unpack_iter.rs | 82 +++-- .../src/bitpacking/compute/compare.rs | 3 - .../src/bitpacking/compute/compare_fused.rs | 38 ++- .../src/bitpacking/compute/filter.rs | 42 ++- .../fastlanes/src/bitpacking/compute/take.rs | 23 +- encodings/fastlanes/src/bitpacking/mod.rs | 1 + .../fastlanes/src/for/array/for_decompress.rs | 28 +- 13 files changed, 448 insertions(+), 142 deletions(-) create mode 100644 encodings/fastlanes/src/bitpacking/array/kernels.rs diff --git a/Cargo.lock b/Cargo.lock index 3e8ed089b25..84e4a262608 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11050,6 +11050,7 @@ dependencies = [ "prost 0.14.4", "rand 0.10.2", "rstest", + "seq-macro", "vortex-alp", "vortex-array", "vortex-buffer", diff --git a/Cargo.toml b/Cargo.toml index f2e5ca7e000..4872fc745cd 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -243,6 +243,7 @@ rstest = "0.26.1" rstest_reuse = "0.7.0" rustc-hash = "2.1.1" rustix = { version = "1.1", features = ["fs"] } +seq-macro = "0.3.6" serde = "1.0.221" serde_json = "1.0.138" serde_test = "1.0.176" diff --git a/encodings/fastlanes/Cargo.toml b/encodings/fastlanes/Cargo.toml index 9085390b67b..42023005d9a 100644 --- a/encodings/fastlanes/Cargo.toml +++ b/encodings/fastlanes/Cargo.toml @@ -26,6 +26,7 @@ lending-iterator = { workspace = true } num-traits = { workspace = true } prost = { workspace = true } rand = { workspace = true, optional = true } +seq-macro = { workspace = true } vortex-array = { workspace = true } vortex-buffer = { workspace = true } vortex-error = { workspace = true } diff --git a/encodings/fastlanes/src/bitpacking/array/bitpack_decompress.rs b/encodings/fastlanes/src/bitpacking/array/bitpack_decompress.rs index d192e50d04f..6a1f9a2cf4e 100644 --- a/encodings/fastlanes/src/bitpacking/array/bitpack_decompress.rs +++ b/encodings/fastlanes/src/bitpacking/array/bitpack_decompress.rs @@ -3,7 +3,6 @@ use std::mem::MaybeUninit; -use fastlanes::BitPacking; use itertools::Itertools; use num_traits::AsPrimitive; use vortex_array::ArrayView; @@ -23,6 +22,8 @@ use vortex_error::VortexResult; use crate::BitPacked; use crate::BitPackedArrayExt; use crate::FL_CHUNK_SIZE; +use crate::bitpacking::array::kernels::BitPackedKernels; +use crate::bitpacking::array::kernels::BitPackedPhysical; use crate::unpack_iter::BitPacked as BitPackedUnpack; use crate::unpack_iter::BitUnpackedChunks; @@ -161,39 +162,37 @@ pub(crate) fn apply_patches_to_uninit_range, index: usize) -> Scalar { - let bit_width = array.bit_width() as usize; let ptype = array.dtype().as_ptype(); - // let packed = array.packed().into_primitive()?; let index_in_encoded = index + array.offset() as usize; let scalar: Scalar = match_each_unsigned_integer_ptype!(ptype.to_unsigned(), |P| { - unsafe { - unpack_single_primitive::

(array.packed_slice::

(), bit_width, index_in_encoded) - .into() - } + unpack_single_primitive::

( + array.kernels::

(), + array.packed_slice::

(), + index_in_encoded, + ) + .into() }); // Cast to fix signedness and nullability scalar.cast(array.dtype()).vortex_expect("cast failure") } -/// # Safety +/// Unpacks the value at `index_to_decode` from `packed`, a buffer of whole FastLanes blocks +/// packed at the bit width `kernels` were resolved for. /// -/// The caller must ensure the following invariants hold: -/// * `packed.len() == (length + 1023) / 1024 * 128 * bit_width` -/// * `index_to_decode < length` +/// # Panics /// -/// Where `length` is the length of the array/slice backed by `packed` -/// (but is not provided to this function). -pub unsafe fn unpack_single_primitive( - packed: &[T], - bit_width: usize, +/// If `index_to_decode` falls outside the blocks held by `packed`. +pub fn unpack_single_primitive( + kernels: BitPackedKernels

, + packed: &[P], index_to_decode: usize, -) -> T { +) -> P { let chunk_index = index_to_decode / 1024; let index_in_chunk = index_to_decode % 1024; - let elems_per_chunk: usize = 128 * bit_width / size_of::(); + let elems_per_chunk = kernels.packed_block_len(); - let packed_chunk = &packed[chunk_index * elems_per_chunk..][0..elems_per_chunk]; - unsafe { BitPacking::unchecked_unpack_single(bit_width, packed_chunk, index_in_chunk) } + let packed_chunk = &packed[chunk_index * elems_per_chunk..][..elems_per_chunk]; + (kernels.unpack_single)(packed_chunk, index_in_chunk) } pub fn count_exceptions(bit_width: u8, bit_width_freq: &[usize]) -> usize { diff --git a/encodings/fastlanes/src/bitpacking/array/kernels.rs b/encodings/fastlanes/src/bitpacking/array/kernels.rs new file mode 100644 index 00000000000..8955196ff62 --- /dev/null +++ b/encodings/fastlanes/src/bitpacking/array/kernels.rs @@ -0,0 +1,292 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: Copyright the Vortex contributors + +//! Width-resolved FastLanes kernels for bit-packed arrays. +//! +//! The FastLanes kernels are generic over the packed bit width `W`, so the runtime-width +//! `unchecked_*` entry points of the `fastlanes` crate dispatch on the width with a `match` on +//! every call. A [`BitPackedArray`](crate::BitPackedArray) knows its width but is type erased, so +//! it cannot name the instantiation statically. Instead, the array resolves function pointers to +//! the concrete instantiations once, lazily, and hands them out as [`BitPackedKernels`]. The +//! decoding paths then call the resolved kernels block after block without re-dispatching. + +use fastlanes::BitPacking; +use fastlanes::BitPackingCompare; +use fastlanes::FastLanesComparable; +use fastlanes::FoR; +use vortex_array::dtype::NativePType; +use vortex_error::vortex_panic; + +/// Unpacks one FastLanes block of 1024 values. +/// +/// `packed` must hold exactly [`BitPackedKernels::packed_block_len`] elements and `output` +/// exactly 1024. +pub type UnpackFn

= fn(packed: &[P], output: &mut [P]); + +/// Unpacks the value at `index` (`< 1024`) of one packed FastLanes block. +/// +/// `packed` must hold exactly [`BitPackedKernels::packed_block_len`] elements. +pub type UnpackSingleFn

= fn(packed: &[P], index: usize) -> P; + +/// Unpacks one FastLanes block and wrapping-adds `reference` to every value. +/// +/// `packed` must hold exactly [`BitPackedKernels::packed_block_len`] elements and `output` +/// exactly 1024. +pub type UnforPackFn

= fn(packed: &[P], reference: P, output: &mut [P]); + +/// Unpacks one FastLanes block, comparing each value against `rhs` with `cmp` and writing the +/// results as a lane-major 1024-bit mask. See [`BitPackingCompare::unpack_cmp`] for the layout. +/// +/// `packed` must hold exactly `128 * bit_width / size_of::

()` elements. +pub type UnpackCmpFn = fn(packed: &[P], output: &mut [u64; 16], cmp: F, rhs: V); + +/// FastLanes kernels resolved for one bit width of physical type `P`. +/// +/// Obtained from [`BitPackedData::kernels`](crate::BitPackedData::kernels). Each kernel is the +/// const-width instantiation for the array's bit width, so calling it does not dispatch on the +/// width. The kernels check the block lengths they are handed and panic on a mismatch. +#[derive(Clone, Copy, Debug)] +pub struct BitPackedKernels

{ + bit_width: u8, + /// Unpacks one packed block into 1024 values. + pub unpack: UnpackFn

, + /// Unpacks a single value of one packed block. + pub unpack_single: UnpackSingleFn

, + /// Unpacks one packed block, adding a frame-of-reference value. + pub unfor_pack: UnforPackFn

, +} + +impl BitPackedKernels

{ + /// The bit width the kernels were resolved for. + #[inline] + pub fn bit_width(&self) -> u8 { + self.bit_width + } + + /// The number of `P` elements holding one packed block of 1024 values. + #[inline] + pub fn packed_block_len(&self) -> usize { + 128 * self.bit_width as usize / size_of::

() + } +} + +/// [`BitPackedKernels`] resolved for one of the physical types, stored type erased by +/// [`BitPackedData`](crate::BitPackedData). +#[derive(Clone, Copy, Debug)] +pub enum ResolvedKernels { + U8(BitPackedKernels), + U16(BitPackedKernels), + U32(BitPackedKernels), + U64(BitPackedKernels), +} + +/// The physical storage types of a bit-packed array, i.e. the unsigned integers the FastLanes +/// kernels are implemented for. Signed arrays are packed as their unsigned counterpart. +pub trait BitPackedPhysical: NativePType + BitPacking + BitPackingCompare + FoR { + /// Resolves the kernels for `bit_width`, which must not exceed the width of `Self`. + fn resolve_kernels(bit_width: u8) -> ResolvedKernels; + + /// Returns the kernels if `resolved` holds kernels for `Self`. + fn kernels_from(resolved: &ResolvedKernels) -> Option>; + + /// Resolves the fused unpack-and-compare kernel for `bit_width`, which must not exceed the + /// width of `Self`. + fn resolve_unpack_cmp(bit_width: u8) -> UnpackCmpFn + where + V: FastLanesComparable, + F: Fn(V, V) -> bool; +} + +fn unpack(packed: &[P], output: &mut [P]) { + P::unpack::(as_block(packed), as_block_mut(output)); +} + +fn unpack_single(packed: &[P], index: usize) -> P { + P::unpack_single::(as_block(packed), index) +} + +fn unfor_pack( + packed: &[P], + reference: P, + output: &mut [P], +) { + P::unfor_pack::(as_block(packed), reference, as_block_mut(output)); +} + +fn unpack_cmp( + packed: &[P], + output: &mut [u64; 16], + cmp: F, + rhs: V, +) where + V: FastLanesComparable, + F: Fn(V, V) -> bool, +{ + P::unpack_cmp::(as_block(packed), output, cmp, rhs); +} + +#[inline(always)] +fn as_block(slice: &[P]) -> &[P; N] { + match slice.try_into() { + Ok(block) => block, + Err(_) => vortex_panic!( + "Expected a FastLanes block of {N} elements, got {}", + slice.len() + ), + } +} + +#[inline(always)] +fn as_block_mut(slice: &mut [P]) -> &mut [P; N] { + let len = slice.len(); + match slice.try_into() { + Ok(block) => block, + Err(_) => vortex_panic!("Expected a FastLanes block of {N} elements, got {len}"), + } +} + +macro_rules! impl_bitpacked_physical { + ($P:ty, $variant:ident, $bits:literal) => { + impl BitPackedPhysical for $P { + fn resolve_kernels(bit_width: u8) -> ResolvedKernels { + seq_macro::seq!(W in 0..=$bits { + match bit_width { + #(W => ResolvedKernels::$variant(BitPackedKernels { + bit_width, + unpack: unpack::<$P, W, { 1024 * W / $bits }>, + unpack_single: unpack_single::<$P, W, { 1024 * W / $bits }>, + unfor_pack: unfor_pack::<$P, W, { 1024 * W / $bits }>, + }),)* + _ => vortex_panic!( + "Unsupported bit width {bit_width} for {}", + <$P as NativePType>::PTYPE + ), + } + }) + } + + fn kernels_from(resolved: &ResolvedKernels) -> Option> { + match resolved { + ResolvedKernels::$variant(kernels) => Some(*kernels), + _ => None, + } + } + + fn resolve_unpack_cmp(bit_width: u8) -> UnpackCmpFn + where + V: FastLanesComparable, + F: Fn(V, V) -> bool, + { + seq_macro::seq!(W in 0..=$bits { + match bit_width { + #(W => unpack_cmp::<$P, W, { 1024 * W / $bits }, V, F>,)* + _ => vortex_panic!( + "Unsupported bit width {bit_width} for {}", + <$P as NativePType>::PTYPE + ), + } + }) + } + } + }; +} + +impl_bitpacked_physical!(u8, U8, 8); +impl_bitpacked_physical!(u16, U16, 16); +impl_bitpacked_physical!(u32, U32, 32); +impl_bitpacked_physical!(u64, U64, 64); + +#[cfg(test)] +mod tests { + use num_traits::WrappingAdd; + use rstest::rstest; + + use super::*; + + /// Every width of every physical type resolves to kernels that agree with the runtime-width + /// FastLanes entry points. + fn assert_kernels_match_fastlanes

() + where + P: BitPackedPhysical + WrappingAdd + FastLanesComparable, + { + let values: [P; 1024] = std::array::from_fn(|i| P::from(i % 251).unwrap()); + for bit_width in 0..=(8 * size_of::

() as u8) { + let kernels = P::kernels_from(&P::resolve_kernels(bit_width)).unwrap(); + assert_eq!(kernels.bit_width(), bit_width); + + let block_len = kernels.packed_block_len(); + let mut packed = vec![P::zero(); block_len]; + // SAFETY: `packed` holds exactly one block at `bit_width` and `values` 1024 values. + unsafe { P::unchecked_pack(bit_width as usize, &values, &mut packed) }; + + let mut expected = [P::zero(); 1024]; + // SAFETY: `packed` holds exactly one block at `bit_width` and `expected` 1024 values. + unsafe { P::unchecked_unpack(bit_width as usize, &packed, &mut expected) }; + + let mut unpacked = [P::zero(); 1024]; + (kernels.unpack)(&packed, &mut unpacked); + assert_eq!(unpacked, expected, "unpack at width {bit_width}"); + + for index in [0, 1, 511, 1023] { + assert_eq!( + (kernels.unpack_single)(&packed, index), + expected[index], + "unpack_single at width {bit_width} index {index}" + ); + } + + let reference = P::from(7).unwrap(); + let mut unfor = [P::zero(); 1024]; + (kernels.unfor_pack)(&packed, reference, &mut unfor); + for (got, want) in unfor.iter().zip(expected) { + assert_eq!( + *got, + want.wrapping_add(&reference), + "unfor_pack at {bit_width}" + ); + } + + let rhs = P::from(100).unwrap(); + let mut mask = [0u64; 16]; + P::resolve_unpack_cmp::(bit_width)(&packed, &mut mask, |a, b| a < b, rhs); + let mut expected_mask = [0u64; 16]; + // SAFETY: `packed` holds exactly one block at `bit_width`. + unsafe { + P::unchecked_unpack_cmp::( + bit_width as usize, + &packed, + &mut expected_mask, + |a, b| a < b, + rhs, + ); + } + assert_eq!(mask, expected_mask, "unpack_cmp at width {bit_width}"); + } + } + + #[rstest] + #[case::u8(assert_kernels_match_fastlanes::)] + #[case::u16(assert_kernels_match_fastlanes::)] + #[case::u32(assert_kernels_match_fastlanes::)] + #[case::u64(assert_kernels_match_fastlanes::)] + fn kernels_match_fastlanes(#[case] check: fn()) { + check(); + } + + #[test] + fn kernels_from_rejects_other_types() { + let resolved = u16::resolve_kernels(3); + assert!(u16::kernels_from(&resolved).is_some()); + assert!(u8::kernels_from(&resolved).is_none()); + assert!(u32::kernels_from(&resolved).is_none()); + } + + #[test] + #[should_panic(expected = "Expected a FastLanes block of 1024 elements")] + fn unpack_rejects_short_output() { + let kernels = u8::kernels_from(&u8::resolve_kernels(1)).unwrap(); + let packed = [0u8; 128]; + let mut output = [0u8; 512]; + (kernels.unpack)(&packed, &mut output); + } +} diff --git a/encodings/fastlanes/src/bitpacking/array/mod.rs b/encodings/fastlanes/src/bitpacking/array/mod.rs index 05cbee8b3ef..f99ab08f7b1 100644 --- a/encodings/fastlanes/src/bitpacking/array/mod.rs +++ b/encodings/fastlanes/src/bitpacking/array/mod.rs @@ -4,6 +4,7 @@ use std::fmt::Display; use std::fmt::Formatter; use std::mem::MaybeUninit; +use std::sync::OnceLock; use fastlanes::BitPacking; use vortex_array::ArrayRef; @@ -21,17 +22,22 @@ use vortex_array::patches::Patches; use vortex_array::patches::PatchesData; use vortex_array::validity::Validity; use vortex_array::vtable::child_to_validity; +use vortex_error::VortexExpect; use vortex_error::VortexResult; use vortex_error::vortex_ensure; use vortex_error::vortex_err; pub mod bitpack_compress; pub mod bitpack_decompress; +pub mod kernels; pub mod unpack_iter; use crate::BitPackedArray; use crate::FL_CHUNK_SIZE; use crate::bitpack_compress::bitpack_encode; +use crate::bitpacking::array::kernels::BitPackedKernels; +use crate::bitpacking::array::kernels::BitPackedPhysical; +use crate::bitpacking::array::kernels::ResolvedKernels; use crate::unpack_iter::BitPacked as BitPackedIter; use crate::unpack_iter::BitUnpackedChunks; @@ -75,6 +81,9 @@ pub struct BitPackedData { pub(super) packed: BufferHandle, /// Patch metadata for reconstructing Patches from slots. pub(super) patches_data: Option, + /// FastLanes kernels for the physical type and bit width of this array, resolved on first use + /// so that decoding never dispatches on the runtime bit width. + kernels: OnceLock, } impl Display for BitPackedData { @@ -142,6 +151,7 @@ impl BitPackedData { bit_width, packed, patches_data: patches.as_ref().map(PatchesData::from_patches), + kernels: OnceLock::new(), }) } @@ -245,6 +255,22 @@ impl BitPackedData { self.bit_width } + /// The FastLanes kernels for this array's bit width, resolved on first use. + /// + /// `P` must be the physical type of the array, i.e. the unsigned counterpart of its + /// [`PType`]. The resolved kernels are cached, so later calls only copy out the pointers. + /// + /// # Panics + /// + /// If the kernels were already resolved for a different physical type. + pub fn kernels(&self) -> BitPackedKernels

{ + let resolved = self + .kernels + .get_or_init(|| P::resolve_kernels(self.bit_width)); + P::kernels_from(resolved) + .vortex_expect("BitPacked kernels were resolved for a different physical type") + } + #[inline] pub fn offset(&self) -> u16 { self.offset @@ -330,6 +356,19 @@ pub trait BitPackedArrayExt: BitPackedArraySlotsExt { scratch, ) } + + /// The FastLanes kernels for this array's bit width, see [`BitPackedData::kernels`]. + /// + /// `P` must be the unsigned counterpart of the array's [`PType`]. + #[inline] + fn kernels(&self) -> BitPackedKernels

{ + assert_eq!( + P::PTYPE, + self.as_ref().dtype().as_ptype().to_unsigned(), + "Requested physical type doesn't match the array ptype" + ); + BitPackedData::kernels::

(self) + } } impl> BitPackedArrayExt for T {} diff --git a/encodings/fastlanes/src/bitpacking/array/unpack_iter.rs b/encodings/fastlanes/src/bitpacking/array/unpack_iter.rs index 518495896e2..f57a33625aa 100644 --- a/encodings/fastlanes/src/bitpacking/array/unpack_iter.rs +++ b/encodings/fastlanes/src/bitpacking/array/unpack_iter.rs @@ -5,7 +5,6 @@ use std::mem; use std::mem::MaybeUninit; use std::ops::Range; -use fastlanes::BitPacking; use lending_iterator::gat; use lending_iterator::prelude::Item; #[gat(Item)] @@ -16,35 +15,31 @@ use vortex_error::vortex_ensure; use crate::BitPackedData; use crate::FL_CHUNK_SIZE; +use crate::bitpacking::array::kernels::BitPackedPhysical; +use crate::bitpacking::array::kernels::UnpackFn; const CHUNK_SIZE: usize = FL_CHUNK_SIZE; /// Strategy trait for fastlanes unpacking operations pub trait UnpackStrategy { - /// Unpack a chunk of packed data into the destination buffer + /// Unpack a chunk of packed data into the destination buffer. /// - /// # Safety - /// - `chunk` must contain exactly `elems_per_chunk` elements - /// - `dst` must have exactly CHUNK_SIZE capacity - unsafe fn unpack_chunk(&self, bit_width: usize, chunk: &[T::Physical], dst: &mut [T::Physical]); + /// `chunk` must contain exactly one packed block (`128 * bit_width / size_of::()` + /// elements) and `dst` exactly `CHUNK_SIZE` elements. + fn unpack_chunk(&self, chunk: &[T::Physical], dst: &mut [T::Physical]); } -/// BitPacking strategy - uses plain bitpacking without reference value -pub struct BitPackingStrategy; +/// BitPacking strategy - plain bitpacking without reference value, using the unpack kernel +/// resolved for the array's bit width. +pub struct BitPackingStrategy

{ + unpack: UnpackFn

, +} -impl> UnpackStrategy for BitPackingStrategy { +impl UnpackStrategy for BitPackingStrategy { #[allow(clippy::inline_always)] #[inline(always)] - unsafe fn unpack_chunk( - &self, - bit_width: usize, - chunk: &[T::Physical], - dst: &mut [T::Physical], - ) { - // SAFETY: Caller must ensure [`BitPacking::unchecked_unpack`] safety requirements hold. - unsafe { - BitPacking::unchecked_unpack(bit_width, chunk, dst); - } + fn unpack_chunk(&self, chunk: &[T::Physical], dst: &mut [T::Physical]) { + (self.unpack)(chunk, dst); } } @@ -98,7 +93,8 @@ pub struct UnpackedChunks<'a, T: PhysicalPType, S: UnpackStrategy> { scratch: &'a mut [MaybeUninit; CHUNK_SIZE], } -pub type BitUnpackedChunks<'a, T> = UnpackedChunks<'a, T, BitPackingStrategy>; +pub type BitUnpackedChunks<'a, T> = + UnpackedChunks<'a, T, BitPackingStrategy<::Physical>>; impl<'a, T: BitPacked> BitUnpackedChunks<'a, T> { pub fn try_new( @@ -107,7 +103,9 @@ impl<'a, T: BitPacked> BitUnpackedChunks<'a, T> { scratch: &'a mut [MaybeUninit; CHUNK_SIZE], ) -> VortexResult { Self::try_new_with_strategy( - BitPackingStrategy, + BitPackingStrategy { + unpack: array.kernels::().unpack, + }, array.packed_slice::(), array.bit_width() as usize, array.offset() as usize, @@ -123,7 +121,7 @@ impl<'a, T: BitPacked> BitUnpackedChunks<'a, T> { BitUnpackIterator::new( self.packed, self.scratch, - self.bit_width, + self.strategy.unpack, elems_per_chunk, self.num_chunks - last_chunk_is_sliced, first_chunk_is_sliced, @@ -172,13 +170,9 @@ impl<'a, T: PhysicalPType, S: UnpackStrategy> UnpackedChunks<'a, T, S> { } else { CHUNK_SIZE - self.offset }; - // SAFETY: - // 1. chunk is elems_per_chunk. - // 2. buffer is exactly CHUNK_SIZE. - unsafe { - self.strategy.unpack_chunk(self.bit_width, chunk, dst); - mem::transmute(&mut self.scratch[self.offset..][..header_end_slice]) - } + self.strategy.unpack_chunk(chunk, dst); + // SAFETY: `unpack_chunk` initialized every element of the buffer. + unsafe { mem::transmute(&mut self.scratch[self.offset..][..header_end_slice]) } }) } @@ -242,7 +236,7 @@ impl<'a, T: PhysicalPType, S: UnpackStrategy> UnpackedChunks<'a, T, S> { let chunk = &packed_slice[i * elems_per_chunk..][..elems_per_chunk]; unsafe { let dst: &mut [T::Physical] = mem::transmute(&mut self.scratch[..]); - self.strategy.unpack_chunk(self.bit_width, chunk, dst); + self.strategy.unpack_chunk(chunk, dst); let unpacked: &mut [T] = mem::transmute(&mut self.scratch[..]); f(unpacked, local_idx..local_idx + CHUNK_SIZE); } @@ -280,7 +274,7 @@ impl<'a, T: PhysicalPType, S: UnpackStrategy> UnpackedChunks<'a, T, S> { let uninit_dst = &mut output[local_idx..local_idx + CHUNK_SIZE]; // SAFETY: &[T] and &[MaybeUninit] have the same layout. let dst: &mut [T::Physical] = mem::transmute(uninit_dst); - self.strategy.unpack_chunk(self.bit_width, chunk, dst); + self.strategy.unpack_chunk(chunk, dst); } local_idx += CHUNK_SIZE; } @@ -299,13 +293,9 @@ impl<'a, T: PhysicalPType, S: UnpackStrategy> UnpackedChunks<'a, T, S> { [(self.num_chunks - 1) * self.elems_per_chunk()..][..self.elems_per_chunk()]; let dst: &mut [MaybeUninit] = self.scratch; let dst: &mut [T::Physical] = unsafe { mem::transmute(dst) }; - // SAFETY: - // 1. chunk is elems_per_chunk. - // 2. buffer is exactly CHUNK_SIZE. - unsafe { - self.strategy.unpack_chunk(self.bit_width, chunk, dst); - mem::transmute(&mut self.scratch[..self.last_chunk_length]) - } + self.strategy.unpack_chunk(chunk, dst); + // SAFETY: `unpack_chunk` initialized every element of the buffer. + unsafe { mem::transmute(&mut self.scratch[..self.last_chunk_length]) } }) } @@ -366,7 +356,7 @@ fn validate_packed( pub struct BitUnpackIterator<'a, T: BitPacked + 'a> { packed: &'a [T::Physical], buffer: &'a mut [MaybeUninit; CHUNK_SIZE], - bit_width: usize, + unpack: UnpackFn, elems_per_chunk: usize, num_chunks: usize, idx: usize, @@ -376,7 +366,7 @@ impl<'a, T: BitPacked> BitUnpackIterator<'a, T> { pub fn new( packed: &'a [T::Physical], buffer: &'a mut [MaybeUninit; CHUNK_SIZE], - bit_width: usize, + unpack: UnpackFn, elems_per_chunk: usize, num_chunks: usize, first_chunk_is_sliced: bool, @@ -384,7 +374,7 @@ impl<'a, T: BitPacked> BitUnpackIterator<'a, T> { Self { packed, buffer, - bit_width, + unpack, elems_per_chunk, num_chunks, idx: if first_chunk_is_sliced { 1 } else { 0 }, @@ -407,11 +397,9 @@ impl<'a, T: BitPacked + 'a> LendingIterator for BitUnpackIterator<'a, T> { let chunk = &self.packed[self.idx * self.elems_per_chunk..][..self.elems_per_chunk]; let dst: &mut [MaybeUninit] = self.buffer; - unsafe { - let dst: &mut [T::Physical] = mem::transmute(dst); - - BitPacking::unchecked_unpack(self.bit_width, chunk, dst); - } + // SAFETY: &[MaybeUninit] and &[T::Physical] have the same layout. + let dst: &mut [T::Physical] = unsafe { mem::transmute(dst) }; + (self.unpack)(chunk, dst); self.idx += 1; // SAFETY: The buffer has the appropriate lifetime, the iterator signature doesn't account for it Some(unsafe { mem::transmute::<&mut [MaybeUninit; 1024], &mut [T; 1024]>(self.buffer) }) @@ -424,7 +412,7 @@ fn write_map(src: &[T], dst: &mut [MaybeUninit], f: &mut impl FnM } } -pub trait BitPacked: PhysicalPType {} +pub trait BitPacked: PhysicalPType {} impl BitPacked for i8 {} impl BitPacked for i16 {} diff --git a/encodings/fastlanes/src/bitpacking/compute/compare.rs b/encodings/fastlanes/src/bitpacking/compute/compare.rs index c9d6b815b0d..95cd018b180 100644 --- a/encodings/fastlanes/src/bitpacking/compute/compare.rs +++ b/encodings/fastlanes/src/bitpacking/compute/compare.rs @@ -11,8 +11,6 @@ //! [`BitPackedArray`]: crate::BitPackedArray //! [`BitBuffer`]: vortex_buffer::BitBuffer -use fastlanes::BitPacking; -use fastlanes::BitPackingCompare; use fastlanes::FastLanesComparable; use vortex_array::ArrayRef; use vortex_array::ArrayView; @@ -78,7 +76,6 @@ where T: NativePType + BitPackedIter + FastLanesComparable::Physical>, - ::Physical: BitPacking + NativePType + BitPackingCompare, { match operator { CompareOperator::Eq => { diff --git a/encodings/fastlanes/src/bitpacking/compute/compare_fused.rs b/encodings/fastlanes/src/bitpacking/compute/compare_fused.rs index 1259ed815fe..6a6afc2f007 100644 --- a/encodings/fastlanes/src/bitpacking/compute/compare_fused.rs +++ b/encodings/fastlanes/src/bitpacking/compute/compare_fused.rs @@ -5,14 +5,16 @@ //! //! Where [`super::stream_predicate`] unpacks a full 1024-element FastLanes block into a scratch //! buffer and *then* folds a predicate over it, this path hands the comparison down into the -//! FastLanes [`BitPackingCompare::unchecked_unpack_cmp`] kernel, which compares each value against -//! the constant *as it is unpacked*, accumulating the boolean results straight into a 1024-bit -//! mask (`[u64; 16]`) in transposed FastLanes lane order - one register-resident word per lane, no -//! `[bool; 1024]` or `[T; 1024]` scratch. A single SIMD [`transpose_bits`] per block then rotates -//! that mask into logical row order. +//! FastLanes [`fastlanes::BitPackingCompare::unpack_cmp`] kernel, which compares each value +//! against the constant *as it is unpacked*, accumulating the boolean results straight into a +//! 1024-bit mask (`[u64; 16]`) in transposed FastLanes lane order - one register-resident word per +//! lane, no `[bool; 1024]` or `[T; 1024]` scratch. A single SIMD [`transpose_bits`] per block then +//! rotates that mask into logical row order. //! //! The packed blocks are walked through [`crate::unpack_iter::for_each_packed_chunk`], so chunk -//! sizing and bounds live in one place without allocating an unpack scratch buffer. +//! sizing and bounds live in one place without allocating an unpack scratch buffer. The kernel +//! instantiation for the array's bit width is resolved once up front, so no block re-dispatches +//! on the width. //! //! Slicing is handled by working in *padded* coordinates: bit `offset + i` holds element `i`. The //! output buffer is over-allocated to whole 1024-bit blocks, so every block - the sliced first @@ -25,8 +27,6 @@ //! [`BitPackedArray`]: crate::BitPackedArray //! [`BitBuffer`]: vortex_buffer::BitBuffer -use fastlanes::BitPacking; -use fastlanes::BitPackingCompare; use fastlanes::FastLanesComparable; use fastlanes::transpose_bits; use num_traits::AsPrimitive; @@ -48,6 +48,7 @@ use vortex_error::VortexResult; use super::stream_predicate::stream_predicate; use crate::BitPacked; use crate::BitPackedArrayExt; +use crate::bitpacking::array::kernels::BitPackedPhysical; use crate::unpack_iter::BitPacked as BitPackedIter; use crate::unpack_iter::for_each_packed_chunk; @@ -74,11 +75,10 @@ where T: NativePType + BitPackedIter + FastLanesComparable::Physical>, - ::Physical: BitPacking + NativePType + BitPackingCompare, F: Fn(T, T) -> bool + Copy, { let len = array.len(); - let bit_width = array.bit_width() as usize; + let bit_width = array.bit_width(); let offset = array.offset() as usize; // A degenerate width has no packed payload for the fused kernel to consume; defer to the scalar @@ -92,12 +92,16 @@ where let num_chunks = (offset + len).div_ceil(CHUNK_SIZE); let mut words: BufferMut = BufferMut::zeroed(num_chunks * WORDS_PER_CHUNK); + let unpack_cmp = <::Physical as BitPackedPhysical>::resolve_unpack_cmp::< + T, + F, + >(bit_width); { let words = words.as_mut_slice(); let mut lane_major = [0u64; WORDS_PER_CHUNK]; for_each_packed_chunk::( array.packed_slice::<::Physical>(), - bit_width, + bit_width as usize, offset, len, |packed_chunk, range| { @@ -105,15 +109,9 @@ where let out = words[range.start / U64_BITS..] .first_chunk_mut::() .vortex_expect("over-allocated buffer holds a full block per chunk"); - // SAFETY: `packed_chunk` holds exactly `128 * bit_width / size_of::()` packed - // elements and `bit_width <= U::T`, satisfying `unchecked_unpack_cmp`'s contract. The - // kernel assigns every word in `transposed`, so its previous contents are irrelevant. - unsafe { - <::Physical as BitPackingCompare>::unchecked_unpack_cmp::< - T, - _, - >(bit_width, packed_chunk, &mut lane_major, cmp, rhs); - } + // The kernel assigns every word in `lane_major`, so its previous contents are + // irrelevant. + unpack_cmp(packed_chunk, &mut lane_major, cmp, rhs); transpose_bits::<::Physical>(&lane_major, out); }, )?; diff --git a/encodings/fastlanes/src/bitpacking/compute/filter.rs b/encodings/fastlanes/src/bitpacking/compute/filter.rs index 0b1b9422f86..b4bc2b33f14 100644 --- a/encodings/fastlanes/src/bitpacking/compute/filter.rs +++ b/encodings/fastlanes/src/bitpacking/compute/filter.rs @@ -3,14 +3,12 @@ use std::mem::MaybeUninit; -use fastlanes::BitPacking; use vortex_array::ArrayRef; use vortex_array::ArrayView; use vortex_array::ExecutionCtx; use vortex_array::IntoArray; use vortex_array::arrays::PrimitiveArray; use vortex_array::arrays::filter::FilterKernel; -use vortex_array::dtype::NativePType; use vortex_array::dtype::PType; use vortex_array::dtype::UnsignedPType; use vortex_array::match_each_unsigned_integer_ptype; @@ -26,6 +24,7 @@ use super::take::UNPACK_CHUNK_THRESHOLD; use crate::BitPacked; use crate::BitPackedArrayExt; use crate::BitPackedData; +use crate::bitpacking::array::kernels::BitPackedPhysical; /// The threshold over which it is faster to fully unpack the entire [`BitPackedArray`](crate::BitPackedArray) and then /// filter the result than to unpack only specific bitpacked values into the output buffer. @@ -106,7 +105,7 @@ impl FilterKernel for BitPacked { /// elements is relatively slow. /// /// Returns a tuple of (values buffer, validity mask). -fn filter_primitive_without_patches( +fn filter_primitive_without_patches( array: ArrayView<'_, BitPacked>, selection: &MaskValuesRef, ) -> VortexResult<(Buffer, Validity)> { @@ -118,12 +117,12 @@ fn filter_primitive_without_patches( Ok((values.freeze(), validity)) } -fn filter_with_indices( +fn filter_with_indices( array: &BitPackedData, indices: &[usize], ) -> BufferMut { let offset = array.offset() as usize; - let bit_width = array.bit_width() as usize; + let kernels = array.kernels::(); let mut values = BufferMut::with_capacity(indices.len()); // Some re-usable memory to store per-chunk indices. @@ -131,7 +130,7 @@ fn filter_with_indices( let packed_bytes = array.packed_slice::(); // Group the indices by the FastLanes chunk they belong to. - let chunk_size = 128 * bit_width / size_of::(); + let chunk_size = kernels.packed_block_len(); chunked_indices( indices.iter().copied(), @@ -141,22 +140,17 @@ fn filter_with_indices( if indices_within_chunk.len() == 1024 { // Unpack the entire chunk. - unsafe { - let values_len = values.len(); - values.set_len(values_len + 1024); - BitPacking::unchecked_unpack( - bit_width, - packed, - &mut values.as_mut_slice()[values_len..], - ); - } + let values_len = values.len(); + // SAFETY: The capacity holds every index, and `unpack` initializes all 1024 + // values before they are read. + unsafe { values.set_len(values_len + 1024) }; + (kernels.unpack)(packed, &mut values.as_mut_slice()[values_len..]); } else if indices_within_chunk.len() > UNPACK_CHUNK_THRESHOLD { // Unpack into a temporary chunk and then copy the values. - unsafe { - let dst: &mut [MaybeUninit] = &mut unpacked; - let dst: &mut [T] = std::mem::transmute(dst); - BitPacking::unchecked_unpack(bit_width, packed, dst); - } + let dst: &mut [MaybeUninit] = &mut unpacked; + // SAFETY: &[MaybeUninit] and &[T] have the same layout. + let dst: &mut [T] = unsafe { std::mem::transmute(dst) }; + (kernels.unpack)(packed, dst); values.extend_trusted( indices_within_chunk .iter() @@ -164,9 +158,11 @@ fn filter_with_indices( ); } else { // Otherwise, unpack each element individually. - values.extend_trusted(indices_within_chunk.iter().map(|&idx| unsafe { - BitPacking::unchecked_unpack_single(bit_width, packed, idx) - })); + values.extend_trusted( + indices_within_chunk + .iter() + .map(|&idx| (kernels.unpack_single)(packed, idx)), + ); } }, ); diff --git a/encodings/fastlanes/src/bitpacking/compute/take.rs b/encodings/fastlanes/src/bitpacking/compute/take.rs index 86e97623cf6..58dd2c5edaf 100644 --- a/encodings/fastlanes/src/bitpacking/compute/take.rs +++ b/encodings/fastlanes/src/bitpacking/compute/take.rs @@ -4,7 +4,6 @@ use std::mem; use std::mem::MaybeUninit; -use fastlanes::BitPacking; use vortex_array::ArrayRef; use vortex_array::ArrayView; use vortex_array::ExecutionCtx; @@ -12,7 +11,6 @@ use vortex_array::IntoArray; use vortex_array::arrays::PrimitiveArray; use vortex_array::arrays::dict::TakeExecute; use vortex_array::dtype::IntegerPType; -use vortex_array::dtype::NativePType; use vortex_array::dtype::PType; use vortex_array::match_each_integer_ptype; use vortex_array::match_each_unsigned_integer_ptype; @@ -25,7 +23,7 @@ use vortex_error::VortexResult; use super::chunked_indices; use crate::BitPacked; use crate::BitPackedArrayExt; -use crate::bitpack_decompress; +use crate::bitpacking::array::kernels::BitPackedPhysical; // TODO(connor): This is duplicated in `encodings/fastlanes/src/bitpacking/kernels/mod.rs`. /// assuming the buffer is already allocated (which will happen at most once) then unpacking @@ -70,7 +68,7 @@ impl TakeExecute for BitPacked { } } -fn take_primitive( +fn take_primitive( array: ArrayView<'_, BitPacked>, indices: &PrimitiveArray, taken_validity: Validity, @@ -81,7 +79,7 @@ fn take_primitive( } let offset = array.offset() as usize; - let bit_width = array.bit_width() as usize; + let kernels = array.kernels::(); let packed = array.packed_slice::(); @@ -93,7 +91,7 @@ fn take_primitive( let mut output = BufferMut::::with_capacity(indices.len()); let mut unpacked = [const { MaybeUninit::uninit() }; 1024]; - let chunk_len = 128 * bit_width / size_of::(); + let chunk_len = kernels.packed_block_len(); chunked_indices(indices_iter, offset, |chunk_idx, indices_within_chunk| { let packed = &packed[chunk_idx * chunk_len..][..chunk_len]; @@ -104,11 +102,10 @@ fn take_primitive( // this loop only runs if we have at least UNPACK_CHUNK_THRESHOLD offsets for offset_chunk in offset_chunks { if !have_unpacked { - unsafe { - let dst: &mut [MaybeUninit] = &mut unpacked; - let dst: &mut [T] = mem::transmute(dst); - BitPacking::unchecked_unpack(bit_width, packed, dst); - } + let dst: &mut [MaybeUninit] = &mut unpacked; + // SAFETY: &[MaybeUninit] and &[T] have the same layout. + let dst: &mut [T] = unsafe { mem::transmute(dst) }; + (kernels.unpack)(packed, dst); have_unpacked = true; } @@ -128,9 +125,7 @@ fn take_primitive( // we had fewer than UNPACK_CHUNK_THRESHOLD offsets in the first place, // so we need to unpack each one individually for &index in remainder { - output.push(unsafe { - bitpack_decompress::unpack_single_primitive::(packed, bit_width, index) - }); + output.push((kernels.unpack_single)(packed, index)); } } } diff --git a/encodings/fastlanes/src/bitpacking/mod.rs b/encodings/fastlanes/src/bitpacking/mod.rs index efa0677a91e..1dedcc106ef 100644 --- a/encodings/fastlanes/src/bitpacking/mod.rs +++ b/encodings/fastlanes/src/bitpacking/mod.rs @@ -9,6 +9,7 @@ pub use array::BitPackedDataParts; pub use array::BitPackedSlots; pub use array::bitpack_compress; pub use array::bitpack_decompress; +pub use array::kernels; pub use array::unpack_iter; pub(crate) mod compute; diff --git a/encodings/fastlanes/src/for/array/for_decompress.rs b/encodings/fastlanes/src/for/array/for_decompress.rs index 6e429abc419..93684fb0ccd 100644 --- a/encodings/fastlanes/src/for/array/for_decompress.rs +++ b/encodings/fastlanes/src/for/array/for_decompress.rs @@ -3,7 +3,6 @@ use std::mem::MaybeUninit; -use fastlanes::FoR; use num_traits::PrimInt; use num_traits::WrappingAdd; use vortex_array::ArrayView; @@ -24,29 +23,25 @@ use crate::BitPackedArrayExt; use crate::FL_CHUNK_SIZE; use crate::FoRArray; use crate::bitpack_decompress; +use crate::bitpacking::kernels::BitPackedPhysical; +use crate::bitpacking::kernels::UnforPackFn; use crate::r#for::array::FoRArrayExt; use crate::r#for::array::FoRArraySlotsExt; use crate::unpack_iter::UnpackStrategy; use crate::unpack_iter::UnpackedChunks; -/// FoR unpacking strategy that applies a reference value during unpacking. +/// FoR unpacking strategy that applies a reference value during unpacking, using the fused +/// kernel resolved for the bit-packed child's width. struct FoRStrategy { reference: T, + unfor_pack: UnforPackFn, } -impl + FoR> UnpackStrategy for FoRStrategy { +impl + BitPackedPhysical> UnpackStrategy for FoRStrategy { #[allow(clippy::inline_always)] #[inline(always)] - unsafe fn unpack_chunk( - &self, - bit_width: usize, - chunk: &[T::Physical], - dst: &mut [T::Physical], - ) { - // SAFETY: Caller ensures chunk and dst have correct sizes. - unsafe { - FoR::unchecked_unfor_pack(bit_width, chunk, self.reference, dst); - } + fn unpack_chunk(&self, chunk: &[T::Physical], dst: &mut [T::Physical]) { + (self.unfor_pack)(chunk, self.reference, dst); } } @@ -84,7 +79,7 @@ pub fn decompress(array: &FoRArray, ctx: &mut ExecutionCtx) -> VortexResult + UnsignedPType + FoR + WrappingAdd, + T: PhysicalPType + UnsignedPType + BitPackedPhysical + WrappingAdd, >( for_: &FoRArray, bp: ArrayView<'_, BitPacked>, @@ -96,7 +91,10 @@ pub(crate) fn fused_decompress< .as_::() .vortex_expect("cannot be null"); - let strategy = FoRStrategy { reference: ref_ }; + let strategy = FoRStrategy { + reference: ref_, + unfor_pack: bp.kernels::().unfor_pack, + }; let mut scratch = [const { MaybeUninit::::uninit() }; FL_CHUNK_SIZE]; // Create [`UnpackedChunks`] with FoR strategy. From d6d7220ed7636f33c0d99cd9576606a8cc8fac19 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 19:10:53 +0000 Subject: [PATCH 2/6] feat(fastlanes): resolve the pack kernel once per buffer `bitpack_primitive` still dispatched on the runtime bit width through `unchecked_pack` for every 1024-value block. Resolve the const-width pack kernel once per call via `BitPackedPhysical::resolve_pack` instead, so no path outside kernel resolution matches on the width. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01R9mScTPzB4Vw4PCdSyc862 Signed-off-by: Claude --- .../src/bitpacking/array/bitpack_compress.rs | 32 ++++++++--------- .../fastlanes/src/bitpacking/array/kernels.rs | 36 +++++++++++++++++-- 2 files changed, 48 insertions(+), 20 deletions(-) diff --git a/encodings/fastlanes/src/bitpacking/array/bitpack_compress.rs b/encodings/fastlanes/src/bitpacking/array/bitpack_compress.rs index a393db6ecc8..3e1ab9ebc86 100644 --- a/encodings/fastlanes/src/bitpacking/array/bitpack_compress.rs +++ b/encodings/fastlanes/src/bitpacking/array/bitpack_compress.rs @@ -1,7 +1,6 @@ // SPDX-License-Identifier: Apache-2.0 // SPDX-FileCopyrightText: Copyright the Vortex contributors -use fastlanes::BitPacking; use itertools::Itertools; use num_traits::PrimInt; use vortex_array::ArrayView; @@ -30,6 +29,7 @@ use vortex_mask::Mask; use crate::BitPacked; use crate::BitPackedArray; use crate::bitpack_decompress; +use crate::bitpacking::array::kernels::BitPackedPhysical; pub fn bitpack_to_best_bit_width( array: &PrimitiveArray, @@ -143,10 +143,11 @@ pub unsafe fn bitpack_unchecked(parray: &PrimitiveArray, bit_width: u8) -> ByteB /// Bitpack a slice of primitives down to the given width. /// /// See `bitpack` for more caller information. -pub fn bitpack_primitive(array: &[T], bit_width: u8) -> Buffer { +pub fn bitpack_primitive(array: &[T], bit_width: u8) -> Buffer { if bit_width == 0 { return Buffer::::empty(); } + let pack = T::resolve_pack(bit_width); let bit_width = bit_width as usize; // How many fastlanes vectors we will process. @@ -164,14 +165,13 @@ pub fn bitpack_primitive(array: &[T], bit_width: u8 (0..num_full_chunks).for_each(|i| { let start_elem = i * 1024; let output_len = output.len(); - unsafe { - output.set_len(output_len + packed_len); - BitPacking::unchecked_pack( - bit_width, - &array[start_elem..][..1024], - &mut output[output_len..][..packed_len], - ); - }; + // SAFETY: The capacity holds every block, and `pack` initializes the new elements before + // they are read. + unsafe { output.set_len(output_len + packed_len) }; + pack( + &array[start_elem..][..1024], + &mut output[output_len..][..packed_len], + ); }); // Pad the last chunk with zeros to a full 1024 elements. @@ -181,14 +181,10 @@ pub fn bitpack_primitive(array: &[T], bit_width: u8 last_chunk[..last_chunk_size].copy_from_slice(&array[array.len() - last_chunk_size..]); let output_len = output.len(); - unsafe { - output.set_len(output_len + packed_len); - BitPacking::unchecked_pack( - bit_width, - &last_chunk, - &mut output[output_len..][..packed_len], - ); - }; + // SAFETY: The capacity holds every block, and `pack` initializes the new elements before + // they are read. + unsafe { output.set_len(output_len + packed_len) }; + pack(&last_chunk, &mut output[output_len..][..packed_len]); } output.freeze() diff --git a/encodings/fastlanes/src/bitpacking/array/kernels.rs b/encodings/fastlanes/src/bitpacking/array/kernels.rs index 8955196ff62..f894526c3f3 100644 --- a/encodings/fastlanes/src/bitpacking/array/kernels.rs +++ b/encodings/fastlanes/src/bitpacking/array/kernels.rs @@ -17,6 +17,11 @@ use fastlanes::FoR; use vortex_array::dtype::NativePType; use vortex_error::vortex_panic; +/// Packs one FastLanes block of 1024 values. +/// +/// `input` must hold exactly 1024 elements and `output` exactly `128 * bit_width / size_of::

()`. +pub type PackFn

= fn(input: &[P], output: &mut [P]); + /// Unpacks one FastLanes block of 1024 values. /// /// `packed` must hold exactly [`BitPackedKernels::packed_block_len`] elements and `output` @@ -89,6 +94,12 @@ pub trait BitPackedPhysical: NativePType + BitPacking + BitPackingCompare + FoR /// Returns the kernels if `resolved` holds kernels for `Self`. fn kernels_from(resolved: &ResolvedKernels) -> Option>; + /// Resolves the pack kernel for `bit_width`, which must not exceed the width of `Self`. + /// + /// Packing happens before an array exists to cache kernels on, so callers resolve this once + /// per buffer instead. + fn resolve_pack(bit_width: u8) -> PackFn; + /// Resolves the fused unpack-and-compare kernel for `bit_width`, which must not exceed the /// width of `Self`. fn resolve_unpack_cmp(bit_width: u8) -> UnpackCmpFn @@ -97,6 +108,10 @@ pub trait BitPackedPhysical: NativePType + BitPacking + BitPackingCompare + FoR F: Fn(V, V) -> bool; } +fn pack(input: &[P], output: &mut [P]) { + P::pack::(as_block(input), as_block_mut(output)); +} + fn unpack(packed: &[P], output: &mut [P]) { P::unpack::(as_block(packed), as_block_mut(output)); } @@ -172,6 +187,18 @@ macro_rules! impl_bitpacked_physical { } } + fn resolve_pack(bit_width: u8) -> PackFn { + seq_macro::seq!(W in 0..=$bits { + match bit_width { + #(W => pack::<$P, W, { 1024 * W / $bits }>,)* + _ => vortex_panic!( + "Unsupported bit width {bit_width} for {}", + <$P as NativePType>::PTYPE + ), + } + }) + } + fn resolve_unpack_cmp(bit_width: u8) -> UnpackCmpFn where V: FastLanesComparable, @@ -215,9 +242,14 @@ mod tests { assert_eq!(kernels.bit_width(), bit_width); let block_len = kernels.packed_block_len(); + let mut expected_packed = vec![P::zero(); block_len]; + // SAFETY: `expected_packed` holds exactly one block at `bit_width` and `values` 1024 + // values. + unsafe { P::unchecked_pack(bit_width as usize, &values, &mut expected_packed) }; + let mut packed = vec![P::zero(); block_len]; - // SAFETY: `packed` holds exactly one block at `bit_width` and `values` 1024 values. - unsafe { P::unchecked_pack(bit_width as usize, &values, &mut packed) }; + P::resolve_pack(bit_width)(&values, &mut packed); + assert_eq!(packed, expected_packed, "pack at width {bit_width}"); let mut expected = [P::zero(); 1024]; // SAFETY: `packed` holds exactly one block at `bit_width` and `expected` 1024 values. From c1bb388094bbdc536ebb061b594b0636ca239a72 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 22:21:05 +0000 Subject: [PATCH 3/6] perf(fastlanes): keep BitPacked kernel wrappers frameless Inspecting the release assembly showed two misses. `BitPackedData::kernels` was not inlined, so `scalar_at` paid an out-of-line call before the indirect kernel call; mark it `#[inline]`. Every kernel wrapper also carried the `vortex_panic!` formatting for a block-length mismatch inline, which reserved a stack frame on the hot path; move it into a `#[cold]` out-of-line function so the wrappers reduce to a length compare and a tail jump into the fastlanes kernel. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01R9mScTPzB4Vw4PCdSyc862 Signed-off-by: Claude --- .../fastlanes/src/bitpacking/array/kernels.rs | 15 ++++++++++----- encodings/fastlanes/src/bitpacking/array/mod.rs | 1 + 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/encodings/fastlanes/src/bitpacking/array/kernels.rs b/encodings/fastlanes/src/bitpacking/array/kernels.rs index f894526c3f3..798000edbf9 100644 --- a/encodings/fastlanes/src/bitpacking/array/kernels.rs +++ b/encodings/fastlanes/src/bitpacking/array/kernels.rs @@ -144,10 +144,7 @@ fn unpack_cmp( fn as_block(slice: &[P]) -> &[P; N] { match slice.try_into() { Ok(block) => block, - Err(_) => vortex_panic!( - "Expected a FastLanes block of {N} elements, got {}", - slice.len() - ), + Err(_) => block_len_mismatch(N, slice.len()), } } @@ -156,10 +153,18 @@ fn as_block_mut(slice: &mut [P]) -> &mut [P; N] { let len = slice.len(); match slice.try_into() { Ok(block) => block, - Err(_) => vortex_panic!("Expected a FastLanes block of {N} elements, got {len}"), + Err(_) => block_len_mismatch(N, len), } } +/// Kept out of line so the kernel wrappers stay frameless trampolines: the panic formatting +/// would otherwise reserve stack on every call. +#[cold] +#[inline(never)] +fn block_len_mismatch(expected: usize, actual: usize) -> ! { + vortex_panic!("Expected a FastLanes block of {expected} elements, got {actual}") +} + macro_rules! impl_bitpacked_physical { ($P:ty, $variant:ident, $bits:literal) => { impl BitPackedPhysical for $P { diff --git a/encodings/fastlanes/src/bitpacking/array/mod.rs b/encodings/fastlanes/src/bitpacking/array/mod.rs index f99ab08f7b1..1c7d91a68a3 100644 --- a/encodings/fastlanes/src/bitpacking/array/mod.rs +++ b/encodings/fastlanes/src/bitpacking/array/mod.rs @@ -263,6 +263,7 @@ impl BitPackedData { /// # Panics /// /// If the kernels were already resolved for a different physical type. + #[inline] pub fn kernels(&self) -> BitPackedKernels

{ let resolved = self .kernels From 624e607738d67860a9bf60bfea1f77c68fb7bbf3 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 09:23:16 +0000 Subject: [PATCH 4/6] refactor(fastlanes): resolve BitPacked kernels eagerly as erased pointers Resolve the FastLanes kernels when a `BitPackedData` is constructed rather than on first use. `BitPackedData::try_new` now takes the array's `PType`, rejects a bit width wider than the type, and stores the kernels directly, so the `OnceLock` is gone. Replace the per-type `ResolvedKernels` enum with a type-erased `BitPackedKernels<()>`: the function pointers are transmuted to a placeholder element type for storage and transmuted back by `typed::

()`, which checks the recorded physical `PType` first. This removes the enum, the `kernels_from` trait method, and the per-call variant match; the accessor is now a ptype compare and a struct copy. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01R9mScTPzB4Vw4PCdSyc862 Signed-off-by: Claude --- .../fastlanes/src/bitpacking/array/kernels.rs | 157 ++++++++++++++---- .../fastlanes/src/bitpacking/array/mod.rs | 33 ++-- .../fastlanes/src/bitpacking/vtable/mod.rs | 3 +- 3 files changed, 133 insertions(+), 60 deletions(-) diff --git a/encodings/fastlanes/src/bitpacking/array/kernels.rs b/encodings/fastlanes/src/bitpacking/array/kernels.rs index 798000edbf9..5155760cbb8 100644 --- a/encodings/fastlanes/src/bitpacking/array/kernels.rs +++ b/encodings/fastlanes/src/bitpacking/array/kernels.rs @@ -7,14 +7,21 @@ //! `unchecked_*` entry points of the `fastlanes` crate dispatch on the width with a `match` on //! every call. A [`BitPackedArray`](crate::BitPackedArray) knows its width but is type erased, so //! it cannot name the instantiation statically. Instead, the array resolves function pointers to -//! the concrete instantiations once, lazily, and hands them out as [`BitPackedKernels`]. The -//! decoding paths then call the resolved kernels block after block without re-dispatching. +//! the concrete instantiations once, when it is constructed, and hands them out as +//! [`BitPackedKernels`]. The decoding paths then call the resolved kernels block after block +//! without re-dispatching. + +use std::mem; use fastlanes::BitPacking; use fastlanes::BitPackingCompare; use fastlanes::FastLanesComparable; use fastlanes::FoR; use vortex_array::dtype::NativePType; +use vortex_array::dtype::PType; +use vortex_array::match_each_unsigned_integer_ptype; +use vortex_error::VortexResult; +use vortex_error::vortex_ensure; use vortex_error::vortex_panic; /// Packs one FastLanes block of 1024 values. @@ -52,6 +59,9 @@ pub type UnpackCmpFn = fn(packed: &[P], output: &mut [u64; 16], cmp: F, /// width. The kernels check the block lengths they are handed and panic on a mismatch. #[derive(Clone, Copy, Debug)] pub struct BitPackedKernels

{ + /// The unsigned [`PType`] of `P`, kept so the type-erased form can check it before + /// re-typing the pointers. + ptype: PType, bit_width: u8, /// Unpacks one packed block into 1024 values. pub unpack: UnpackFn

, @@ -75,24 +85,82 @@ impl BitPackedKernels

{ } } -/// [`BitPackedKernels`] resolved for one of the physical types, stored type erased by +/// [`BitPackedKernels`] with the physical type erased, as stored by /// [`BitPackedData`](crate::BitPackedData). -#[derive(Clone, Copy, Debug)] -pub enum ResolvedKernels { - U8(BitPackedKernels), - U16(BitPackedKernels), - U32(BitPackedKernels), - U64(BitPackedKernels), +/// +/// The function pointers are those of a `BitPackedKernels

` for the recorded `ptype`, cast to +/// a placeholder element type. [`Self::typed`] casts them back; they are never called in the +/// erased form. +pub type ResolvedKernels = BitPackedKernels<()>; + +impl ResolvedKernels { + /// Resolves the kernels for an array of `ptype` packed to `bit_width` bits. + /// + /// Signed types resolve to their unsigned counterpart, which is what the packed buffer holds. + pub fn try_new(ptype: PType, bit_width: u8) -> VortexResult { + vortex_ensure!(ptype.is_int(), MismatchedTypes: "integer", ptype); + vortex_ensure!( + bit_width as usize <= ptype.bit_width(), + "Unsupported bit width {bit_width} for {ptype}" + ); + Ok(match_each_unsigned_integer_ptype!( + ptype.to_unsigned(), + |P| { P::resolve_kernels(bit_width).erase() } + )) + } + + /// The kernels typed for `P`, which must be the physical type they were resolved for. + /// + /// # Panics + /// + /// If `P` is not the physical type the kernels were resolved for. + #[inline] + pub fn typed(self) -> BitPackedKernels

{ + assert!( + self.ptype == P::PTYPE, + "BitPacked kernels were resolved for a different physical type" + ); + // SAFETY: `ptype` records the `P` these pointers were erased from (see `erase`), and + // transmuting a function pointer back to its original signature is lossless. + unsafe { + BitPackedKernels { + ptype: self.ptype, + bit_width: self.bit_width, + unpack: mem::transmute::, UnpackFn

>(self.unpack), + unpack_single: mem::transmute::, UnpackSingleFn

>( + self.unpack_single, + ), + unfor_pack: mem::transmute::, UnforPackFn

>(self.unfor_pack), + } + } + } +} + +impl BitPackedKernels

{ + /// Erases `P` from the function pointers; [`ResolvedKernels::typed`] restores it. + fn erase(self) -> ResolvedKernels { + // SAFETY: Function pointers of every signature share one layout, and the erased pointers + // are only ever called after `typed` casts them back to this signature, which `ptype` + // enforces. + unsafe { + BitPackedKernels { + ptype: self.ptype, + bit_width: self.bit_width, + unpack: mem::transmute::, UnpackFn<()>>(self.unpack), + unpack_single: mem::transmute::, UnpackSingleFn<()>>( + self.unpack_single, + ), + unfor_pack: mem::transmute::, UnforPackFn<()>>(self.unfor_pack), + } + } + } } /// The physical storage types of a bit-packed array, i.e. the unsigned integers the FastLanes /// kernels are implemented for. Signed arrays are packed as their unsigned counterpart. pub trait BitPackedPhysical: NativePType + BitPacking + BitPackingCompare + FoR { /// Resolves the kernels for `bit_width`, which must not exceed the width of `Self`. - fn resolve_kernels(bit_width: u8) -> ResolvedKernels; - - /// Returns the kernels if `resolved` holds kernels for `Self`. - fn kernels_from(resolved: &ResolvedKernels) -> Option>; + fn resolve_kernels(bit_width: u8) -> BitPackedKernels; /// Resolves the pack kernel for `bit_width`, which must not exceed the width of `Self`. /// @@ -166,17 +234,18 @@ fn block_len_mismatch(expected: usize, actual: usize) -> ! { } macro_rules! impl_bitpacked_physical { - ($P:ty, $variant:ident, $bits:literal) => { + ($P:ty, $bits:literal) => { impl BitPackedPhysical for $P { - fn resolve_kernels(bit_width: u8) -> ResolvedKernels { + fn resolve_kernels(bit_width: u8) -> BitPackedKernels { seq_macro::seq!(W in 0..=$bits { match bit_width { - #(W => ResolvedKernels::$variant(BitPackedKernels { + #(W => BitPackedKernels { + ptype: <$P as NativePType>::PTYPE, bit_width, unpack: unpack::<$P, W, { 1024 * W / $bits }>, unpack_single: unpack_single::<$P, W, { 1024 * W / $bits }>, unfor_pack: unfor_pack::<$P, W, { 1024 * W / $bits }>, - }),)* + },)* _ => vortex_panic!( "Unsupported bit width {bit_width} for {}", <$P as NativePType>::PTYPE @@ -185,13 +254,6 @@ macro_rules! impl_bitpacked_physical { }) } - fn kernels_from(resolved: &ResolvedKernels) -> Option> { - match resolved { - ResolvedKernels::$variant(kernels) => Some(*kernels), - _ => None, - } - } - fn resolve_pack(bit_width: u8) -> PackFn { seq_macro::seq!(W in 0..=$bits { match bit_width { @@ -223,10 +285,10 @@ macro_rules! impl_bitpacked_physical { }; } -impl_bitpacked_physical!(u8, U8, 8); -impl_bitpacked_physical!(u16, U16, 16); -impl_bitpacked_physical!(u32, U32, 32); -impl_bitpacked_physical!(u64, U64, 64); +impl_bitpacked_physical!(u8, 8); +impl_bitpacked_physical!(u16, 16); +impl_bitpacked_physical!(u32, 32); +impl_bitpacked_physical!(u64, 64); #[cfg(test)] mod tests { @@ -236,15 +298,22 @@ mod tests { use super::*; /// Every width of every physical type resolves to kernels that agree with the runtime-width - /// FastLanes entry points. + /// FastLanes entry points, both directly and after a round trip through the erased form. fn assert_kernels_match_fastlanes

() where P: BitPackedPhysical + WrappingAdd + FastLanesComparable, { let values: [P; 1024] = std::array::from_fn(|i| P::from(i % 251).unwrap()); for bit_width in 0..=(8 * size_of::

() as u8) { - let kernels = P::kernels_from(&P::resolve_kernels(bit_width)).unwrap(); + let kernels = ResolvedKernels::try_new(P::PTYPE, bit_width) + .unwrap() + .typed::

(); assert_eq!(kernels.bit_width(), bit_width); + assert_eq!( + kernels.unpack as usize, + P::resolve_kernels(bit_width).unpack as usize, + "erased round trip at width {bit_width}" + ); let block_len = kernels.packed_block_len(); let mut expected_packed = vec![P::zero(); block_len]; @@ -311,17 +380,33 @@ mod tests { } #[test] - fn kernels_from_rejects_other_types() { - let resolved = u16::resolve_kernels(3); - assert!(u16::kernels_from(&resolved).is_some()); - assert!(u8::kernels_from(&resolved).is_none()); - assert!(u32::kernels_from(&resolved).is_none()); + fn signed_resolves_to_unsigned_kernels() { + let resolved = ResolvedKernels::try_new(PType::I16, 3).unwrap(); + assert_eq!( + resolved.typed::().unpack as usize, + u16::resolve_kernels(3).unpack as usize + ); + } + + #[test] + fn rejects_width_beyond_type() { + assert!(ResolvedKernels::try_new(PType::U8, 9).is_err()); + assert!(ResolvedKernels::try_new(PType::F32, 3).is_err()); + assert!(ResolvedKernels::try_new(PType::U8, 8).is_ok()); + } + + #[test] + #[should_panic(expected = "resolved for a different physical type")] + fn typed_rejects_other_types() { + ResolvedKernels::try_new(PType::U16, 3) + .unwrap() + .typed::(); } #[test] #[should_panic(expected = "Expected a FastLanes block of 1024 elements")] fn unpack_rejects_short_output() { - let kernels = u8::kernels_from(&u8::resolve_kernels(1)).unwrap(); + let kernels = u8::resolve_kernels(1); let packed = [0u8; 128]; let mut output = [0u8; 512]; (kernels.unpack)(&packed, &mut output); diff --git a/encodings/fastlanes/src/bitpacking/array/mod.rs b/encodings/fastlanes/src/bitpacking/array/mod.rs index 1c7d91a68a3..6d92174f169 100644 --- a/encodings/fastlanes/src/bitpacking/array/mod.rs +++ b/encodings/fastlanes/src/bitpacking/array/mod.rs @@ -4,7 +4,6 @@ use std::fmt::Display; use std::fmt::Formatter; use std::mem::MaybeUninit; -use std::sync::OnceLock; use fastlanes::BitPacking; use vortex_array::ArrayRef; @@ -22,7 +21,6 @@ use vortex_array::patches::Patches; use vortex_array::patches::PatchesData; use vortex_array::validity::Validity; use vortex_array::vtable::child_to_validity; -use vortex_error::VortexExpect; use vortex_error::VortexResult; use vortex_error::vortex_ensure; use vortex_error::vortex_err; @@ -81,9 +79,9 @@ pub struct BitPackedData { pub(super) packed: BufferHandle, /// Patch metadata for reconstructing Patches from slots. pub(super) patches_data: Option, - /// FastLanes kernels for the physical type and bit width of this array, resolved on first use - /// so that decoding never dispatches on the runtime bit width. - kernels: OnceLock, + /// FastLanes kernels for the physical type and bit width of this array, resolved at + /// construction so that decoding never dispatches on the runtime bit width. + kernels: ResolvedKernels, } impl Display for BitPackedData { @@ -127,7 +125,7 @@ impl BitPackedData { /// /// # Validation /// - /// * The `ptype` must be an integer + /// * The `ptype` must be an integer and `bit_width` must not exceed its width /// * `validity` must have `length` len /// * Any patches must have any `array_len` equal to `length` /// * The `packed` buffer must be exactly sized to hold `length` values of `bit_width` rounded @@ -137,10 +135,10 @@ impl BitPackedData { pub fn try_new( packed: BufferHandle, patches: Option, + ptype: PType, bit_width: u8, offset: u16, ) -> VortexResult { - vortex_ensure!(bit_width <= 64, "Unsupported bit width {bit_width}"); vortex_ensure!( offset < 1024, "Offset must be less than the full block i.e., 1024, got {offset}" @@ -151,7 +149,7 @@ impl BitPackedData { bit_width, packed, patches_data: patches.as_ref().map(PatchesData::from_patches), - kernels: OnceLock::new(), + kernels: ResolvedKernels::try_new(ptype, bit_width)?, }) } @@ -255,21 +253,17 @@ impl BitPackedData { self.bit_width } - /// The FastLanes kernels for this array's bit width, resolved on first use. + /// The FastLanes kernels for this array's bit width, resolved when the array was built. /// /// `P` must be the physical type of the array, i.e. the unsigned counterpart of its - /// [`PType`]. The resolved kernels are cached, so later calls only copy out the pointers. + /// [`PType`]. /// /// # Panics /// - /// If the kernels were already resolved for a different physical type. + /// If `P` is not the array's physical type. #[inline] pub fn kernels(&self) -> BitPackedKernels

{ - let resolved = self - .kernels - .get_or_init(|| P::resolve_kernels(self.bit_width)); - P::kernels_from(resolved) - .vortex_expect("BitPacked kernels were resolved for a different physical type") + self.kernels.typed::

() } #[inline] @@ -359,15 +353,8 @@ pub trait BitPackedArrayExt: BitPackedArraySlotsExt { } /// The FastLanes kernels for this array's bit width, see [`BitPackedData::kernels`]. - /// - /// `P` must be the unsigned counterpart of the array's [`PType`]. #[inline] fn kernels(&self) -> BitPackedKernels

{ - assert_eq!( - P::PTYPE, - self.as_ref().dtype().as_ptype().to_unsigned(), - "Requested physical type doesn't match the array ptype" - ); BitPackedData::kernels::

(self) } } diff --git a/encodings/fastlanes/src/bitpacking/vtable/mod.rs b/encodings/fastlanes/src/bitpacking/vtable/mod.rs index 68fbf1b41d3..64fedf37258 100644 --- a/encodings/fastlanes/src/bitpacking/vtable/mod.rs +++ b/encodings/fastlanes/src/bitpacking/vtable/mod.rs @@ -239,6 +239,7 @@ impl VTable for BitPacked { let data = BitPackedData::try_new( packed, patches, + dtype.as_ptype(), u8::try_from(metadata.bit_width).map_err(|_| { vortex_err!( "BitPackedMetadata bit_width {} does not fit in u8", @@ -319,7 +320,7 @@ impl BitPacked { s.push(validity_to_child(&validity, len)); s }; - let data = BitPackedData::try_new(packed, patches, bit_width, offset)?; + let data = BitPackedData::try_new(packed, patches, ptype, bit_width, offset)?; Array::try_from_parts(ArrayParts::new(BitPacked, dtype, len, data).with_slots(slots)) } From 5dc1cb11d97d7e039ab347faf9555424972a1cd9 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 10:15:41 +0000 Subject: [PATCH 5/6] refactor(fastlanes): unchecked block casts and single bit-width record Make the resolved kernel pointers `unsafe fn` with the same length contract the fastlanes `unchecked_*` entry points had. The wrappers now reinterpret the slices with a `debug_assert` and a pointer cast, as the original code did, instead of a checked `try_into` and an out-of-line panic. In release the block wrappers reduce to a bare tail jump into the fastlanes kernel. Callers carry the SAFETY reasoning the checks used to enforce. `BitPackedData` no longer duplicates `bit_width`: the resolved kernels already record it, and `bit_width()` reads it from there. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01R9mScTPzB4Vw4PCdSyc862 Signed-off-by: Claude --- .../src/bitpacking/array/bitpack_compress.rs | 26 ++-- .../bitpacking/array/bitpack_decompress.rs | 3 +- .../fastlanes/src/bitpacking/array/kernels.rs | 130 ++++++++++-------- .../fastlanes/src/bitpacking/array/mod.rs | 14 +- .../src/bitpacking/array/unpack_iter.rs | 43 ++++-- .../src/bitpacking/compute/compare_fused.rs | 7 +- .../src/bitpacking/compute/filter.rs | 27 ++-- .../fastlanes/src/bitpacking/compute/take.rs | 14 +- encodings/fastlanes/src/bitpacking/plugin.rs | 2 +- .../fastlanes/src/bitpacking/vtable/mod.rs | 8 +- .../fastlanes/src/for/array/for_decompress.rs | 6 +- 11 files changed, 169 insertions(+), 111 deletions(-) diff --git a/encodings/fastlanes/src/bitpacking/array/bitpack_compress.rs b/encodings/fastlanes/src/bitpacking/array/bitpack_compress.rs index 3e1ab9ebc86..8fac1aabb37 100644 --- a/encodings/fastlanes/src/bitpacking/array/bitpack_compress.rs +++ b/encodings/fastlanes/src/bitpacking/array/bitpack_compress.rs @@ -165,13 +165,15 @@ pub fn bitpack_primitive(array: &[T], bit_width: u8) -> Bu (0..num_full_chunks).for_each(|i| { let start_elem = i * 1024; let output_len = output.len(); - // SAFETY: The capacity holds every block, and `pack` initializes the new elements before - // they are read. - unsafe { output.set_len(output_len + packed_len) }; - pack( - &array[start_elem..][..1024], - &mut output[output_len..][..packed_len], - ); + // SAFETY: The capacity holds every block, so the new slots exist; the input is exactly + // 1024 values and the output exactly one block, which `pack` fully initializes. + unsafe { + output.set_len(output_len + packed_len); + pack( + &array[start_elem..][..1024], + &mut output[output_len..][..packed_len], + ); + } }); // Pad the last chunk with zeros to a full 1024 elements. @@ -181,10 +183,12 @@ pub fn bitpack_primitive(array: &[T], bit_width: u8) -> Bu last_chunk[..last_chunk_size].copy_from_slice(&array[array.len() - last_chunk_size..]); let output_len = output.len(); - // SAFETY: The capacity holds every block, and `pack` initializes the new elements before - // they are read. - unsafe { output.set_len(output_len + packed_len) }; - pack(&last_chunk, &mut output[output_len..][..packed_len]); + // SAFETY: The capacity holds every block, so the new slots exist; the input is exactly + // 1024 values and the output exactly one block, which `pack` fully initializes. + unsafe { + output.set_len(output_len + packed_len); + pack(&last_chunk, &mut output[output_len..][..packed_len]); + } } output.freeze() diff --git a/encodings/fastlanes/src/bitpacking/array/bitpack_decompress.rs b/encodings/fastlanes/src/bitpacking/array/bitpack_decompress.rs index 6a1f9a2cf4e..459236afa52 100644 --- a/encodings/fastlanes/src/bitpacking/array/bitpack_decompress.rs +++ b/encodings/fastlanes/src/bitpacking/array/bitpack_decompress.rs @@ -192,7 +192,8 @@ pub fn unpack_single_primitive( let elems_per_chunk = kernels.packed_block_len(); let packed_chunk = &packed[chunk_index * elems_per_chunk..][..elems_per_chunk]; - (kernels.unpack_single)(packed_chunk, index_in_chunk) + // SAFETY: `packed_chunk` is exactly one block at the width `kernels` were resolved for. + unsafe { (kernels.unpack_single)(packed_chunk, index_in_chunk) } } pub fn count_exceptions(bit_width: u8, bit_width_freq: &[usize]) -> usize { diff --git a/encodings/fastlanes/src/bitpacking/array/kernels.rs b/encodings/fastlanes/src/bitpacking/array/kernels.rs index 5155760cbb8..f4793802a29 100644 --- a/encodings/fastlanes/src/bitpacking/array/kernels.rs +++ b/encodings/fastlanes/src/bitpacking/array/kernels.rs @@ -26,37 +26,50 @@ use vortex_error::vortex_panic; /// Packs one FastLanes block of 1024 values. /// +/// # Safety +/// /// `input` must hold exactly 1024 elements and `output` exactly `128 * bit_width / size_of::

()`. -pub type PackFn

= fn(input: &[P], output: &mut [P]); +/// The lengths are checked only with `debug_assert`. +pub type PackFn

= unsafe fn(input: &[P], output: &mut [P]); /// Unpacks one FastLanes block of 1024 values. /// +/// # Safety +/// /// `packed` must hold exactly [`BitPackedKernels::packed_block_len`] elements and `output` -/// exactly 1024. -pub type UnpackFn

= fn(packed: &[P], output: &mut [P]); +/// exactly 1024. The lengths are checked only with `debug_assert`. +pub type UnpackFn

= unsafe fn(packed: &[P], output: &mut [P]); -/// Unpacks the value at `index` (`< 1024`) of one packed FastLanes block. +/// Unpacks the value at `index` of one packed FastLanes block. +/// +/// # Safety /// -/// `packed` must hold exactly [`BitPackedKernels::packed_block_len`] elements. -pub type UnpackSingleFn

= fn(packed: &[P], index: usize) -> P; +/// `packed` must hold exactly [`BitPackedKernels::packed_block_len`] elements. The length is +/// checked only with `debug_assert`. An `index` of 1024 or more panics. +pub type UnpackSingleFn

= unsafe fn(packed: &[P], index: usize) -> P; /// Unpacks one FastLanes block and wrapping-adds `reference` to every value. /// +/// # Safety +/// /// `packed` must hold exactly [`BitPackedKernels::packed_block_len`] elements and `output` -/// exactly 1024. -pub type UnforPackFn

= fn(packed: &[P], reference: P, output: &mut [P]); +/// exactly 1024. The lengths are checked only with `debug_assert`. +pub type UnforPackFn

= unsafe fn(packed: &[P], reference: P, output: &mut [P]); /// Unpacks one FastLanes block, comparing each value against `rhs` with `cmp` and writing the /// results as a lane-major 1024-bit mask. See [`BitPackingCompare::unpack_cmp`] for the layout. /// -/// `packed` must hold exactly `128 * bit_width / size_of::

()` elements. -pub type UnpackCmpFn = fn(packed: &[P], output: &mut [u64; 16], cmp: F, rhs: V); +/// # Safety +/// +/// `packed` must hold exactly `128 * bit_width / size_of::

()` elements. The length is checked +/// only with `debug_assert`. +pub type UnpackCmpFn = unsafe fn(packed: &[P], output: &mut [u64; 16], cmp: F, rhs: V); /// FastLanes kernels resolved for one bit width of physical type `P`. /// /// Obtained from [`BitPackedData::kernels`](crate::BitPackedData::kernels). Each kernel is the /// const-width instantiation for the array's bit width, so calling it does not dispatch on the -/// width. The kernels check the block lengths they are handed and panic on a mismatch. +/// width. #[derive(Clone, Copy, Debug)] pub struct BitPackedKernels

{ /// The unsigned [`PType`] of `P`, kept so the type-erased form can check it before @@ -71,13 +84,15 @@ pub struct BitPackedKernels

{ pub unfor_pack: UnforPackFn

, } -impl BitPackedKernels

{ +impl

BitPackedKernels

{ /// The bit width the kernels were resolved for. #[inline] pub fn bit_width(&self) -> u8 { self.bit_width } +} +impl BitPackedKernels

{ /// The number of `P` elements holding one packed block of 1024 values. #[inline] pub fn packed_block_len(&self) -> usize { @@ -176,27 +191,34 @@ pub trait BitPackedPhysical: NativePType + BitPacking + BitPackingCompare + FoR F: Fn(V, V) -> bool; } -fn pack(input: &[P], output: &mut [P]) { - P::pack::(as_block(input), as_block_mut(output)); +unsafe fn pack(input: &[P], output: &mut [P]) { + // SAFETY: The caller upholds the `PackFn` length contract. + unsafe { P::pack::(as_block(input), as_block_mut(output)) } } -fn unpack(packed: &[P], output: &mut [P]) { - P::unpack::(as_block(packed), as_block_mut(output)); +unsafe fn unpack(packed: &[P], output: &mut [P]) { + // SAFETY: The caller upholds the `UnpackFn` length contract. + unsafe { P::unpack::(as_block(packed), as_block_mut(output)) } } -fn unpack_single(packed: &[P], index: usize) -> P { - P::unpack_single::(as_block(packed), index) +unsafe fn unpack_single( + packed: &[P], + index: usize, +) -> P { + // SAFETY: The caller upholds the `UnpackSingleFn` length contract. + P::unpack_single::(unsafe { as_block(packed) }, index) } -fn unfor_pack( +unsafe fn unfor_pack( packed: &[P], reference: P, output: &mut [P], ) { - P::unfor_pack::(as_block(packed), reference, as_block_mut(output)); + // SAFETY: The caller upholds the `UnforPackFn` length contract. + unsafe { P::unfor_pack::(as_block(packed), reference, as_block_mut(output)) } } -fn unpack_cmp( +unsafe fn unpack_cmp( packed: &[P], output: &mut [u64; 16], cmp: F, @@ -205,32 +227,32 @@ fn unpack_cmp( V: FastLanesComparable, F: Fn(V, V) -> bool, { - P::unpack_cmp::(as_block(packed), output, cmp, rhs); + // SAFETY: The caller upholds the `UnpackCmpFn` length contract. + P::unpack_cmp::(unsafe { as_block(packed) }, output, cmp, rhs); } +/// Reinterprets `slice` as a block of exactly `N` elements. +/// +/// # Safety +/// +/// `slice.len()` must be `N`. This is checked only with `debug_assert`. #[inline(always)] -fn as_block(slice: &[P]) -> &[P; N] { - match slice.try_into() { - Ok(block) => block, - Err(_) => block_len_mismatch(N, slice.len()), - } +unsafe fn as_block(slice: &[P]) -> &[P; N] { + debug_assert_eq!(slice.len(), N); + // SAFETY: The caller guarantees `N` elements, and `[P; N]` has the alignment of `P`. + unsafe { &*slice.as_ptr().cast::<[P; N]>() } } +/// Reinterprets `slice` as a mutable block of exactly `N` elements. +/// +/// # Safety +/// +/// `slice.len()` must be `N`. This is checked only with `debug_assert`. #[inline(always)] -fn as_block_mut(slice: &mut [P]) -> &mut [P; N] { - let len = slice.len(); - match slice.try_into() { - Ok(block) => block, - Err(_) => block_len_mismatch(N, len), - } -} - -/// Kept out of line so the kernel wrappers stay frameless trampolines: the panic formatting -/// would otherwise reserve stack on every call. -#[cold] -#[inline(never)] -fn block_len_mismatch(expected: usize, actual: usize) -> ! { - vortex_panic!("Expected a FastLanes block of {expected} elements, got {actual}") +unsafe fn as_block_mut(slice: &mut [P]) -> &mut [P; N] { + debug_assert_eq!(slice.len(), N); + // SAFETY: The caller guarantees `N` elements, and `[P; N]` has the alignment of `P`. + unsafe { &mut *slice.as_mut_ptr().cast::<[P; N]>() } } macro_rules! impl_bitpacked_physical { @@ -322,20 +344,23 @@ mod tests { unsafe { P::unchecked_pack(bit_width as usize, &values, &mut expected_packed) }; let mut packed = vec![P::zero(); block_len]; - P::resolve_pack(bit_width)(&values, &mut packed); + // SAFETY: `values` holds 1024 values and `packed` exactly one block at `bit_width`. + unsafe { P::resolve_pack(bit_width)(&values, &mut packed) }; assert_eq!(packed, expected_packed, "pack at width {bit_width}"); let mut expected = [P::zero(); 1024]; // SAFETY: `packed` holds exactly one block at `bit_width` and `expected` 1024 values. unsafe { P::unchecked_unpack(bit_width as usize, &packed, &mut expected) }; + // SAFETY: `packed` holds exactly one block at `bit_width` and `unpacked` 1024 values. let mut unpacked = [P::zero(); 1024]; - (kernels.unpack)(&packed, &mut unpacked); + unsafe { (kernels.unpack)(&packed, &mut unpacked) }; assert_eq!(unpacked, expected, "unpack at width {bit_width}"); for index in [0, 1, 511, 1023] { assert_eq!( - (kernels.unpack_single)(&packed, index), + // SAFETY: `packed` holds exactly one block at `bit_width`. + unsafe { (kernels.unpack_single)(&packed, index) }, expected[index], "unpack_single at width {bit_width} index {index}" ); @@ -343,7 +368,8 @@ mod tests { let reference = P::from(7).unwrap(); let mut unfor = [P::zero(); 1024]; - (kernels.unfor_pack)(&packed, reference, &mut unfor); + // SAFETY: `packed` holds exactly one block at `bit_width` and `unfor` 1024 values. + unsafe { (kernels.unfor_pack)(&packed, reference, &mut unfor) }; for (got, want) in unfor.iter().zip(expected) { assert_eq!( *got, @@ -354,7 +380,10 @@ mod tests { let rhs = P::from(100).unwrap(); let mut mask = [0u64; 16]; - P::resolve_unpack_cmp::(bit_width)(&packed, &mut mask, |a, b| a < b, rhs); + // SAFETY: `packed` holds exactly one block at `bit_width`. + unsafe { + P::resolve_unpack_cmp::(bit_width)(&packed, &mut mask, |a, b| a < b, rhs) + }; let mut expected_mask = [0u64; 16]; // SAFETY: `packed` holds exactly one block at `bit_width`. unsafe { @@ -402,13 +431,4 @@ mod tests { .unwrap() .typed::(); } - - #[test] - #[should_panic(expected = "Expected a FastLanes block of 1024 elements")] - fn unpack_rejects_short_output() { - let kernels = u8::resolve_kernels(1); - let packed = [0u8; 128]; - let mut output = [0u8; 512]; - (kernels.unpack)(&packed, &mut output); - } } diff --git a/encodings/fastlanes/src/bitpacking/array/mod.rs b/encodings/fastlanes/src/bitpacking/array/mod.rs index 6d92174f169..c83cc31274b 100644 --- a/encodings/fastlanes/src/bitpacking/array/mod.rs +++ b/encodings/fastlanes/src/bitpacking/array/mod.rs @@ -75,18 +75,23 @@ pub struct BitPackedData { /// The offset within the first block (created with a slice). /// 0 <= offset < 1024 pub(super) offset: u16, - pub(super) bit_width: u8, pub(super) packed: BufferHandle, /// Patch metadata for reconstructing Patches from slots. pub(super) patches_data: Option, /// FastLanes kernels for the physical type and bit width of this array, resolved at - /// construction so that decoding never dispatches on the runtime bit width. + /// construction so that decoding never dispatches on the runtime bit width. Also the only + /// record of the bit width. kernels: ResolvedKernels, } impl Display for BitPackedData { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - write!(f, "bit_width: {}, offset: {}", self.bit_width, self.offset) + write!( + f, + "bit_width: {}, offset: {}", + self.bit_width(), + self.offset + ) } } @@ -146,7 +151,6 @@ impl BitPackedData { Ok(Self { offset, - bit_width, packed, patches_data: patches.as_ref().map(PatchesData::from_patches), kernels: ResolvedKernels::try_new(ptype, bit_width)?, @@ -250,7 +254,7 @@ impl BitPackedData { /// Bit-width of the packed values #[inline] pub fn bit_width(&self) -> u8 { - self.bit_width + self.kernels.bit_width() } /// The FastLanes kernels for this array's bit width, resolved when the array was built. diff --git a/encodings/fastlanes/src/bitpacking/array/unpack_iter.rs b/encodings/fastlanes/src/bitpacking/array/unpack_iter.rs index f57a33625aa..58f948b7ad6 100644 --- a/encodings/fastlanes/src/bitpacking/array/unpack_iter.rs +++ b/encodings/fastlanes/src/bitpacking/array/unpack_iter.rs @@ -24,9 +24,12 @@ const CHUNK_SIZE: usize = FL_CHUNK_SIZE; pub trait UnpackStrategy { /// Unpack a chunk of packed data into the destination buffer. /// - /// `chunk` must contain exactly one packed block (`128 * bit_width / size_of::()` - /// elements) and `dst` exactly `CHUNK_SIZE` elements. - fn unpack_chunk(&self, chunk: &[T::Physical], dst: &mut [T::Physical]); + /// # Safety + /// + /// - `chunk` must contain exactly one packed block (`128 * bit_width / size_of::()` + /// elements) + /// - `dst` must contain exactly `CHUNK_SIZE` elements + unsafe fn unpack_chunk(&self, chunk: &[T::Physical], dst: &mut [T::Physical]); } /// BitPacking strategy - plain bitpacking without reference value, using the unpack kernel @@ -38,8 +41,9 @@ pub struct BitPackingStrategy

{ impl UnpackStrategy for BitPackingStrategy { #[allow(clippy::inline_always)] #[inline(always)] - fn unpack_chunk(&self, chunk: &[T::Physical], dst: &mut [T::Physical]) { - (self.unpack)(chunk, dst); + unsafe fn unpack_chunk(&self, chunk: &[T::Physical], dst: &mut [T::Physical]) { + // SAFETY: The caller upholds the `unpack_chunk` length contract, which is `UnpackFn`'s. + unsafe { (self.unpack)(chunk, dst) } } } @@ -170,9 +174,13 @@ impl<'a, T: PhysicalPType, S: UnpackStrategy> UnpackedChunks<'a, T, S> { } else { CHUNK_SIZE - self.offset }; - self.strategy.unpack_chunk(chunk, dst); - // SAFETY: `unpack_chunk` initialized every element of the buffer. - unsafe { mem::transmute(&mut self.scratch[self.offset..][..header_end_slice]) } + // SAFETY: + // 1. chunk is elems_per_chunk. + // 2. buffer is exactly CHUNK_SIZE, and `unpack_chunk` initializes all of it. + unsafe { + self.strategy.unpack_chunk(chunk, dst); + mem::transmute(&mut self.scratch[self.offset..][..header_end_slice]) + } }) } @@ -293,9 +301,13 @@ impl<'a, T: PhysicalPType, S: UnpackStrategy> UnpackedChunks<'a, T, S> { [(self.num_chunks - 1) * self.elems_per_chunk()..][..self.elems_per_chunk()]; let dst: &mut [MaybeUninit] = self.scratch; let dst: &mut [T::Physical] = unsafe { mem::transmute(dst) }; - self.strategy.unpack_chunk(chunk, dst); - // SAFETY: `unpack_chunk` initialized every element of the buffer. - unsafe { mem::transmute(&mut self.scratch[..self.last_chunk_length]) } + // SAFETY: + // 1. chunk is elems_per_chunk. + // 2. buffer is exactly CHUNK_SIZE, and `unpack_chunk` initializes all of it. + unsafe { + self.strategy.unpack_chunk(chunk, dst); + mem::transmute(&mut self.scratch[..self.last_chunk_length]) + } }) } @@ -397,9 +409,12 @@ impl<'a, T: BitPacked + 'a> LendingIterator for BitUnpackIterator<'a, T> { let chunk = &self.packed[self.idx * self.elems_per_chunk..][..self.elems_per_chunk]; let dst: &mut [MaybeUninit] = self.buffer; - // SAFETY: &[MaybeUninit] and &[T::Physical] have the same layout. - let dst: &mut [T::Physical] = unsafe { mem::transmute(dst) }; - (self.unpack)(chunk, dst); + // SAFETY: &[MaybeUninit] and &[T::Physical] have the same layout; `chunk` is exactly + // `elems_per_chunk` and `dst` exactly CHUNK_SIZE. + unsafe { + let dst: &mut [T::Physical] = mem::transmute(dst); + (self.unpack)(chunk, dst); + } self.idx += 1; // SAFETY: The buffer has the appropriate lifetime, the iterator signature doesn't account for it Some(unsafe { mem::transmute::<&mut [MaybeUninit; 1024], &mut [T; 1024]>(self.buffer) }) diff --git a/encodings/fastlanes/src/bitpacking/compute/compare_fused.rs b/encodings/fastlanes/src/bitpacking/compute/compare_fused.rs index 6a6afc2f007..3abf9774297 100644 --- a/encodings/fastlanes/src/bitpacking/compute/compare_fused.rs +++ b/encodings/fastlanes/src/bitpacking/compute/compare_fused.rs @@ -109,9 +109,10 @@ where let out = words[range.start / U64_BITS..] .first_chunk_mut::() .vortex_expect("over-allocated buffer holds a full block per chunk"); - // The kernel assigns every word in `lane_major`, so its previous contents are - // irrelevant. - unpack_cmp(packed_chunk, &mut lane_major, cmp, rhs); + // SAFETY: `packed_chunk` holds exactly one block at the array's bit width, which is + // the width `unpack_cmp` was resolved for. The kernel assigns every word in + // `lane_major`, so its previous contents are irrelevant. + unsafe { unpack_cmp(packed_chunk, &mut lane_major, cmp, rhs) }; transpose_bits::<::Physical>(&lane_major, out); }, )?; diff --git a/encodings/fastlanes/src/bitpacking/compute/filter.rs b/encodings/fastlanes/src/bitpacking/compute/filter.rs index b4bc2b33f14..2f77a72b9ab 100644 --- a/encodings/fastlanes/src/bitpacking/compute/filter.rs +++ b/encodings/fastlanes/src/bitpacking/compute/filter.rs @@ -140,17 +140,23 @@ fn filter_with_indices( if indices_within_chunk.len() == 1024 { // Unpack the entire chunk. - let values_len = values.len(); - // SAFETY: The capacity holds every index, and `unpack` initializes all 1024 - // values before they are read. - unsafe { values.set_len(values_len + 1024) }; - (kernels.unpack)(packed, &mut values.as_mut_slice()[values_len..]); + // SAFETY: The capacity holds every index, so the 1024 new slots exist; `packed` + // is exactly one block and `unpack` initializes all 1024 slots before they are + // read. + unsafe { + let values_len = values.len(); + values.set_len(values_len + 1024); + (kernels.unpack)(packed, &mut values.as_mut_slice()[values_len..]); + } } else if indices_within_chunk.len() > UNPACK_CHUNK_THRESHOLD { // Unpack into a temporary chunk and then copy the values. - let dst: &mut [MaybeUninit] = &mut unpacked; - // SAFETY: &[MaybeUninit] and &[T] have the same layout. - let dst: &mut [T] = unsafe { std::mem::transmute(dst) }; - (kernels.unpack)(packed, dst); + // SAFETY: &[MaybeUninit] and &[T] have the same layout; `packed` is exactly + // one block and `unpacked` exactly 1024 values. + unsafe { + let dst: &mut [MaybeUninit] = &mut unpacked; + let dst: &mut [T] = std::mem::transmute(dst); + (kernels.unpack)(packed, dst); + } values.extend_trusted( indices_within_chunk .iter() @@ -158,10 +164,11 @@ fn filter_with_indices( ); } else { // Otherwise, unpack each element individually. + // SAFETY: `packed` is exactly one block and every index is within the chunk. values.extend_trusted( indices_within_chunk .iter() - .map(|&idx| (kernels.unpack_single)(packed, idx)), + .map(|&idx| unsafe { (kernels.unpack_single)(packed, idx) }), ); } }, diff --git a/encodings/fastlanes/src/bitpacking/compute/take.rs b/encodings/fastlanes/src/bitpacking/compute/take.rs index 58dd2c5edaf..ec6ad18a3d4 100644 --- a/encodings/fastlanes/src/bitpacking/compute/take.rs +++ b/encodings/fastlanes/src/bitpacking/compute/take.rs @@ -102,10 +102,13 @@ fn take_primitive( // this loop only runs if we have at least UNPACK_CHUNK_THRESHOLD offsets for offset_chunk in offset_chunks { if !have_unpacked { - let dst: &mut [MaybeUninit] = &mut unpacked; - // SAFETY: &[MaybeUninit] and &[T] have the same layout. - let dst: &mut [T] = unsafe { mem::transmute(dst) }; - (kernels.unpack)(packed, dst); + // SAFETY: &[MaybeUninit] and &[T] have the same layout; `packed` is exactly + // one block and `unpacked` exactly 1024 values. + unsafe { + let dst: &mut [MaybeUninit] = &mut unpacked; + let dst: &mut [T] = mem::transmute(dst); + (kernels.unpack)(packed, dst); + } have_unpacked = true; } @@ -125,7 +128,8 @@ fn take_primitive( // we had fewer than UNPACK_CHUNK_THRESHOLD offsets in the first place, // so we need to unpack each one individually for &index in remainder { - output.push((kernels.unpack_single)(packed, index)); + // SAFETY: `packed` is exactly one block and `index` is within the chunk. + output.push(unsafe { (kernels.unpack_single)(packed, index) }); } } } diff --git a/encodings/fastlanes/src/bitpacking/plugin.rs b/encodings/fastlanes/src/bitpacking/plugin.rs index 3ff07db7e5c..fa4692ead67 100644 --- a/encodings/fastlanes/src/bitpacking/plugin.rs +++ b/encodings/fastlanes/src/bitpacking/plugin.rs @@ -75,7 +75,7 @@ impl ArrayPlugin for BitPackedPatchedPlugin { let packed = bitpacked.packed().clone(); let ptype = bitpacked.dtype().as_ptype(); let validity = bitpacked.validity()?; - let bw = bitpacked.bit_width; + let bw = bitpacked.bit_width(); let len = bitpacked.len(); let offset = bitpacked.offset(); diff --git a/encodings/fastlanes/src/bitpacking/vtable/mod.rs b/encodings/fastlanes/src/bitpacking/vtable/mod.rs index 64fedf37258..b8ebee7a26f 100644 --- a/encodings/fastlanes/src/bitpacking/vtable/mod.rs +++ b/encodings/fastlanes/src/bitpacking/vtable/mod.rs @@ -75,7 +75,7 @@ pub struct BitPackedMetadata { impl ArrayHash for BitPackedData { fn array_hash(&self, state: &mut H, accuracy: EqMode) { self.offset.hash(state); - self.bit_width.hash(state); + self.bit_width().hash(state); self.packed.array_hash(state, accuracy); self.patches_data.hash(state); } @@ -84,7 +84,7 @@ impl ArrayHash for BitPackedData { impl ArrayEq for BitPackedData { fn array_eq(&self, other: &Self, accuracy: EqMode) -> bool { self.offset == other.offset - && self.bit_width == other.bit_width + && self.bit_width() == other.bit_width() && self.packed.array_eq(&other.packed, accuracy) && self.patches_data == other.patches_data } @@ -118,7 +118,7 @@ impl VTable for BitPacked { dtype.as_ptype(), &validity, patches.as_ref(), - data.bit_width, + data.bit_width(), len, data.offset, ) @@ -331,7 +331,7 @@ impl BitPacked { let data = array.into_data(); BitPackedDataParts { offset: data.offset, - bit_width: data.bit_width, + bit_width: data.bit_width(), len, packed: data.packed, patches, diff --git a/encodings/fastlanes/src/for/array/for_decompress.rs b/encodings/fastlanes/src/for/array/for_decompress.rs index 93684fb0ccd..61f1666d8d8 100644 --- a/encodings/fastlanes/src/for/array/for_decompress.rs +++ b/encodings/fastlanes/src/for/array/for_decompress.rs @@ -40,8 +40,10 @@ struct FoRStrategy { impl + BitPackedPhysical> UnpackStrategy for FoRStrategy { #[allow(clippy::inline_always)] #[inline(always)] - fn unpack_chunk(&self, chunk: &[T::Physical], dst: &mut [T::Physical]) { - (self.unfor_pack)(chunk, self.reference, dst); + unsafe fn unpack_chunk(&self, chunk: &[T::Physical], dst: &mut [T::Physical]) { + // SAFETY: The caller upholds the `unpack_chunk` length contract, which is + // `UnforPackFn`'s. + unsafe { (self.unfor_pack)(chunk, self.reference, dst) } } } From 0c12a8fbb52e8103571c0ab9572d509a0dac9d11 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 10:23:02 +0000 Subject: [PATCH 6/6] chore(fastlanes): drop inline(always) from block casts Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01R9mScTPzB4Vw4PCdSyc862 Signed-off-by: Claude --- encodings/fastlanes/src/bitpacking/array/kernels.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/encodings/fastlanes/src/bitpacking/array/kernels.rs b/encodings/fastlanes/src/bitpacking/array/kernels.rs index f4793802a29..f098edf0f69 100644 --- a/encodings/fastlanes/src/bitpacking/array/kernels.rs +++ b/encodings/fastlanes/src/bitpacking/array/kernels.rs @@ -236,7 +236,6 @@ unsafe fn unpack_cmp /// # Safety /// /// `slice.len()` must be `N`. This is checked only with `debug_assert`. -#[inline(always)] unsafe fn as_block(slice: &[P]) -> &[P; N] { debug_assert_eq!(slice.len(), N); // SAFETY: The caller guarantees `N` elements, and `[P; N]` has the alignment of `P`. @@ -248,7 +247,6 @@ unsafe fn as_block(slice: &[P]) -> &[P; N] { /// # Safety /// /// `slice.len()` must be `N`. This is checked only with `debug_assert`. -#[inline(always)] unsafe fn as_block_mut(slice: &mut [P]) -> &mut [P; N] { debug_assert_eq!(slice.len(), N); // SAFETY: The caller guarantees `N` elements, and `[P; N]` has the alignment of `P`.