Anonymous Coward #174 has posted comments on this change. Change subject: WIP: flatbuffers-based codegen ......................................................................
Patch Set 2: (4 comments) This is really cool! Really lowers the barrier to codegenning! http://gerrit.cloudera.org:8080/#/c/2903/2/src/kudu/codegen/CMakeLists.txt File src/kudu/codegen/CMakeLists.txt: Line 136: chnage sp: chnage -> change http://gerrit.cloudera.org:8080/#/c/2903/2/src/kudu/codegen/precompiled.cc File src/kudu/codegen/precompiled.cc: Line 111: 0 #if 0 ? Line 142: for (auto dst_col_idx : *fb->default_cols()) { Why no #pragma unroll here? Why #pragma unroll at all? Shouldn't LLVM figure that out on its own? http://gerrit.cloudera.org:8080/#/c/2903/2/src/kudu/codegen/row_projector.cc File src/kudu/codegen/row_projector.cc: Line 286: // TODO: it seems like there's a bug here since we're using the pointer value : // of the read_default in the case of STRINGs, which could get reused. Why would that be a bug? If it gets reused then the pointer is still valid and the codegenned code would be the same - we would want to copy from that pointer value anyway. I admit that's a case worth explicitly documenting. -- To view, visit http://gerrit.cloudera.org:8080/2903 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I583ef1edad379bb13b34362f121263de28c8334c Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon <[email protected]> Gerrit-Reviewer: Anonymous Coward #174 Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
