mbutrovich commented on code in PR #11829:
URL: https://github.com/apache/arrow-rs/pull/11829#discussion_r4354827153
##########
arrow-select/src/take.rs:
##########
@@ -207,7 +207,14 @@ where
};
};
- if indices.null_count() > 0 {
+ // Null slots usually hold in-bounds values, so first check every value
with a loop that
+ // vectorizes, and look at validity only if that finds one out of bounds
+ let in_bounds = indices.values().iter().fold(true, |in_bounds, &i| {
+ in_bounds & (i >= T::Native::ZERO) & (i < len)
Review Comment:
> using bit and here is intentional to enable vectorization?
Yep. This is the fold from #8966, moved unchanged from the [no-nulls branch
on
`main`](https://github.com/apache/arrow-rs/blob/01a9917658fecd639991e92a5a5810be710b539c/arrow-select/src/take.rs#L220-L222)
so that it also runs when the indices have nulls. `&&` would short-circuit,
which puts a branch on each element. With `&`, every element takes the same
path and LLVM vectorizes the loop, as the godbolt link in #8966 shows.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
