impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Marcel Kornacker (Code Review)" <ger...@cloudera.org>
Subject [Impala-CR](cdh5-trunk) Specify whether to clone IR function in GetFunction() instead of ReplaceCallSites()
Date Tue, 08 Mar 2016 00:23:48 GMT
Marcel Kornacker has posted comments on this change.

Change subject: Specify whether to clone IR function in GetFunction() instead of ReplaceCallSites()
......................................................................


Patch Set 3:

(2 comments)

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

Line 455: Function* LlvmCodeGen::GetFunction(IRFunction::Type function, bool clone) {
instead of this clone param, why not simply have the callers that need the function cloned
call CloneFunction() excplicitly? that makes it perfectly clear what's happening, and involves
barely any more code (and you can get rid of a whole bunch of ', false);' ).


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

Line 263:   int ReplaceCallSites(llvm::Function* caller, llvm::Function* new_fn,
since new_fn is an in/out param, it should go to the end


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8eabb5c062ee223c5de9df40aacfdc9dcda5ba63
Gerrit-PatchSet: 3
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Skye Wanderman-Milne <skye@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <marcel@cloudera.com>
Gerrit-Reviewer: Michael Ho <kwho@cloudera.com>
Gerrit-Reviewer: Skye Wanderman-Milne <skye@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message