impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dan Hecht (Code Review)" <ger...@cloudera.org>
Subject [Impala-CR](cdh5-trunk) IMPALA-3674: Lazy materialization of LLVM module bitcode.
Date Tue, 19 Jul 2016 20:51:05 GMT
Dan Hecht has posted comments on this change.

Change subject: IMPALA-3674: Lazy materialization of LLVM module bitcode.
......................................................................


Patch Set 11: Code-Review+2

(4 comments)

http://gerrit.cloudera.org:8080/#/c/3220/11/be/src/codegen/llvm-codegen-test.cc
File be/src/codegen/llvm-codegen-test.cc:

Line 162:   const char* loop_name = "_Z8TestLoopi";
just make these "const string" and then you can get rid of the string() on lines 173/176.


http://gerrit.cloudera.org:8080/#/c/3220/11/be/src/codegen/llvm-codegen.cc
File be/src/codegen/llvm-codegen.cc:

Line 536: }
is this function dead code?


http://gerrit.cloudera.org:8080/#/c/3220/11/be/src/codegen/llvm-codegen.h
File be/src/codegen/llvm-codegen.h:

Line 199:     /// externally defined native function.
thanks, that's a lot clearer.  but then I wonder why can't we just use 'builder != NULL' for
this? i.e. shouldn't we just exclude printing anything that's just a prototype and not an
IR crafted routine?


http://gerrit.cloudera.org:8080/#/c/3220/11/be/src/exprs/scalar-fn-call.cc
File be/src/exprs/scalar-fn-call.cc:

PS11, Line 460: len
shouldn't this say varargs, not "var len"?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6ed7862fc5e86005ecea83fa2ceb489e737d66b2
Gerrit-PatchSet: 11
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Michael Ho <kwho@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Michael Ho <kwho@cloudera.com>
Gerrit-Reviewer: Mostafa Mokhtar <mmokhtar@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message