You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by st...@apache.org on 2020/08/12 14:34:13 UTC
[impala] branch master updated (ac63e19 -> 5149ea7)
This is an automated email from the ASF dual-hosted git repository.
stakiar pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git.
from ac63e19 IMPALA-10043: Keep more logs when using EE_TEST_SHARDS
new 7a02e37 IMPALA-10061 Fix bugs of IMPALA-9645
new 5149ea7 IMPALA-10054: Fix flakiness in test_multiple_sort_run_bytes_limits
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
be/src/codegen/llvm-codegen.cc | 39 ++++++++++++++++++++++++++++++++++++---
be/src/exprs/scalar-fn-call.cc | 4 +++-
be/src/gutil/sysinfo.cc | 8 ++++++++
be/src/gutil/sysinfo.h | 3 +++
tests/query_test/test_sort.py | 10 +++++-----
5 files changed, 55 insertions(+), 9 deletions(-)
[impala] 01/02: IMPALA-10061 Fix bugs of IMPALA-9645
Posted by st...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
stakiar pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit 7a02e370be2aada393d6ab062e1c959c712abb9f
Author: zhaorenhai <zh...@hotmail.com>
AuthorDate: Sun Apr 12 12:05:52 2020 +0000
IMPALA-10061 Fix bugs of IMPALA-9645
Fix one bug of IMPALA-9645.
And fix issue when return type is decimal,
codegen code lack a 'StructRet' attribute,
this is not a issue on x86, but on aarch64,
the "StructRet" attribute is necessary.
And fix the hash function on aarch64.
Change-Id: I219588992715b7d5c69cd7c0d48ff4d90b980338
Reviewed-on: http://gerrit.cloudera.org:8080/16306
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
be/src/codegen/llvm-codegen.cc | 39 ++++++++++++++++++++++++++++++++++++---
be/src/exprs/scalar-fn-call.cc | 4 +++-
be/src/gutil/sysinfo.cc | 8 ++++++++
be/src/gutil/sysinfo.h | 3 +++
4 files changed, 50 insertions(+), 4 deletions(-)
diff --git a/be/src/codegen/llvm-codegen.cc b/be/src/codegen/llvm-codegen.cc
index 1834f8e..e36c22e 100644
--- a/be/src/codegen/llvm-codegen.cc
+++ b/be/src/codegen/llvm-codegen.cc
@@ -75,6 +75,7 @@
#include "runtime/runtime-state.h"
#include "runtime/string-value.h"
#include "runtime/timestamp-value.h"
+#include "gutil/sysinfo.h"
#include "util/cpu-info.h"
#include "util/debug-util.h"
#include "util/hdfs-util.h"
@@ -892,7 +893,12 @@ Status LlvmCodeGen::LoadFunction(const TFunction& fn, const string& symbol,
// declaration, not a definition, since we do not create any basic blocks or
// instructions in it.
*llvm_fn = prototype.GeneratePrototype(nullptr, nullptr);
-
+#ifdef __aarch64__
+ if (is_decimal) {
+ // Mark first argument as sret
+ (*llvm_fn)->addAttribute(1, llvm::Attribute::StructRet);
+ }
+#endif
// Associate the dynamically loaded function pointer with the Function* we defined.
// This tells LLVM where the compiled function definition is located in memory.
execution_engine_->addGlobalMapping(*llvm_fn, fn_ptr);
@@ -1483,10 +1489,17 @@ Status LlvmCodeGen::LoadIntrinsics() {
llvm::Intrinsic::ID id;
const char* error;
} non_overloaded_intrinsics[] = {
+#ifdef __aarch64__
+ {llvm::Intrinsic::aarch64_crc32cb, "aarch64 crc32_u8"},
+ {llvm::Intrinsic::aarch64_crc32ch, "aarch64 crc32_u16"},
+ {llvm::Intrinsic::aarch64_crc32cw, "aarch64 crc32_u32"},
+ {llvm::Intrinsic::aarch64_crc32cx, "aarch64 crc32_u64"},
+#else
{llvm::Intrinsic::x86_sse42_crc32_32_8, "sse4.2 crc32_u8"},
{llvm::Intrinsic::x86_sse42_crc32_32_16, "sse4.2 crc32_u16"},
{llvm::Intrinsic::x86_sse42_crc32_32_32, "sse4.2 crc32_u32"},
{llvm::Intrinsic::x86_sse42_crc32_64_64, "sse4.2 crc32_u64"},
+#endif
};
const int num_intrinsics =
sizeof(non_overloaded_intrinsics) / sizeof(non_overloaded_intrinsics[0]);
@@ -1597,7 +1610,7 @@ void LlvmCodeGen::ClearHashFns() {
// ret i32 %12
// }
llvm::Function* LlvmCodeGen::GetHashFunction(int num_bytes) {
- if (IsCPUFeatureEnabled(CpuInfo::SSE4_2)) {
+ if (base::IsAarch64() || IsCPUFeatureEnabled(CpuInfo::SSE4_2)) {
if (num_bytes == -1) {
// -1 indicates variable length, just return the generic loop based
// hash fn.
@@ -1622,25 +1635,39 @@ llvm::Function* LlvmCodeGen::GetHashFunction(int num_bytes) {
llvm::Function* fn = prototype.GeneratePrototype(&builder, &args[0]);
llvm::Value* data = args[0];
llvm::Value* result = args[2];
-
+#ifdef __aarch64__
+ llvm::Function* crc8_fn = llvm_intrinsics_[llvm::Intrinsic::aarch64_crc32cb];
+ llvm::Function* crc16_fn = llvm_intrinsics_[llvm::Intrinsic::aarch64_crc32ch];
+ llvm::Function* crc32_fn = llvm_intrinsics_[llvm::Intrinsic::aarch64_crc32cw];
+ llvm::Function* crc64_fn = llvm_intrinsics_[llvm::Intrinsic::aarch64_crc32cx];
+#else
llvm::Function* crc8_fn = llvm_intrinsics_[llvm::Intrinsic::x86_sse42_crc32_32_8];
llvm::Function* crc16_fn = llvm_intrinsics_[llvm::Intrinsic::x86_sse42_crc32_32_16];
llvm::Function* crc32_fn = llvm_intrinsics_[llvm::Intrinsic::x86_sse42_crc32_32_32];
llvm::Function* crc64_fn = llvm_intrinsics_[llvm::Intrinsic::x86_sse42_crc32_64_64];
+#endif
// Generate the crc instructions starting with the highest number of bytes
if (num_bytes >= 8) {
+#ifndef __aarch64__
llvm::Value* result_64 = builder.CreateZExt(result, i64_type());
+#endif
llvm::Value* ptr = builder.CreateBitCast(data, i64_ptr_type());
int i = 0;
while (num_bytes >= 8) {
llvm::Value* index[] = {GetI32Constant(i++)};
llvm::Value* d = builder.CreateLoad(builder.CreateInBoundsGEP(ptr, index));
+#ifdef __aarch64__
+ result = builder.CreateCall(crc64_fn, llvm::ArrayRef<llvm::Value*>({result, d}));
+#else
result_64 =
builder.CreateCall(crc64_fn, llvm::ArrayRef<llvm::Value*>({result_64, d}));
+#endif
num_bytes -= 8;
}
+#ifndef __aarch64__
result = builder.CreateTrunc(result_64, i32_type());
+#endif
llvm::Value* index[] = {GetI32Constant(i * 8)};
// Update data to past the 8-byte chunks
data = builder.CreateInBoundsGEP(data, index);
@@ -1660,6 +1687,9 @@ llvm::Function* LlvmCodeGen::GetHashFunction(int num_bytes) {
DCHECK_LT(num_bytes, 4);
llvm::Value* ptr = builder.CreateBitCast(data, i16_ptr_type());
llvm::Value* d = builder.CreateLoad(ptr);
+#ifdef __aarch64__
+ d = builder.CreateZExt(d, i32_type());
+#endif
result = builder.CreateCall(crc16_fn, llvm::ArrayRef<llvm::Value*>({result, d}));
llvm::Value* index[] = {GetI16Constant(2)};
data = builder.CreateInBoundsGEP(data, index);
@@ -1669,6 +1699,9 @@ llvm::Function* LlvmCodeGen::GetHashFunction(int num_bytes) {
if (num_bytes > 0) {
DCHECK_EQ(num_bytes, 1);
llvm::Value* d = builder.CreateLoad(data);
+#ifdef __aarch64__
+ d = builder.CreateZExt(d, i32_type());
+#endif
result = builder.CreateCall(crc8_fn, llvm::ArrayRef<llvm::Value*>({result, d}));
--num_bytes;
}
diff --git a/be/src/exprs/scalar-fn-call.cc b/be/src/exprs/scalar-fn-call.cc
index 0c52bc6..aaae77e 100644
--- a/be/src/exprs/scalar-fn-call.cc
+++ b/be/src/exprs/scalar-fn-call.cc
@@ -348,13 +348,15 @@ Status ScalarFnCall::GetCodegendComputeFnImpl(LlvmCodeGen* codegen, llvm::Functi
DCHECK(child_fn != NULL);
llvm::Type* arg_type = CodegenAnyVal::GetUnloweredType(codegen, children_[i]->type());
llvm::Value* arg_val_ptr;
+#ifdef __aarch64__
+ PrimitiveType col_type = children_[i]->type().type;
+#endif
if (i < NumFixedArgs()) {
#ifndef __aarch64__
// Allocate space to store 'child_fn's result so we can pass the pointer to the UDF.
arg_val_ptr = codegen->CreateEntryBlockAlloca(builder, arg_type, "arg_val_ptr");
udf_args.push_back(arg_val_ptr);
#else
- PrimitiveType col_type = children_[i]->type().type;
if (col_type != TYPE_BOOLEAN and col_type != TYPE_TINYINT
and col_type != TYPE_SMALLINT) {
arg_val_ptr = codegen->CreateEntryBlockAlloca(builder, arg_type, "arg_val_ptr");
diff --git a/be/src/gutil/sysinfo.cc b/be/src/gutil/sysinfo.cc
index 01f2597..adc7554 100644
--- a/be/src/gutil/sysinfo.cc
+++ b/be/src/gutil/sysinfo.cc
@@ -469,4 +469,12 @@ int MaxCPUIndex(void) {
return cpuinfo_max_cpu_index;
}
+bool IsAarch64(void) {
+#ifdef __aarch64__
+ return true;
+#else
+ return false;
+#endif
+}
+
} // namespace base
diff --git a/be/src/gutil/sysinfo.h b/be/src/gutil/sysinfo.h
index d46cfe5..df37b72 100644
--- a/be/src/gutil/sysinfo.h
+++ b/be/src/gutil/sysinfo.h
@@ -65,5 +65,8 @@ extern double CyclesPerSecond(void);
// Exposed for testing.
extern int ParseMaxCpuIndex(const char* str);
+// Return current platform is aarch64 or not
+extern bool IsAarch64();
+
} // namespace base
#endif /* #ifndef _SYSINFO_H_ */
[impala] 02/02: IMPALA-10054: Fix flakiness in
test_multiple_sort_run_bytes_limits
Posted by st...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
stakiar pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit 5149ea7ea3f9bb6eb5b1304f6d09f74915696926
Author: Riza Suminto <ri...@cloudera.com>
AuthorDate: Thu Aug 6 08:43:52 2020 -0700
IMPALA-10054: Fix flakiness in test_multiple_sort_run_bytes_limits
test_multiple_sort_run_bytes_limits seems to become flaky in
ubuntu-16.04-dockerised-tests. This flakiness may come from accuracy
change in query estimates, the mem_limit specified in the test does not
fit anymore, or query concurrency in mini cluster that may disturb
expected memory allocation. This patch remove the second test case of
test_multiple_sort_run_bytes_limits due to variability in several test
run in the past. It does not compromise the test itself because the
basic feature of sort_run_bytes_limit is still verifiable by the
remaining test cases. The assertion is also changed a bit to allow
easier debugging in case test regression occurs again in the future.
Testing:
- Run and pass test_sort.py
Change-Id: I84a8b579c943cddba4432cf183f7f002ef8ec6ad
Reviewed-on: http://gerrit.cloudera.org:8080/16301
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
tests/query_test/test_sort.py | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/tests/query_test/test_sort.py b/tests/query_test/test_sort.py
index 3eec934..2be5748 100644
--- a/tests/query_test/test_sort.py
+++ b/tests/query_test/test_sort.py
@@ -15,6 +15,7 @@
# specific language governing permissions and limitations
# under the License.
+import re
from copy import copy, deepcopy
from tests.common.impala_test_suite import ImpalaTestSuite
@@ -76,17 +77,16 @@ class TestQueryFullSort(ImpalaTestSuite):
a query is, the more sort runs are likely to be produced and spilled.
Case 1 : 0 SpilledRuns, because all rows fit within the maximum reservation.
sort_run_bytes_limit is not enforced.
- Case 2 : 3 SpilledRuns, because the first run hit reservation limit, and the
- next 2 runs are capped to 100m.
- Case 3 : 4 SpilledRuns, because sort node estimate that spill is inevitable.
+ Case 2 : 4 SpilledRuns, because sort node estimate that spill is inevitable.
So all runs are capped to 130m, including the first one."""
- options = [('3g', '100m', '0'), ('620m', '100m', '3'), ('400m', '130m', '4')]
+ options = [('2g', '100m', '0'), ('400m', '130m', '4')]
for (mem_limit, sort_run_bytes_limit, spilled_runs) in options:
exec_option['mem_limit'] = mem_limit
exec_option['sort_run_bytes_limit'] = sort_run_bytes_limit
query_result = self.execute_query(
query, exec_option, table_format=table_format)
- assert "SpilledRuns: " + spilled_runs in query_result.runtime_profile
+ m = re.search(r'\s+\- SpilledRuns: .*', query_result.runtime_profile)
+ assert "SpilledRuns: " + spilled_runs in m.group()
result = transpose_results(query_result.data)
assert(result[0] == sorted(result[0]))