Thomas Tauber-Marshall has posted comments on this change. Change subject: IMPALA-3360: Codegen inserting into runtime filters ......................................................................
Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/8029/3/be/src/exec/filter-context.cc File be/src/exec/filter-context.cc: Line 225: Constant* expr_type_arg = codegen->ConstantToGVPtr( > Yeah, I have no idea why it doesn't work. The types definitely line up or i So I've been playing around with this more, and I have a theory. I noticed that it works if I only replace 'GetType' and not 'GetValue', so the problem may actually be with 'GetValue'. I think that the value that's being returned by the function generated by GetCodegendComputePtrFn() is stack allocated (because ToNativePtr() is called on 'result' which then calls CreateEntryBlockAlloca, which allocates stack space). Basically, GetCodegendComputePtrFn() is trying to create a codegend copy of ScalarExprEvaluator::GetValue() but its not currently doing the part where the value is stored in 'result_' so that a pointer to it can be passed back. One solution would be to figure out how to write IRBuilder for storing the value in 'result_' (or else cross compile ScalarExprEvaluator::GetValue() and sub things in), but that's potentially complicated. It may also be worth just going back to the original version of this patch to eliminate the perf cost from having to store the value in 'result_'. Or maybe you had something in mind with your other comment about not needing to create a whole new function that would solve this problem as well. -- To view, visit http://gerrit.cloudera.org:8080/8029 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I79cf23ad92dadaab996a50a2ca07ef9ebe8639bb Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Thomas Tauber-Marshall <tmarsh...@cloudera.com> Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com> Gerrit-Reviewer: Jim Apple <jbapple-imp...@apache.org> Gerrit-Reviewer: Matthew Jacobs <mjac...@apache.org> Gerrit-Reviewer: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Philip Zeyliger <phi...@cloudera.com> Gerrit-Reviewer: Thomas Tauber-Marshall <tmarsh...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-HasComments: Yes
