kou commented on code in PR #48180:
URL: https://github.com/apache/arrow/pull/48180#discussion_r2543957465
##########
cpp/src/arrow/compute/util.cc:
##########
@@ -30,34 +30,40 @@ namespace util {
namespace bit_util {
inline uint64_t SafeLoadUpTo8Bytes(const uint8_t* bytes, int num_bytes) {
- // This will not be correct on big-endian architectures.
-#if !ARROW_LITTLE_ENDIAN
- ARROW_DCHECK(false);
-#endif
ARROW_DCHECK(num_bytes >= 0 && num_bytes <= 8);
if (num_bytes == 8) {
return util::SafeLoad(reinterpret_cast<const uint64_t*>(bytes));
Review Comment:
Does this work on big-endian system?
##########
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:
Why do we need this?
The `SafeLoadUpTo8Bytes()` change adds support for big-endian, right?
--
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]
