Michael Ho has posted comments on this change. Change subject: IMPALA-4809: Add Stability for codegen constants ......................................................................
Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/5848/1//COMMIT_MSG Commit Message: PS1, Line 16: overloaded >From the codegen perspective, we only care if a value is compilation time >constant (compilation in the LlvmCodeGen::FinalizeModule()). Not entirely >clear to me the benefit of distinguishing between these various types of >constant values ? I could be missing something. PS1, Line 17: is_constant() I believe this is a concept confined to Expr only. This is a different notion from say other runtime constants such as the constants in a hash table (e.g. stores_duplicates). http://gerrit.cloudera.org:8080/#/c/5848/1/be/src/exprs/expr.h File be/src/exprs/expr.h: PS1, Line 266: RUNTIME_CONSTANT This seems odd to be in Expr class if an example of this class of constants is CPUInfo's flags. -- To view, visit http://gerrit.cloudera.org:8080/5848 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0431bcd84b908bcf130a6d618f57f8f7c5498428 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Zach Amsden <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
