Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/24940 )
Change subject: IMPALA-11917: Upgrade to GCC 15 and LLVM 22 ...................................................................... Patch Set 8: (9 comments) Went through the simple changes, postponed diving into the llvm. http://gerrit.cloudera.org:8080/#/c/24940/8//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/24940/8//COMMIT_MSG@9 PS8, Line 9: Clang to a newer : version Info on the version used would be nice. http://gerrit.cloudera.org:8080/#/c/24940/8//COMMIT_MSG@32 PS8, Line 32: Updates all backend executables to call InitCommonRuntime to force the : static linker to pull in Common/GlobalFlags archive members. Could this and similar changes go to a preparation commit to make this patch smaller? Most of these look trivial, e.g. adding new headers (I assumed that they were transitively included earlier). Besides less code to review committing these changes early could help avoid conflicts during rebases. I left a few comments around the code for changes that seem doable with existing compilers. http://gerrit.cloudera.org:8080/#/c/24940/8//COMMIT_MSG@56 PS8, Line 56: some info would be nice abuut effect on debug/release build times and tpch/tpcds benchmarks http://gerrit.cloudera.org:8080/#/c/24940/8/be/src/benchmarks/atof-benchmark.cc File be/src/benchmarks/atof-benchmark.cc: http://gerrit.cloudera.org:8080/#/c/24940/8/be/src/benchmarks/atof-benchmark.cc@118 PS8, Line 118: impala::InitCommonRuntime(argc, argv, false, impala::TestInfo::BE_TEST); How are these test init changes connected to this patch? Maybe they could be moved to a preparation commit? http://gerrit.cloudera.org:8080/#/c/24940/8/be/src/catalog/catalog-service-client-wrapper.h File be/src/catalog/catalog-service-client-wrapper.h: http://gerrit.cloudera.org:8080/#/c/24940/8/be/src/catalog/catalog-service-client-wrapper.h@35 PS8, Line 35: void ExecDdlWithRetry( Would this this code compile with old gcc? Maybe they could be moved to a preparation commit? http://gerrit.cloudera.org:8080/#/c/24940/8/be/src/udf/udf.h File be/src/udf/udf.h: http://gerrit.cloudera.org:8080/#/c/24940/8/be/src/udf/udf.h@735 PS8, Line 735: // val16 is __int128_t, whose natural alignment is 16 bytes, but instances of this : // struct routinely live in memory that's only 8-byte aligned (tuple slots, UDA : // intermediate buffers from FunctionContext::Allocate(), etc). "packed" tells the : // compiler not to assume 16-byte alignment when generating loads/stores/copies for : // this type, avoiding SIGSEGV from misaligned SSE instructions (e.g. movaps). : struct __attribute__((packed)) DecimalVal : public impala_udf::AnyVal { What is the effect of this on existing UDFs? I assume recompilation is needed - this is generally the assumption for new Impala releases, but could be stressed in the commit message. Could this also break the compilation of existing UDF code? If yes, then it is a bit of a breaking change, preferably shipped in Impala 5.0. http://gerrit.cloudera.org:8080/#/c/24940/8/be/src/util/codec.h File be/src/util/codec.h: http://gerrit.cloudera.org:8080/#/c/24940/8/be/src/util/codec.h@61 PS8, Line 61: typedef std::map<std::string, THdfsCompression::type> CodecMap; Would the const rewrites compile with old gcc? Maybe they could be moved to a preparation commit? http://gerrit.cloudera.org:8080/#/c/24940/8/be/src/util/compression-util.cc File be/src/util/compression-util.cc: http://gerrit.cloudera.org:8080/#/c/24940/8/be/src/util/compression-util.cc@26 PS8, Line 26: : Maybe they could be moved to a preparation commit? http://gerrit.cloudera.org:8080/#/c/24940/8/be/src/util/jwt-util-internal.h File be/src/util/jwt-util-internal.h: http://gerrit.cloudera.org:8080/#/c/24940/8/be/src/util/jwt-util-internal.h@56 PS8, Line 56: : verifier_(jwt::verify()), algorithm_(std::move(algorithm)), Would the move rewrites compile with old gcc? Maybe they could be moved to a preparation commit? -- To view, visit http://gerrit.cloudera.org:8080/24940 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7dda730fa98ebe3825969265627a560b0c3095f9 Gerrit-Change-Number: 24940 Gerrit-PatchSet: 8 Gerrit-Owner: Michael Smith <[email protected]> Gerrit-Reviewer: Balazs Hevele <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Laszlo Gaal <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Comment-Date: Wed, 30 Sep 2026 06:18:44 +0000 Gerrit-HasComments: Yes
