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:

(1 comment)

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 {
> Still investigating, I'm not certain it actually changes anything compared
This was inconsistent between x86_64 and aarch64 before:
- x86_64 added explicit padding so DecimalVal is 32-bytes, with 15-bytes of 
padding, then the union selector, then 16 bytes for the value (i32, i64, i128).
- aarch64 didn't, so it had the union selector (presumably taking 16 bytes), 
then 16 bytes for the value.

That's how we got this difference in LLVM codegen: 
https://github.com/apache/impala/blob/master/be/src/codegen/codegen-anyval.cc#L394-L400.

I see 3 options:
1. switch to packed, which results in 17 bytes at 1-byte alignment and is not 
backward compatible on either architecture
2. switch to explicit 8-byte alignment with explicit padding, which would be 
consistent across platforms; that retains sizeof and location so it's backward 
compatible on x86_64 (but probably not aarch64)
3. switch to explicit 8-byte alignment, but only add the explicit padding on 
x86_64; that should retain backwards compatibility on both platforms

I prefer option 2, and we can communicate that aarch64 UDFs need to be rebuilt 
(which are likely a lot less common than x86_64 UDFs).



--
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 18:35:23 +0000
Gerrit-HasComments: Yes

Reply via email to