Michael Ho has posted comments on this change. Change subject: IMPALA-3637,IMPALA-3636: refactor codegen constant replacement ......................................................................
Patch Set 6: (2 comments) http://gerrit.cloudera.org:8080/#/c/3401/7/be/src/codegen/constant-replacement.cc File be/src/codegen/constant-replacement.cc: PS7, Line 31: gen, nit: long line http://gerrit.cloudera.org:8080/#/c/3401/6/be/src/codegen/llvm-codegen.cc File be/src/codegen/llvm-codegen.cc: PS6, Line 651: symbol_map.find(callee->getName()) != symbol_map.end()) > I'd rather expose that information because it allows us to implement the pa I am not sure I understand this comment here. This function as it's written now only handles one replacer so it's unclear what it means to "call into every replacer" as there is only one. With the way it's implemented now, don't you have to consult the maps of all replacers if there are more than one of them ? So, how is it more efficient ? -- To view, visit http://gerrit.cloudera.org:8080/3401 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6ba029ed8589698eb15dbfb0a20dd2a7ea752635 Gerrit-PatchSet: 6 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Tim Armstrong <[email protected]> Gerrit-Reviewer: Jim Apple <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
