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