github-actions[bot] commented on code in PR #65972:
URL: https://github.com/apache/doris/pull/65972#discussion_r3638698421


##########
be/src/util/simd/parquet_kernels.cpp:
##########
@@ -0,0 +1,637 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "util/simd/parquet_kernels.h"
+
+#include <array>
+#include <cmath>
+#include <cstring>
+#include <type_traits>
+
+#if defined(__x86_64__) && (defined(__GNUC__) || defined(__clang__))
+#include <immintrin.h>
+#define DORIS_PARQUET_X86_SIMD
+#endif
+
+namespace doris::parquet_simd {
+namespace {
+
+bool has_avx2() {
+#ifdef DORIS_PARQUET_X86_SIMD
+    return __builtin_cpu_supports("avx2");
+#else
+    return false;
+#endif
+}
+
+void byte_stream_split_decode_scalar(const uint8_t* src, size_t width, size_t 
offset,
+                                     size_t num_values, size_t stride, 
uint8_t* dest) {
+    for (size_t row = 0; row < num_values; ++row) {
+        for (size_t byte = 0; byte < width; ++byte) {
+            dest[row * width + byte] = src[byte * stride + offset + row];
+        }
+    }
+}
+
+#ifdef DORIS_PARQUET_X86_SIMD
+template <size_t WIDTH>
+__attribute__((target("avx2"))) void byte_stream_split_decode_avx2(const 
uint8_t* src,
+                                                                   size_t 
offset, size_t num_values,
+                                                                   size_t 
stride, uint8_t* dest) {
+    static_assert(WIDTH == 4 || WIDTH == 8);
+    constexpr size_t STEPS = WIDTH == 8 ? 3 : 2;
+    constexpr size_t LANES = sizeof(__m256i);
+    const size_t blocks = num_values / LANES;
+    __m256i stage[STEPS + 1][WIDTH];
+    __m256i result[WIDTH];
+
+    for (size_t block = 0; block < blocks; ++block) {
+        for (size_t stream = 0; stream < WIDTH; ++stream) {
+            stage[0][stream] = _mm256_loadu_si256(reinterpret_cast<const 
__m256i*>(
+                    src + stream * stride + offset + block * LANES));
+        }
+        for (size_t step = 0; step < STEPS; ++step) {
+            // AVX2 unpack instructions operate independently in each 128-bit 
half. Keep the
+            // byte-transpose hierarchy lane-local, then stitch the halves 
only after the last
+            // unpack so every output vector contains contiguous decoded rows.
+            for (size_t pair = 0; pair < WIDTH / 2; ++pair) {
+                stage[step + 1][pair * 2] =
+                        _mm256_unpacklo_epi8(stage[step][pair], 
stage[step][WIDTH / 2 + pair]);
+                stage[step + 1][pair * 2 + 1] =
+                        _mm256_unpackhi_epi8(stage[step][pair], 
stage[step][WIDTH / 2 + pair]);
+            }
+        }
+        for (size_t pair = 0; pair < WIDTH / 2; ++pair) {
+            result[pair] = _mm256_permute2x128_si256(stage[STEPS][pair * 2],
+                                                     stage[STEPS][pair * 2 + 
1], 0x20);
+            result[WIDTH / 2 + pair] = 
_mm256_permute2x128_si256(stage[STEPS][pair * 2],
+                                                                 
stage[STEPS][pair * 2 + 1], 0x31);
+        }
+        for (size_t lane = 0; lane < WIDTH; ++lane) {
+            _mm256_storeu_si256(reinterpret_cast<__m256i*>(dest + (block * 
WIDTH + lane) * LANES),
+                                result[lane]);
+        }
+    }
+    const size_t processed = blocks * LANES;
+    byte_stream_split_decode_scalar(src, WIDTH, offset + processed, num_values 
- processed, stride,
+                                    dest + processed * WIDTH);
+}
+
+__attribute__((target("avx2"))) void delta_decode_int32_avx2(int32_t* values, 
size_t count,
+                                                             int32_t min_delta,
+                                                             int32_t* 
last_value) {
+    const __m256i min_delta_vec = _mm256_set1_epi32(min_delta);
+    const size_t vector_count = count / 8 * 8;
+    for (size_t row = 0; row < vector_count; row += 8) {
+        __m256i value = _mm256_loadu_si256(reinterpret_cast<const 
__m256i*>(values + row));
+        value = _mm256_add_epi32(value, min_delta_vec);
+        value = _mm256_add_epi32(value, _mm256_slli_si256(value, 4));
+        value = _mm256_add_epi32(value, _mm256_slli_si256(value, 8));
+        _mm256_storeu_si256(reinterpret_cast<__m256i*>(values + row), value);
+    }
+    __m128i carry = _mm_set1_epi32(*last_value);
+    for (size_t row = 0; row < vector_count; row += 4) {
+        __m128i value = _mm_loadu_si128(reinterpret_cast<const 
__m128i*>(values + row));
+        value = _mm_add_epi32(value, carry);
+        _mm_storeu_si128(reinterpret_cast<__m128i*>(values + row), value);
+        carry = _mm_shuffle_epi32(value, _MM_SHUFFLE(3, 3, 3, 3));
+    }
+    if (vector_count != 0) {
+        *last_value = _mm_cvtsi128_si32(carry);
+    }
+    using Unsigned = uint32_t;
+    for (size_t row = vector_count; row < count; ++row) {
+        values[row] = static_cast<int32_t>(static_cast<Unsigned>(values[row]) +
+                                           static_cast<Unsigned>(min_delta) +
+                                           static_cast<Unsigned>(*last_value));
+        *last_value = values[row];
+    }
+}
+
+__attribute__((target("avx2"))) void delta_decode_int64_avx2(int64_t* values, 
size_t count,
+                                                             int64_t min_delta,
+                                                             int64_t* 
last_value) {
+    const __m256i min_delta_vec = _mm256_set1_epi64x(min_delta);
+    const size_t vector_count = count / 4 * 4;
+    for (size_t row = 0; row < vector_count; row += 4) {
+        __m256i value = _mm256_loadu_si256(reinterpret_cast<const 
__m256i*>(values + row));
+        value = _mm256_add_epi64(value, min_delta_vec);
+        value = _mm256_add_epi64(value, _mm256_slli_si256(value, 8));
+        _mm256_storeu_si256(reinterpret_cast<__m256i*>(values + row), value);
+    }
+    __m128i carry = _mm_set1_epi64x(*last_value);
+    for (size_t row = 0; row < vector_count; row += 2) {
+        __m128i value = _mm_loadu_si128(reinterpret_cast<const 
__m128i*>(values + row));
+        value = _mm_add_epi64(value, carry);
+        _mm_storeu_si128(reinterpret_cast<__m128i*>(values + row), value);
+        carry = _mm_unpackhi_epi64(value, value);
+    }
+    if (vector_count != 0) {
+        *last_value = _mm_cvtsi128_si64(carry);
+    }
+    using Unsigned = uint64_t;
+    for (size_t row = vector_count; row < count; ++row) {
+        values[row] = static_cast<int64_t>(static_cast<Unsigned>(values[row]) +
+                                           static_cast<Unsigned>(min_delta) +
+                                           static_cast<Unsigned>(*last_value));
+        *last_value = values[row];
+    }
+}
+
+__attribute__((target("avx2"))) void dictionary_gather_avx2(const uint8_t* 
dictionary,
+                                                            const uint32_t* 
indices, size_t count,
+                                                            size_t 
value_width, uint8_t* dest) {
+    size_t row = 0;
+    if (value_width == 4) {
+        for (; row + 8 <= count; row += 8) {
+            const __m256i ids = _mm256_loadu_si256(reinterpret_cast<const 
__m256i*>(indices + row));
+            const __m256i gathered =
+                    _mm256_i32gather_epi32(reinterpret_cast<const 
int*>(dictionary), ids, 4);
+            _mm256_storeu_si256(reinterpret_cast<__m256i*>(dest + row * 4), 
gathered);
+        }
+    } else {
+        for (; row + 4 <= count; row += 4) {
+            const __m128i ids = _mm_loadu_si128(reinterpret_cast<const 
__m128i*>(indices + row));
+            const __m256i gathered =
+                    _mm256_i32gather_epi64(reinterpret_cast<const long 
long*>(dictionary), ids, 8);
+            _mm256_storeu_si256(reinterpret_cast<__m256i*>(dest + row * 8), 
gathered);
+        }
+    }
+    for (; row < count; ++row) {
+        memcpy(dest + row * value_width, dictionary + indices[row] * 
value_width, value_width);
+    }
+}
+
+template <size_t LANES>
+constexpr auto make_expand_permute_lut() {
+    std::array<std::array<int32_t, 8>, 1U << LANES> lut {};
+    for (size_t mask = 0; mask < lut.size(); ++mask) {
+        int32_t source = 0;
+        for (size_t lane = 0; lane < LANES; ++lane) {
+            const int32_t value = (mask & (1U << lane)) != 0 ? source++ : 0;
+            if constexpr (LANES == 8) {
+                lut[mask][lane] = value;
+            } else {
+                lut[mask][lane * 2] = value * 2;
+                lut[mask][lane * 2 + 1] = value * 2 + 1;
+            }
+        }
+    }
+    return lut;
+}
+
+constexpr auto EXPAND_PERMUTE_32 = make_expand_permute_lut<8>();
+constexpr auto EXPAND_PERMUTE_64 = make_expand_permute_lut<4>();
+
+__attribute__((target("avx2"))) void expand_nullable_avx2(uint8_t* bytes, 
size_t compact_count,
+                                                          const uint8_t* 
nulls, size_t output_count,
+                                                          size_t value_width) {
+    size_t source = compact_count;
+    size_t output = output_count;
+    if (value_width == 4) {
+        auto* values = reinterpret_cast<int32_t*>(bytes);
+        while (output >= 8) {
+            const size_t start = output - 8;
+            uint32_t valid_mask = 0;
+            for (size_t lane = 0; lane < 8; ++lane) {
+                valid_mask |= static_cast<uint32_t>(nulls[start + lane] == 0) 
<< lane;
+            }
+            const size_t valid = std::popcount(valid_mask);

Review Comment:
   [P1] Include the header that declares `std::popcount`
   
   This translation unit uses `std::popcount` here and at line 237 but never 
includes `<bit>`. GCC builds explicitly default to `ENABLE_PCH=OFF`, so a 
transitive declaration from Clang's PCH cannot make this a supported build 
contract. Please add a direct `#include <bit>` and validate the no-PCH 
configuration.



##########
be/benchmark/parquet/benchmark_parquet_decoder.hpp:
##########
@@ -459,7 +459,7 @@ inline void run_decoder(benchmark::State& state, 
DecoderScenario scenario, int s
 
 inline bool register_decoder_benchmarks() {
     for (const auto& scenario : decoder_scenarios()) {
-        for (const int selectivity : {1, 10, 50, 100}) {
+        for (const int selectivity : {0, 1, 10, 50, 90, 100}) {

Review Comment:
   [P2] Make the new decoder boundary cases verifiable
   
   This loop now registers `19 * 6 * 2 = 228` decoder cases, plus 80 new kernel 
cases, while the mandatory `be/benchmark/parquet/AGENTS.md` still tells 
maintainers to expect 152 and omits `ParquetKernel`. Also, `run_decoder` only 
resets each sink's `consumed` field; it never checks that field against 
`plan.selected_rows` or validates representative decoded values outside timing. 
Consequently `sel_0` can emit rows, or `sel_90` can return the wrong 
selection/payload, and still publish a successful benchmark. Please update the 
current count guidance and add an untimed count/checksum oracle before 
measuring these new cases.



##########
be/src/util/simd/parquet_kernels.cpp:
##########
@@ -0,0 +1,637 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "util/simd/parquet_kernels.h"
+
+#include <array>
+#include <cmath>
+#include <cstring>
+#include <type_traits>
+
+#if defined(__x86_64__) && (defined(__GNUC__) || defined(__clang__))
+#include <immintrin.h>
+#define DORIS_PARQUET_X86_SIMD
+#endif
+
+namespace doris::parquet_simd {
+namespace {
+
+bool has_avx2() {

Review Comment:
   [P1] Guard this helper on non-x86 builds
   
   When `DORIS_PARQUET_X86_SIMD` is absent, every call to `has_avx2()` is 
compiled out, but this anonymous-namespace definition remains. Doris builds ARM 
with `-Wall -Wextra -Werror` (and Clang enables `-Wunused`), so the unused 
internal function is a build error before the scalar fallback can run. Please 
put the helper itself under the x86 guard (or otherwise remove the non-x86 
definition) and cover an arm64/AArch64 compile.



##########
be/src/util/simd/parquet_kernels.cpp:
##########
@@ -0,0 +1,637 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "util/simd/parquet_kernels.h"
+
+#include <array>
+#include <cmath>
+#include <cstring>
+#include <type_traits>
+
+#if defined(__x86_64__) && (defined(__GNUC__) || defined(__clang__))
+#include <immintrin.h>
+#define DORIS_PARQUET_X86_SIMD
+#endif
+
+namespace doris::parquet_simd {
+namespace {
+
+bool has_avx2() {
+#ifdef DORIS_PARQUET_X86_SIMD
+    return __builtin_cpu_supports("avx2");
+#else
+    return false;
+#endif
+}
+
+void byte_stream_split_decode_scalar(const uint8_t* src, size_t width, size_t 
offset,
+                                     size_t num_values, size_t stride, 
uint8_t* dest) {
+    for (size_t row = 0; row < num_values; ++row) {
+        for (size_t byte = 0; byte < width; ++byte) {
+            dest[row * width + byte] = src[byte * stride + offset + row];
+        }
+    }
+}
+
+#ifdef DORIS_PARQUET_X86_SIMD
+template <size_t WIDTH>
+__attribute__((target("avx2"))) void byte_stream_split_decode_avx2(const 
uint8_t* src,
+                                                                   size_t 
offset, size_t num_values,
+                                                                   size_t 
stride, uint8_t* dest) {
+    static_assert(WIDTH == 4 || WIDTH == 8);
+    constexpr size_t STEPS = WIDTH == 8 ? 3 : 2;
+    constexpr size_t LANES = sizeof(__m256i);
+    const size_t blocks = num_values / LANES;
+    __m256i stage[STEPS + 1][WIDTH];
+    __m256i result[WIDTH];
+
+    for (size_t block = 0; block < blocks; ++block) {
+        for (size_t stream = 0; stream < WIDTH; ++stream) {
+            stage[0][stream] = _mm256_loadu_si256(reinterpret_cast<const 
__m256i*>(
+                    src + stream * stride + offset + block * LANES));
+        }
+        for (size_t step = 0; step < STEPS; ++step) {
+            // AVX2 unpack instructions operate independently in each 128-bit 
half. Keep the
+            // byte-transpose hierarchy lane-local, then stitch the halves 
only after the last
+            // unpack so every output vector contains contiguous decoded rows.
+            for (size_t pair = 0; pair < WIDTH / 2; ++pair) {
+                stage[step + 1][pair * 2] =
+                        _mm256_unpacklo_epi8(stage[step][pair], 
stage[step][WIDTH / 2 + pair]);
+                stage[step + 1][pair * 2 + 1] =
+                        _mm256_unpackhi_epi8(stage[step][pair], 
stage[step][WIDTH / 2 + pair]);
+            }
+        }
+        for (size_t pair = 0; pair < WIDTH / 2; ++pair) {
+            result[pair] = _mm256_permute2x128_si256(stage[STEPS][pair * 2],
+                                                     stage[STEPS][pair * 2 + 
1], 0x20);
+            result[WIDTH / 2 + pair] = 
_mm256_permute2x128_si256(stage[STEPS][pair * 2],
+                                                                 
stage[STEPS][pair * 2 + 1], 0x31);
+        }
+        for (size_t lane = 0; lane < WIDTH; ++lane) {
+            _mm256_storeu_si256(reinterpret_cast<__m256i*>(dest + (block * 
WIDTH + lane) * LANES),
+                                result[lane]);
+        }
+    }
+    const size_t processed = blocks * LANES;
+    byte_stream_split_decode_scalar(src, WIDTH, offset + processed, num_values 
- processed, stride,
+                                    dest + processed * WIDTH);
+}
+
+__attribute__((target("avx2"))) void delta_decode_int32_avx2(int32_t* values, 
size_t count,
+                                                             int32_t min_delta,
+                                                             int32_t* 
last_value) {
+    const __m256i min_delta_vec = _mm256_set1_epi32(min_delta);
+    const size_t vector_count = count / 8 * 8;
+    for (size_t row = 0; row < vector_count; row += 8) {
+        __m256i value = _mm256_loadu_si256(reinterpret_cast<const 
__m256i*>(values + row));
+        value = _mm256_add_epi32(value, min_delta_vec);
+        value = _mm256_add_epi32(value, _mm256_slli_si256(value, 4));
+        value = _mm256_add_epi32(value, _mm256_slli_si256(value, 8));
+        _mm256_storeu_si256(reinterpret_cast<__m256i*>(values + row), value);
+    }
+    __m128i carry = _mm_set1_epi32(*last_value);
+    for (size_t row = 0; row < vector_count; row += 4) {
+        __m128i value = _mm_loadu_si128(reinterpret_cast<const 
__m128i*>(values + row));
+        value = _mm_add_epi32(value, carry);
+        _mm_storeu_si128(reinterpret_cast<__m128i*>(values + row), value);
+        carry = _mm_shuffle_epi32(value, _MM_SHUFFLE(3, 3, 3, 3));
+    }
+    if (vector_count != 0) {
+        *last_value = _mm_cvtsi128_si32(carry);
+    }
+    using Unsigned = uint32_t;
+    for (size_t row = vector_count; row < count; ++row) {
+        values[row] = static_cast<int32_t>(static_cast<Unsigned>(values[row]) +
+                                           static_cast<Unsigned>(min_delta) +
+                                           static_cast<Unsigned>(*last_value));
+        *last_value = values[row];
+    }
+}
+
+__attribute__((target("avx2"))) void delta_decode_int64_avx2(int64_t* values, 
size_t count,
+                                                             int64_t min_delta,
+                                                             int64_t* 
last_value) {
+    const __m256i min_delta_vec = _mm256_set1_epi64x(min_delta);
+    const size_t vector_count = count / 4 * 4;
+    for (size_t row = 0; row < vector_count; row += 4) {
+        __m256i value = _mm256_loadu_si256(reinterpret_cast<const 
__m256i*>(values + row));
+        value = _mm256_add_epi64(value, min_delta_vec);
+        value = _mm256_add_epi64(value, _mm256_slli_si256(value, 8));
+        _mm256_storeu_si256(reinterpret_cast<__m256i*>(values + row), value);
+    }
+    __m128i carry = _mm_set1_epi64x(*last_value);
+    for (size_t row = 0; row < vector_count; row += 2) {
+        __m128i value = _mm_loadu_si128(reinterpret_cast<const 
__m128i*>(values + row));
+        value = _mm_add_epi64(value, carry);
+        _mm_storeu_si128(reinterpret_cast<__m128i*>(values + row), value);
+        carry = _mm_unpackhi_epi64(value, value);
+    }
+    if (vector_count != 0) {
+        *last_value = _mm_cvtsi128_si64(carry);
+    }
+    using Unsigned = uint64_t;
+    for (size_t row = vector_count; row < count; ++row) {
+        values[row] = static_cast<int64_t>(static_cast<Unsigned>(values[row]) +
+                                           static_cast<Unsigned>(min_delta) +
+                                           static_cast<Unsigned>(*last_value));
+        *last_value = values[row];
+    }
+}
+
+__attribute__((target("avx2"))) void dictionary_gather_avx2(const uint8_t* 
dictionary,
+                                                            const uint32_t* 
indices, size_t count,
+                                                            size_t 
value_width, uint8_t* dest) {
+    size_t row = 0;
+    if (value_width == 4) {
+        for (; row + 8 <= count; row += 8) {
+            const __m256i ids = _mm256_loadu_si256(reinterpret_cast<const 
__m256i*>(indices + row));
+            const __m256i gathered =
+                    _mm256_i32gather_epi32(reinterpret_cast<const 
int*>(dictionary), ids, 4);
+            _mm256_storeu_si256(reinterpret_cast<__m256i*>(dest + row * 4), 
gathered);
+        }
+    } else {
+        for (; row + 4 <= count; row += 4) {
+            const __m128i ids = _mm_loadu_si128(reinterpret_cast<const 
__m128i*>(indices + row));
+            const __m256i gathered =
+                    _mm256_i32gather_epi64(reinterpret_cast<const long 
long*>(dictionary), ids, 8);
+            _mm256_storeu_si256(reinterpret_cast<__m256i*>(dest + row * 8), 
gathered);
+        }
+    }
+    for (; row < count; ++row) {
+        memcpy(dest + row * value_width, dictionary + indices[row] * 
value_width, value_width);
+    }
+}
+
+template <size_t LANES>
+constexpr auto make_expand_permute_lut() {
+    std::array<std::array<int32_t, 8>, 1U << LANES> lut {};
+    for (size_t mask = 0; mask < lut.size(); ++mask) {
+        int32_t source = 0;
+        for (size_t lane = 0; lane < LANES; ++lane) {
+            const int32_t value = (mask & (1U << lane)) != 0 ? source++ : 0;
+            if constexpr (LANES == 8) {
+                lut[mask][lane] = value;
+            } else {
+                lut[mask][lane * 2] = value * 2;
+                lut[mask][lane * 2 + 1] = value * 2 + 1;
+            }
+        }
+    }
+    return lut;
+}
+
+constexpr auto EXPAND_PERMUTE_32 = make_expand_permute_lut<8>();
+constexpr auto EXPAND_PERMUTE_64 = make_expand_permute_lut<4>();
+
+__attribute__((target("avx2"))) void expand_nullable_avx2(uint8_t* bytes, 
size_t compact_count,
+                                                          const uint8_t* 
nulls, size_t output_count,
+                                                          size_t value_width) {
+    size_t source = compact_count;
+    size_t output = output_count;
+    if (value_width == 4) {
+        auto* values = reinterpret_cast<int32_t*>(bytes);
+        while (output >= 8) {
+            const size_t start = output - 8;
+            uint32_t valid_mask = 0;
+            for (size_t lane = 0; lane < 8; ++lane) {
+                valid_mask |= static_cast<uint32_t>(nulls[start + lane] == 0) 
<< lane;
+            }
+            const size_t valid = std::popcount(valid_mask);
+            source -= valid;
+            const __m256i load_mask = 
_mm256_cmpgt_epi32(_mm256_set1_epi32(static_cast<int>(valid)),
+                                                         _mm256_setr_epi32(0, 
1, 2, 3, 4, 5, 6, 7));
+            const __m256i compact = _mm256_maskload_epi32(values + source, 
load_mask);
+            const __m256i permute = _mm256_loadu_si256(
+                    reinterpret_cast<const 
__m256i*>(EXPAND_PERMUTE_32[valid_mask].data()));
+            __m256i expanded = _mm256_permutevar8x32_epi32(compact, permute);
+            const __m256i valid_lanes = _mm256_setr_epi32(
+                    -(valid_mask & 1U), -((valid_mask >> 1) & 1U), 
-((valid_mask >> 2) & 1U),
+                    -((valid_mask >> 3) & 1U), -((valid_mask >> 4) & 1U), 
-((valid_mask >> 5) & 1U),
+                    -((valid_mask >> 6) & 1U), -((valid_mask >> 7) & 1U));
+            expanded = _mm256_and_si256(expanded, valid_lanes);
+            _mm256_storeu_si256(reinterpret_cast<__m256i*>(values + start), 
expanded);
+            output = start;
+        }
+    } else {
+        auto* values = reinterpret_cast<int64_t*>(bytes);
+        while (output >= 4) {
+            const size_t start = output - 4;
+            uint32_t valid_mask = 0;
+            for (size_t lane = 0; lane < 4; ++lane) {
+                valid_mask |= static_cast<uint32_t>(nulls[start + lane] == 0) 
<< lane;
+            }
+            const size_t valid = std::popcount(valid_mask);
+            source -= valid;
+            const __m256i load_mask =
+                    _mm256_setr_epi64x(valid > 0 ? -1LL : 0, valid > 1 ? -1LL 
: 0,
+                                       valid > 2 ? -1LL : 0, valid > 3 ? -1LL 
: 0);
+            const __m256i compact = _mm256_maskload_epi64(
+                    reinterpret_cast<const long long*>(values + source), 
load_mask);
+            const __m256i permute = _mm256_loadu_si256(
+                    reinterpret_cast<const 
__m256i*>(EXPAND_PERMUTE_64[valid_mask].data()));
+            __m256i expanded = _mm256_permutevar8x32_epi32(compact, permute);
+            const __m256i valid_lanes = _mm256_setr_epi64x(
+                    (valid_mask & 1U) != 0 ? -1LL : 0, (valid_mask & 2U) != 0 
? -1LL : 0,
+                    (valid_mask & 4U) != 0 ? -1LL : 0, (valid_mask & 8U) != 0 
? -1LL : 0);
+            expanded = _mm256_and_si256(expanded, valid_lanes);
+            _mm256_storeu_si256(reinterpret_cast<__m256i*>(values + start), 
expanded);
+            output = start;
+        }
+    }
+    while (output > 0) {
+        --output;
+        if (nulls[output] != 0) {
+            memset(bytes + output * value_width, 0, value_width);
+        } else {
+            --source;
+            memmove(bytes + output * value_width, bytes + source * 
value_width, value_width);
+        }
+    }
+}
+
+template <typename Vec>
+Vec combine_comparison(Vec equal, Vec greater, Vec less, RawComparisonOp op) {

Review Comment:
   [P1] Keep every AVX2 intrinsic helper in an AVX2 target scope
   
   `combine_comparison` (including its generated lambda call operators) 
directly invokes `_mm256_*` intrinsics and returns 256-bit vectors, but it has 
no `target("avx2")` scope. The attribute on `raw_compare_*_avx2` does not 
propagate to a separately instantiated helper, so the supported `USE_AVX2=0` 
x86 build encounters AVX ABI/always-inline target-mismatch diagnostics before 
runtime dispatch. Please inline this selection into the annotated functions or 
place the complete helper implementation in an AVX2 target scope, then 
compile-check `USE_AVX2=0`.



-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to