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 2023/08/09 23:43:06 UTC
[impala] branch master updated (3435baba6 -> 4a8701925)
This is an automated email from the ASF dual-hosted git repository.
stigahuang pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
from 3435baba6 IMPALA-12306: (Part 1) Make codegen cache tests with symbol emitter more robust
new 826da8203 IMPALA-12311: Remove extra newlines in the updated golden file
new 4a8701925 IMPALA-12314: (Addendum) Fix building on aarch64
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/CMakeLists.txt | 6 +++---
be/src/codegen/llvm-codegen.cc | 32 +++++++++++++-------------------
tests/util/test_file_parser.py | 13 +++++++++++--
3 files changed, 27 insertions(+), 24 deletions(-)
[impala] 01/02: IMPALA-12311: Remove extra newlines in the updated golden file
Posted by st...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
stigahuang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit 826da820347387bd0587324cd29d44c9a29dd166
Author: Fang-Yu Rao <fa...@cloudera.com>
AuthorDate: Mon Jul 31 16:52:17 2023 -0700
IMPALA-12311: Remove extra newlines in the updated golden file
This patch removes extra newlines added to subsections when we parse
test queries in an end-to-end test file. Specifically, in
parse_test_file_text(), we append an extra newline in every subsection
of a test query, resulting in one extra newline in the updated golden
file if we add '--update_results' when running this test file to produce
the updated golden file. This could be seen by looking at the updated
golding file under $IMPALA_HOME/logs/ee_tests after executing the
following.
$IMPALA_HOME/bin/impala-py.test \
--update_results \
$IMPALA_HOME/tests/query_test/test_tpcds_queries.py::TestTpcdsDecimalV2Query::test_tpcds_q1
The extra newline is needed for the verification of the subsections of
RESULTS, DML_RESULTS, ERRORS to disambiguate the case of no lines from
a single line with no text and will not be needed after the
verification.
To remove such extra newlines, we choose to do it in the place when
write_test_file() is called to output the updated golden file since this
requires fewer changes. An alternative could be to only add an extra
newline for those 3 subsections mentioned above and also remove the last
newline added in join_section_lines(), which would be called when the
actual contents do not match the expected contents specified in the
original golden file in the subsections of ERRORS, TYPES, LABELS,
RESULTS and DML_RESULTS. Additional changes to TestTestFileParser are
also required if we adopted the alternative.
Testing:
- Verified that the extra newlines are removed after this patch.
Change-Id: Ic7668a437267bd76afecba8f87ead32d82580414
Reviewed-on: http://gerrit.cloudera.org:8080/20272
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
tests/util/test_file_parser.py | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/tests/util/test_file_parser.py b/tests/util/test_file_parser.py
index 256bc5277..3fe347f7d 100644
--- a/tests/util/test_file_parser.py
+++ b/tests/util/test_file_parser.py
@@ -208,6 +208,9 @@ def parse_test_file_text(text, valid_section_names, skip_unknown_sections=True):
if len(lines_content) != 0:
# Add trailing newline to last line if present. This disambiguates between the
# case of no lines versus a single line with no text.
+ # This extra newline is needed only for the verification of the subsections of
+ # RESULTS, DML_RESULTS and ERRORS and will be removed in write_test_file()
+ # when '--update_results' is added to output the updated golden file.
subsection_str += "\n"
if subsection_name not in valid_section_names:
@@ -294,6 +297,9 @@ def split_section_lines(section_str):
def join_section_lines(lines):
"""
The inverse of split_section_lines().
+ The extra newline at the end will be removed in write_test_file() so that when the
+ actual contents of a subsection do not match the expected contents, we won't see
+ extra newlines in those subsections (ERRORS, TYPES, LABELS, RESULTS and DML_RESULTS).
"""
return '\n'.join(lines) + '\n'
@@ -323,8 +329,11 @@ def write_test_file(test_file_name, test_file_sections, encoding=None):
if section_name == 'RESULTS' and test_case.get('VERIFIER'):
full_section_name = '%s: %s' % (section_name, test_case['VERIFIER'])
test_file_text.append("%s %s" % (SUBSECTION_DELIMITER, full_section_name))
- section_value = ''.join(test_case[section_name])
- if section_value.strip():
+ # We remove the extra newlines added in parse_test_file_text() so that in the
+ # updated golden file we will not see an extra newline at the end of each
+ # subsection.
+ section_value = ''.join(test_case[section_name]).strip()
+ if section_value:
test_file_text.append(section_value)
test_file_text.append(SECTION_DELIMITER)
test_file.write(('\n').join(test_file_text))
[impala] 02/02: IMPALA-12314: (Addendum) Fix building on aarch64
Posted by st...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
stigahuang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit 4a8701925b1939a066cb3d0a5cfd7d8f36ad05ed
Author: Michael Smith <mi...@apache.org>
AuthorDate: Tue Aug 8 21:26:38 2023 +0000
IMPALA-12314: (Addendum) Fix building on aarch64
Change-Id: Ie7223b82c606c91f98edbf583dfab2f9fe7ddbfd
Reviewed-on: http://gerrit.cloudera.org:8080/20333
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
be/src/codegen/CMakeLists.txt | 6 +++---
be/src/codegen/llvm-codegen.cc | 32 +++++++++++++-------------------
2 files changed, 16 insertions(+), 22 deletions(-)
diff --git a/be/src/codegen/CMakeLists.txt b/be/src/codegen/CMakeLists.txt
index 4c5f497bd..0026601ec 100644
--- a/be/src/codegen/CMakeLists.txt
+++ b/be/src/codegen/CMakeLists.txt
@@ -65,16 +65,16 @@ add_custom_target(gen_ir_descriptions ALL DEPENDS ${IR_DESC_GEN_OUTPUT})
set(IR_INPUT_FILES impala-ir.cc)
-function(COMPILE_TO_IR_C_ARRAY IR_C_FILE VARNAME OPT PLATFORM_SPECIFIC)
+function(COMPILE_TO_IR_C_ARRAY IR_C_FILE VARNAME)
# Run the clang compiler to generate IR. Then run the LLVM opt tool to apply specific
# optimisations. We need to compile to IR several times for different optimization settings
- # and legacy AVX support.
+ # and legacy AVX support. Additional args (ARGN) are passed to clang as compiler flags.
get_filename_component(BASE_NAME ${IR_C_FILE} NAME_WE)
set(IR_OUTPUT_FILE "${LLVM_IR_OUTPUT_DIRECTORY}/${BASE_NAME}.bc")
set(IR_TMP_OUTPUT_FILE "${LLVM_IR_OUTPUT_DIRECTORY}/${BASE_NAME}-tmp.bc")
add_custom_command(
OUTPUT ${IR_OUTPUT_FILE}
- COMMAND ${LLVM_CLANG_EXECUTABLE} ${CLANG_IR_CXX_FLAGS} ${OPT} ${PLATFORM_SPECIFIC}
+ COMMAND ${LLVM_CLANG_EXECUTABLE} ${CLANG_IR_CXX_FLAGS} ${ARGN}
${CLANG_INCLUDE_FLAGS} ${IR_INPUT_FILES} -o ${IR_TMP_OUTPUT_FILE}
COMMAND ${LLVM_OPT_EXECUTABLE} ${LLVM_OPT_IR_FLAGS} < ${IR_TMP_OUTPUT_FILE} > ${IR_OUTPUT_FILE}
COMMAND rm ${IR_TMP_OUTPUT_FILE}
diff --git a/be/src/codegen/llvm-codegen.cc b/be/src/codegen/llvm-codegen.cc
index 5e2f8be24..ac523da9d 100644
--- a/be/src/codegen/llvm-codegen.cc
+++ b/be/src/codegen/llvm-codegen.cc
@@ -272,8 +272,19 @@ Status LlvmCodeGen::CreateFromMemory(FragmentState* state, ObjectPool* pool,
SCOPED_THREAD_COUNTER_MEASUREMENT((*codegen)->llvm_thread_counters());
llvm::StringRef module_ir;
- string module_name;
-
+ string module_name = "Impala IR";
+ if (FLAGS_llvm_ir_opt == "O1") {
+ module_ir = llvm::StringRef(
+ reinterpret_cast<const char*>(impala_llvm_o1_ir), impala_llvm_o1_ir_len);
+ } else if (FLAGS_llvm_ir_opt == "O2") {
+ module_ir = llvm::StringRef(
+ reinterpret_cast<const char*>(impala_llvm_o2_ir), impala_llvm_o2_ir_len);
+ } else if (FLAGS_llvm_ir_opt == "Os") {
+ module_ir = llvm::StringRef(
+ reinterpret_cast<const char*>(impala_llvm_os_ir), impala_llvm_os_ir_len);
+ } else {
+ CHECK(false) << "llvm_ir_opt flag invalid; try O1, O2, or Os.";
+ }
#if __x86_64__
// By default, Impala now requires AVX2 support, but the enable_legacy_avx_support
// flag can allow running on AVX machines. The minimum requirement must have already
@@ -281,18 +292,6 @@ Status LlvmCodeGen::CreateFromMemory(FragmentState* state, ObjectPool* pool,
// LLVM IR to use.
if (IsCPUFeatureEnabled(CpuInfo::AVX2)) {
// Use the default IR that supports AVX2
- if (FLAGS_llvm_ir_opt == "O1") {
- module_ir = llvm::StringRef(
- reinterpret_cast<const char*>(impala_llvm_o1_ir), impala_llvm_o1_ir_len);
- } else if (FLAGS_llvm_ir_opt == "O2") {
- module_ir = llvm::StringRef(
- reinterpret_cast<const char*>(impala_llvm_o2_ir), impala_llvm_o2_ir_len);
- } else if (FLAGS_llvm_ir_opt == "Os") {
- module_ir = llvm::StringRef(
- reinterpret_cast<const char*>(impala_llvm_os_ir), impala_llvm_os_ir_len);
- } else {
- CHECK(false) << "llvm_ir_opt flag invalid; try O1, O2, or Os.";
- }
module_name = "Impala IR with AVX2 support";
} else if (FLAGS_enable_legacy_avx_support && IsCPUFeatureEnabled(CpuInfo::AVX)) {
// If there is no AVX but legacy mode is enabled, use legacy IR with AVX support
@@ -304,11 +303,6 @@ Status LlvmCodeGen::CreateFromMemory(FragmentState* state, ObjectPool* pool,
// This should have been enforced earlier.
CHECK(false) << "CPU is missing AVX/AVX2 support";
}
-#else
- // Non-x86_64 always use the default IR
- module_ir = llvm::StringRef(
- reinterpret_cast<const char*>(impala_llvm_ir), impala_llvm_ir_len);
- module_name = "Impala IR";
#endif
unique_ptr<llvm::MemoryBuffer> module_ir_buf(