zanmato1984 commented on code in PR #43389:
URL: https://github.com/apache/arrow/pull/43389#discussion_r1718443954
##########
cpp/src/arrow/compute/row/compare_internal_avx2.cc:
##########
@@ -412,13 +408,16 @@ inline uint64_t Compare8_64bit_avx2(const uint8_t*
left_base, const uint8_t* rig
template <bool use_selection>
inline uint64_t Compare8_Binary_avx2(uint32_t length, const uint8_t* left_base,
const uint8_t* right_base, __m256i
irow_left,
- uint32_t irow_left_first, __m256i
offset_right) {
+ uint32_t irow_left_first, __m256i
offset_right_lo,
+ __m256i offset_right_hi) {
uint32_t irow_left_array[8];
- uint32_t offset_right_array[8];
+ RowTableImpl::offset_type offset_right_array[8];
if (use_selection) {
_mm256_storeu_si256(reinterpret_cast<__m256i*>(irow_left_array),
irow_left);
}
- _mm256_storeu_si256(reinterpret_cast<__m256i*>(offset_right_array),
offset_right);
+ _mm256_storeu_si256(reinterpret_cast<__m256i*>(offset_right_array),
offset_right_lo);
Review Comment:
I assume you are suggesting we assert the relationship between the offset
type size and the vector register size so we load the expected number of
elements into the register. And yes, we better do so. I'll add them to where
necessary. Thank you, this is very insightful!
--
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]
