Dan Hecht has posted comments on this change.

Change subject: Reduce size of cross-compiled IR
......................................................................


Patch Set 1:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/2464/1//COMMIT_MSG
Commit Message:

Line 9: instnamer
what did that do and why were we using it?


Line 20: reduced from 5698 to 3883.
did you do any query benchmarks?


http://gerrit.cloudera.org:8080/#/c/2464/1/be/src/common/status.h
File be/src/common/status.h:

Line 101: 
all the ones in this file we always want inlined whether it's IR or not, which 
is why they are marked "inline" already.  llvm is ignoring the "inline"?  Maybe 
we should add an ALWAYS_INLINE.


http://gerrit.cloudera.org:8080/#/c/2464/1/be/src/exec/hash-table.h
File be/src/exec/hash-table.h:

Line 131:   int IR_ALWAYS_INLINE level() const { return level_; }
seems strange that llvm wouldn't already inline this given that it should be a 
shorter instruction than doing the function call.  maybe we can tune llvm 
compile options further to get these to inline without marking them?


-- 
To view, visit http://gerrit.cloudera.org:8080/2464
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iea04ad2e4b365564ee71082657262485d3a85446
Gerrit-PatchSet: 1
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Tim Armstrong <[email protected]>
Gerrit-Reviewer: Alex Behm <[email protected]>
Gerrit-Reviewer: Dan Hecht <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-HasComments: Yes

Reply via email to