Skip to content

Commit

Permalink
Fix bitmask creation in chunked part of simd comparison (#1286)
Browse files Browse the repository at this point in the history
  • Loading branch information
jhorstmann authored and alamb committed Feb 7, 2022
1 parent fa72873 commit 411171a
Showing 1 changed file with 15 additions and 1 deletion.
16 changes: 15 additions & 1 deletion arrow/src/compute/kernels/comparison.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1708,7 +1708,7 @@ where
let simd_result = simd_op(simd_left, simd_right);

let m = T::mask_to_u64(&simd_result);
bitmask |= m << (i / lanes);
bitmask |= m << i;

i += lanes;
}
Expand Down Expand Up @@ -2442,6 +2442,20 @@ mod tests {
let b = b.slice(0, b.len());
let c = $DYN_KERNEL(a.as_ref(), b.as_ref()).unwrap();
assert_eq!(BooleanArray::from($EXPECTED), c);

// test with a larger version of the same data to ensure we cover the chunked part of the comparison
let mut a = vec![];
let mut b = vec![];
let mut e = vec![];
for _i in 0..10 {
a.extend($A_VEC);
b.extend($B_VEC);
e.extend($EXPECTED);
}
let a = $ARRAY::from(a);
let b = $ARRAY::from(b);
let c = $KERNEL(&a, &b).unwrap();
assert_eq!(BooleanArray::from(e), c);
};
}

Expand Down

3 comments on commit 411171a

@JasonLi-cn
Copy link
Contributor

Choose a reason for hiding this comment

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

The simd_compare_op_scalar function also needs fixing

@alamb
Copy link
Contributor

@alamb alamb commented on 411171a Feb 9, 2022

Choose a reason for hiding this comment

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

Thank you for the report @JasonLi-cn -- FYI @jhorstmann . I will look into this later today if you haven't had a chance to

@alamb
Copy link
Contributor

@alamb alamb commented on 411171a Feb 9, 2022

Choose a reason for hiding this comment

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

The simd_compare_op_scalar function also needs fixing

Fixed in #1290 thank you @jhorstmann

Please sign in to comment.