Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bq with u8 alignment support #25

Merged
merged 10 commits into from
May 30, 2024
Merged

Conversation

IvanPleshkov
Copy link
Collaborator

@IvanPleshkov IvanPleshkov commented May 24, 2024

For multivector 128-dim alignment is too large. This PR introduces generic BQ storage with u128 and u8 alignment.

In u128 case there are no any changes for backward compatibility. In u8 case there is a flexible alignment: if dim >= 128, alignment is 128 and simd is optimized for 128-bit alignment (the same for 64 and 32).

@IvanPleshkov IvanPleshkov requested a review from generall May 24, 2024 10:07
@IvanPleshkov IvanPleshkov marked this pull request as draft May 24, 2024 10:12
@IvanPleshkov IvanPleshkov marked this pull request as draft May 24, 2024 10:12
EXPORT uint64_t impl_xor_popcnt_neon(
const uint64_t* query_ptr,
const uint64_t* vector_ptr,
EXPORT uint32_t impl_xor_popcnt_neon_uint128(
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just simplification. not much difference in benches


return (uint64_t)vaddvq_u32(result);
EXPORT uint32_t impl_xor_popcnt_neon_uint64(
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are no xor_popcnt for u32 in neon, because neon doesn't have intrinsic for 32bit popcnt

stop_condition: impl Fn() -> bool,
) -> Result<Self, EncodingError> {
debug_assert!(validate_vector_parameters(orig_data.clone(), vector_parameters).is_ok());
pub trait BitsStoreType:
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To separate dense and multivector behaviour, I defined here a generic type which is u128 for dense and u8 for multivector

#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
if is_x86_feature_detected!("sse4.2") {
unsafe {
if v1.len() > 16 {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For large-dim multivector use more efficient simd implementation


fn encode_vector(vector: &[f32]) -> EncodedBinVector<TBitsStoreType> {
let mut encoded_vector =
vec![Default::default(); TBitsStoreType::get_storage_size(vector.len())];
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just realised that for u128 we may have the wrong alignment here. Need additional investigation but it's not a part of this PR

@IvanPleshkov IvanPleshkov marked this pull request as ready for review May 30, 2024 09:46
@IvanPleshkov IvanPleshkov merged commit 0caf67d into master May 30, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants