Zach Amsden has posted comments on this change. Change subject: Impala ABM / LZCNT support ......................................................................
Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/5821/1/be/src/util/bit-util.h File be/src/util/bit-util.h: Line 98: static inline uint32_t RoundUpToPowerOfTwoNoHw32(uint32_t v) { > we don't do unsigned arithmetic, unless we're dealing with bit vectors (but One of the primary reasons I created this change was to probe exactly that topic. I completely agree that almost all arithmetic should be signed. There are special cases with bit vectors and hash functions that actually require unsigned math, the rotation below is one of them. This function itself could be int32_t without a problem if users are careful about bounds - which they need to be anyway. -- To view, visit http://gerrit.cloudera.org:8080/5821 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9f6a465ab4a9ee4f582847f8e211a779bdede3d2 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Zach Amsden <zams...@cloudera.com> Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com> Gerrit-Reviewer: Marcel Kornacker <mar...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Zach Amsden <zams...@cloudera.com> Gerrit-HasComments: Yes