Michael Ho has posted comments on this change. Change subject: IMPALA-4705, IMPALA-4779, IMPALA-4780: Fix some Expr bugs with codegen ......................................................................
Patch Set 6: (8 comments) http://gerrit.cloudera.org:8080/#/c/5732/5/be/src/codegen/llvm-codegen.cc File be/src/codegen/llvm-codegen.cc: PS5, Line 328: in case they : // are chosen by the linker or referenced by functions in the new module. Note tha > Okay. How about clarifying: Done http://gerrit.cloudera.org:8080/#/c/5732/6/be/src/codegen/llvm-codegen.cc File be/src/codegen/llvm-codegen.cc: Line 172: RETURN_IF_ERROR(LlvmCodeGen::CreateFromMemory(&init_pool, NULL, "init", &init_codegen)); > how about a comment then: Done PS6, Line 195: only > this "only" doesn't make sense, because you also include functions that don Done PS6, Line 195: by > delete double "by" Done PS6, Line 198: refereced > spelling and period not comma at end. Done http://gerrit.cloudera.org:8080/#/c/5732/6/be/src/codegen/llvm-codegen.h File be/src/codegen/llvm-codegen.h: PS6, Line 85: global variables'/functions' > function names Done PS6, Line 146: libbackend > is this referring to libfesupport.so? Done Line 616: /// returns the set of names of all functions referenced by it. > Now that this only stores the call graph, how about just explaining it in t Done -- To view, visit http://gerrit.cloudera.org:8080/5732 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I40fdb035a565ae2f9c9fbf4db48a548653ef7608 Gerrit-PatchSet: 6 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael Ho <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
