Michael Ho has posted comments on this change.

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


Patch Set 8:

(2 comments)

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

Line 742:   Status status = MaterializeFunctionAndCallee(fn);
> Why?  Doesn't impala code assume that any llvm Function object it gets a ho
There are some cases in llvm-codegen-test.cc which used to get the function 
declarations only. The test is now rewritten to also call GetFunction(). Please 
see the changes in the new patch.


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

Line 231:   llvm::Module* module() { return module_; }
> Would it make sense to use Codegen::FnPrototype there?  It would be nice to
It's possible with some rewrite in ScalarFnCall::GetUdf(). Please see the 
changes in the new patch.


-- 
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: 8
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Michael Ho <[email protected]>
Gerrit-Reviewer: Dan Hecht <[email protected]>
Gerrit-Reviewer: Michael Ho <[email protected]>
Gerrit-Reviewer: Mostafa Mokhtar <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-HasComments: Yes

Reply via email to