Skye Wanderman-Milne has posted comments on this change.

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


Patch Set 2:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/2464/2/be/CMakeLists.txt
File be/CMakeLists.txt:

Line 137: set(LLVM_OPT_IR_FLAGS "-O1" "-inline" "-inlinehint-threshold=10" 
"-inline-threshold=10")
Do you mean to include O1?


http://gerrit.cloudera.org:8080/#/c/2464/2/be/src/codegen/CMakeLists.txt
File be/src/codegen/CMakeLists.txt:

Line 59: # unnamed instr.  This makes the IR verifiable and more readable.
Update stale comment. Also I'm a little worried that it says we need instnamer 
to make the IR verifiable, do you know what that's about? I'm trying to figure 
it out locally now but maybe you know.


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

Line 131:   int IR_ALWAYS_INLINE level() const { return level_; }
ALWAYS_INLINE?


http://gerrit.cloudera.org:8080/#/c/2464/2/be/src/exec/partitioned-hash-join-node.h
File be/src/exec/partitioned-hash-join-node.h:

Line 439:     BufferedTupleStream* IR_ALWAYS_INLINE build_rows() { return 
build_rows_; }
ALWAYS_INLINE for these?


-- 
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: 2
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: Marcel Kornacker <[email protected]>
Gerrit-Reviewer: Skye Wanderman-Milne <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-HasComments: Yes

Reply via email to