Dan Hecht has posted comments on this change.

Change subject: IMPALA-5192: Don't bake MemPool* into IR
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6657/3/be/src/runtime/tuple.h
File be/src/runtime/tuple.h:

PS3, Line 167: There are two different MaterializeExprs symbols to 
differentiate between
             :   /// these cases when we replace the function calls during 
codegen.
> Please see Tuple::MATERIALIZE_EXPRS_SYMBOL and Tuple::MATERIALIZE_EXPRS_NUL
I see. But why do we need all that in the first place? All of this code gets 
inlined, right? So, it seems that we could just emit the null check against 
pool into the codegn and then let inlining and constant propagation take care 
of optimizing it out of the IR. Anyway, we can look at that as a follow on 
cleanup later.


-- 
To view, visit http://gerrit.cloudera.org:8080/6657
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I09d620e48032351ab9805825a4afb6536bed2302
Gerrit-PatchSet: 3
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: Taras Bobrovytsky <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-HasComments: Yes

Reply via email to