Vishwanatha-HD commented on code in PR #48180:
URL: https://github.com/apache/arrow/pull/48180#discussion_r2601769494
##########
cpp/src/arrow/compute/util.cc:
##########
@@ -118,7 +124,22 @@ void bits_to_indexes_internal(int64_t hardware_flags,
const int num_bits,
// Optionally process the last partial word with masking out bits outside
range
if (tail) {
const uint8_t* bits_tail = bits + (num_bits - tail) / 8;
+#if ARROW_LITTLE_ENDIAN
uint64_t word = SafeLoadUpTo8Bytes(bits_tail, (tail + 7) / 8);
+#else
+ int tail_bytes = (tail + 7) / 8;
+ uint64_t word;
+ if (tail_bytes == 8) {
+ word = util::SafeLoad(reinterpret_cast<const uint64_t*>(bits_tail));
+ } else {
+ // For bit manipulation, always load into least significant bits
+ // to ensure compatibility with CountTrailingZeros on Big-endian systems
+ word = 0;
+ for (int i = 0; i < tail_bytes; ++i) {
+ word |= static_cast<uint64_t>(bits_tail[i]) << (8 * i);
+ }
+ }
+#endif
Review Comment:
@zanmato1984.. Please get my latest code changes to util.cc file..
```
inline uint64_t SafeLoadUpTo8Bytes(const uint8_t* bytes, int num_bytes) {
ARROW_DCHECK(num_bytes >= 0 && num_bytes <= 8);
if (num_bytes == 8) {
return util::SafeLoad(reinterpret_cast<const uint64_t*>(bytes));
} else {
uint64_t word = 0;
#if ARROW_LITTLE_ENDIAN
for (int i = 0; i < num_bytes; ++i) {
word |= static_cast<uint64_t>(bytes[i]) << (8 * i);
}
#else
// Big-endian: most significant byte first
for (int i = 0; i < num_bytes; ++i) {
word |= static_cast<uint64_t>(bytes[i]) << (8 * (num_bytes - 1 - i));
}
#endif
return word;
}
}
```
In the bits_to_indexes_internal() function >>>>>>>>>>
```
// Optionally process the last partial word with masking out bits outside
range
if (tail) {
const uint8_t* bits_tail = bits + (num_bits - tail) / 8;
uint64_t word = SafeLoadUpTo8Bytes(bits_tail, (tail + 7) / 8);
#if !ARROW_LITTLE_ENDIAN
word = ::arrow::bit_util::ByteSwap(word);
#endif
if (bit_to_search == 0) {
word = ~word;
}
word &= ~0ULL >> (64 - tail);
if (filter_input_indexes) {
bits_filter_indexes_helper(word, input_indexes + num_bits - tail,
num_indexes,
indexes);
} else {
bits_to_indexes_helper(word, num_bits - tail + base_index,
num_indexes, indexes);
}
}
}
```
In the bytes_to_bits() function >>>>>>>>>>>>
```
if (tail) {
uint64_t bytes_next;
bytes_next = SafeLoadUpTo8Bytes(bytes + num_bits - tail, tail);
#if !ARROW_LITTLE_ENDIAN
bytes_next = ::arrow::bit_util::ByteSwap(bytes_next);
#endif
bytes_next &= 0x0101010101010101ULL;
bytes_next |= (bytes_next >> 7); // Pairs of adjacent output bits in
individual bytes
bytes_next |= (bytes_next >> 14); // 4 adjacent output bits in
individual bytes
bytes_next |= (bytes_next >> 28); // All 8 output bits in the lowest
byte
bits[num_bits / 8] = static_cast<uint8_t>(bytes_next & 0xff);
}
```
And, yes.. This looks to be a new testcase failure with the above mentioned
changes..
--
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]
