WillAyd commented on code in PR #326:
URL: https://github.com/apache/arrow-nanoarrow/pull/326#discussion_r1407063468


##########
src/nanoarrow/buffer_inline.h:
##########
@@ -223,14 +224,11 @@ static inline int64_t _ArrowBytesForBits(int64_t bits) {
 }
 
 static inline void _ArrowBitsUnpackInt8(const uint8_t word, int8_t* out) {
-  out[0] = (word & 0x1) != 0;
-  out[1] = (word & 0x2) != 0;
-  out[2] = (word & 0x4) != 0;
-  out[3] = (word & 0x8) != 0;
-  out[4] = (word & 0x10) != 0;
-  out[5] = (word & 0x20) != 0;
-  out[6] = (word & 0x40) != 0;
-  out[7] = (word & 0x80) != 0;
+  // see https://stackoverflow.com/a/51750902/621736
+  const uint64_t magic = 0x8040201008040201ULL;
+  const uint64_t mask = 0x8080808080808080ULL;
+  const uint64_t tmp = htobe64((magic * word) & mask) >> 7;

Review Comment:
   I don't expect this `htobe64` to be part of the final design. The SO post 
actually suggests using a byte-reversed `magic` entry to get what we need, but 
that did not work in the special-case of every bit in the `word` being set 
(assumedly due to wrap-around). I left a comment on the SO post about that - 
hopefully someone out there has a smarter way of handling this



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

Reply via email to