lidavidm commented on a change in pull request #10538:
URL: https://github.com/apache/arrow/pull/10538#discussion_r663273941
##########
File path: cpp/src/arrow/compute/kernels/scalar_if_else.cc
##########
@@ -201,70 +226,61 @@ static constexpr int64_t word_len = sizeof(Word) * 8;
/// Runs the main if_else loop. Here, it is expected that the right data has
already
/// been copied to the output.
/// If `invert` is meant to invert the cond.data. If is set to `true`, then the
-/// buffer will be inverted before calling the handle_bulk or handle_each
functions.
+/// buffer will be inverted before calling the handle_block or handle_each
functions.
/// This is useful, when left is an array and right is scalar. Then rather than
/// copying data from the right to output, we can copy left data to the output
and
/// invert the cond data to fill right values. Filling out with a scalar is
presumed to
/// be more efficient than filling with an array
-template <typename HandleBulk, typename HandleEach, bool invert = false>
-static void RunIfElseLoop(const ArrayData& cond, HandleBulk handle_bulk,
- HandleEach handle_each) {
+///
+/// `HandleBlock` has the signature:
+/// [](int64_t offset, int64_t length){...}
+/// It should copy `length` number of elements from source array to output
array with
+/// `offset` offset in both arrays
+template <typename HandleBlock, bool invert = false>
+void RunIfElseLoop(const ArrayData& cond, const HandleBlock& handle_block) {
int64_t data_offset = 0;
int64_t bit_offset = cond.offset;
const auto* cond_data = cond.buffers[1]->data(); // this is a BoolArray
BitmapWordReader<Word> cond_reader(cond_data, cond.offset, cond.length);
+ constexpr Word pickAll = invert ? 0 : UINT64_MAX;
+ constexpr Word pickNone = ~pickAll;
+
int64_t cnt = cond_reader.words();
while (cnt--) {
Word word = cond_reader.NextWord();
- if (invert) {
- if (word == 0) {
- handle_bulk(data_offset, word_len);
- } else if (word != UINT64_MAX) {
- for (int64_t i = 0; i < word_len; ++i) {
- if (!BitUtil::GetBit(cond_data, bit_offset + i)) {
- handle_each(data_offset + i);
- }
- }
- }
- } else {
- if (word == UINT64_MAX) {
- handle_bulk(data_offset, word_len);
- } else if (word) {
- for (int64_t i = 0; i < word_len; ++i) {
- if (BitUtil::GetBit(cond_data, bit_offset + i)) {
- handle_each(data_offset + i);
- }
+
+ // todo: check if this passes MSVC
Review comment:
Seems like it does.
--
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]
