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

Reply via email to