Michael Smith 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 10:

(10 comments)

http://gerrit.cloudera.org:8080/#/c/24940/7//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/24940/7//COMMIT_MSG@38
PS7, Line 38: e
> nit: capitalize please :)
Done


http://gerrit.cloudera.org:8080/#/c/24940/7//COMMIT_MSG@40
PS7, Line 40: c
> nit: typo
Done


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: to Clang 12+. Upgrades
           : to LLVM
> Info on the version used would be nice.
Done


http://gerrit.cloudera.org:8080/#/c/24940/8//COMMIT_MSG@32
PS8, Line 32: IcebergFunctions::TruncatePartitionTransformNumericImpl.
            :
> Could this and similar changes go to a preparation commit to make this patc
These weren't needed after updating linkers, so removed.


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/
Done. I'll kick off a new set of perf-AB-test runs.


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:   cout << Benchmark::GetMachineInfo() << endl;
> How are these test init changes connected to this patch? Maybe they could b
Removed.


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:       std::shared_ptr<::apache::thrift::protocol::TProtocol> 
iprot,
> Would this this code compile with old gcc? Maybe they could be moved to a p
Switched to adding GCC diagnostic instead and cleaning up includes.


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 {
> Yes, if that's the case, we'll need to track it carefully:
Still investigating, I'm not certain it actually changes anything compared to 
building with GCC 10.


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
Done


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
Done



--
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: 10
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 17:45:48 +0000
Gerrit-HasComments: Yes

Reply via email to