You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Tim Armstrong (Code Review)" <ge...@cloudera.org> on 2016/10/03 21:34:45 UTC

[Impala-ASF-CR] IMPALA-3637,IMPALA-3636: refactor codegen constant replacement

Hello Jim Apple,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/3843

to look at the new patch set (#5).

Change subject: IMPALA-3637,IMPALA-3636: refactor codegen constant replacement
......................................................................

IMPALA-3637,IMPALA-3636: refactor codegen constant replacement

Merge the disparate codegen constant replacement mechanisms into a
single general-purpose mechanism that can replace function calls,
or function calls with a constant argument, with constants. This
new mechanism should be more easily extensible to handle further
instances of constant replacement.

Use the new infrastructure to replace CpuInfo::IsSupported() calls
in all codegen'd functions using the new infastructure.

This also provides an immediate benefit that some interpreted code paths
that previously called Expr::GetConstant() will be faster, since they
are now just simple inlinable function calls, instead of going through
more levels of runtime indirection.

Testing:
We have good coverage of expression and operator codegen through
end-to-end tests. Update the expr-codegen-test unit test to test the new
mechanism along with the CpuInfo::IsSupported() mechanism.

Change-Id: I6ba029ed8589698eb15dbfb0a20dd2a7ea752635
---
M be/src/codegen/CMakeLists.txt
A be/src/codegen/constant-replacement-test.cc
A be/src/codegen/constant-replacement.cc
A be/src/codegen/constant-replacement.h
M be/src/codegen/llvm-codegen.cc
M be/src/codegen/llvm-codegen.h
M be/src/exec/CMakeLists.txt
A be/src/exec/hash-table-constant-replacer.cc
A be/src/exec/hash-table-constant-replacer.h
M be/src/exec/hash-table.cc
M be/src/exec/hash-table.h
M be/src/exec/partitioned-aggregation-node.cc
M be/src/exec/partitioned-hash-join-builder.cc
M be/src/exec/partitioned-hash-join-node.cc
M be/src/exprs/CMakeLists.txt
M be/src/exprs/aggregate-functions-ir.cc
M be/src/exprs/conditional-functions-ir.cc
M be/src/exprs/decimal-functions-ir.cc
M be/src/exprs/decimal-operators-ir.cc
D be/src/exprs/expr-codegen-test.cc
A be/src/exprs/expr-constant-replacement.cc
A be/src/exprs/expr-constant-replacement.h
M be/src/exprs/expr.cc
M be/src/exprs/expr.h
M be/src/exprs/math-functions-ir.cc
M be/src/exprs/scalar-fn-call.cc
M be/src/runtime/types.h
M be/src/udf/udf-internal.h
M be/src/util/cpu-info.cc
M be/src/util/cpu-info.h
M testdata/workloads/functional-query/queries/QueryTest/exprs.test
31 files changed, 1,216 insertions(+), 795 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/43/3843/5
-- 
To view, visit http://gerrit.cloudera.org:8080/3843
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6ba029ed8589698eb15dbfb0a20dd2a7ea752635
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>