Skip to content

Commit ec3c02a

Browse files
committed
fix alignment computation for nested objects (#57722)
The alignment of a nested object (in C layouts) is not affected by the alignment of its parent container when computing a field offset (as if it will be allocated at address 0). This can be strongly counter-intuitive (as it implies it should add padding where it does not seem to provide value to the user), but this is required to match the C standard. It also permits users to write custom allocators which happen to provide alignment in excess of that which codegen may assume is guaranteed, and get the behavioral characteristics they intended to specify (without resorting to computing explicit padding). Addresses #57713 (comment) (Cherry-picked from c9008ff, with typo fix to typed_loaded memcpy which was already deleted from master)
1 parent 363a119 commit ec3c02a

File tree

4 files changed

+48
-19
lines changed

4 files changed

+48
-19
lines changed

doc/src/manual/calling-c-and-fortran-code.md

+15-9
Original file line numberDiff line numberDiff line change
@@ -547,15 +547,14 @@ is not valid, since the type layout of `T` is not known statically.
547547

548548
### SIMD Values
549549

550-
Note: This feature is currently implemented on 64-bit x86 and AArch64 platforms only.
551-
552550
If a C/C++ routine has an argument or return value that is a native SIMD type, the corresponding
553551
Julia type is a homogeneous tuple of `VecElement` that naturally maps to the SIMD type. Specifically:
554552

555-
> * The tuple must be the same size as the SIMD type. For example, a tuple representing an `__m128`
556-
> on x86 must have a size of 16 bytes.
557-
> * The element type of the tuple must be an instance of `VecElement{T}` where `T` is a primitive type that
558-
> is 1, 2, 4 or 8 bytes.
553+
> * The tuple must be the same size and elements as the SIMD type. For example, a tuple
554+
> representing an `__m128` on x86 must have a size of 16 bytes and Float32 elements.
555+
> * The element type of the tuple must be an instance of `VecElement{T}` where `T` is a
556+
> primitive type with a power-of-two number of bytes (e.g. 1, 2, 4, 8, 16, etc) such as
557+
> Int8 or Float64.
559558
560559
For instance, consider this C routine that uses AVX intrinsics:
561560

@@ -628,6 +627,10 @@ For translating a C argument list to Julia:
628627

629628
* `T`, where `T` is a Julia leaf type
630629
* argument value will be copied (passed by value)
630+
* `vector T` (or `__attribute__ vector_size`, or a typedef such as `__m128`)
631+
632+
* `NTuple{N, VecElement{T}}`, where `T` is a primitive Julia type of the correct size
633+
and N is the number of elements in the vector (equal to `vector_size / sizeof T`).
631634
* `void*`
632635

633636
* depends on how this parameter is used, first translate this to the intended pointer type, then
@@ -674,13 +677,16 @@ For translating a C return type to Julia:
674677
* `T`, where `T` is one of the primitive types: `char`, `int`, `long`, `short`, `float`, `double`,
675678
`complex`, `enum` or any of their `typedef` equivalents
676679

677-
* `T`, where `T` is an equivalent Julia Bits Type (per the table above)
678-
* if `T` is an `enum`, the argument type should be equivalent to `Cint` or `Cuint`
680+
* same as C argument list
679681
* argument value will be copied (returned by-value)
680682
* `struct T` (including typedef to a struct)
681683

682-
* `T`, where `T` is a Julia Leaf Type
684+
* same as C argument list
683685
* argument value will be copied (returned by-value)
686+
687+
* `vector T`
688+
689+
* same as C argument list
684690
* `void*`
685691

686692
* depends on how this parameter is used, first translate this to the intended pointer type, then

src/cgutils.cpp

+4-3
Original file line numberDiff line numberDiff line change
@@ -1971,7 +1971,7 @@ static jl_cgval_t typed_load(jl_codectx_t &ctx, Value *ptr, Value *idx_0based, j
19711971
else if (!alignment)
19721972
alignment = julia_alignment(jltype);
19731973
if (intcast && Order == AtomicOrdering::NotAtomic) {
1974-
emit_memcpy(ctx, intcast, jl_aliasinfo_t::fromTBAA(ctx, ctx.tbaa().tbaa_stack), data, jl_aliasinfo_t::fromTBAA(ctx, tbaa), nb, Align(alignment), intcast->getAlign());
1974+
emit_memcpy(ctx, intcast, jl_aliasinfo_t::fromTBAA(ctx, ctx.tbaa().tbaa_stack), data, jl_aliasinfo_t::fromTBAA(ctx, tbaa), nb, intcast->getAlign(), Align(alignment));
19751975
}
19761976
else {
19771977
if (!isboxed && jl_is_genericmemoryref_type(jltype)) {
@@ -3214,7 +3214,7 @@ static void union_alloca_type(jl_uniontype_t *ut,
32143214
[&](unsigned idx, jl_datatype_t *jt) {
32153215
if (!jl_is_datatype_singleton(jt)) {
32163216
size_t nb1 = jl_datatype_size(jt);
3217-
size_t align1 = jl_datatype_align(jt);
3217+
size_t align1 = julia_alignment((jl_value_t*)jt);
32183218
if (nb1 > nbytes)
32193219
nbytes = nb1;
32203220
if (align1 > align)
@@ -3796,9 +3796,10 @@ static jl_cgval_t emit_new_struct(jl_codectx_t &ctx, jl_value_t *ty, size_t narg
37963796

37973797
// whether we should perform the initialization with the struct as a IR value
37983798
// or instead initialize the stack buffer with stores
3799+
// although we do the former if it is a vector or could be a vector element
37993800
auto tracked = CountTrackedPointers(lt);
38003801
bool init_as_value = false;
3801-
if (lt->isVectorTy() || jl_is_vecelement_type(ty)) { // maybe also check the size ?
3802+
if (lt->isVectorTy() || jl_special_vector_alignment(1, ty) != 0) {
38023803
init_as_value = true;
38033804
}
38043805
else if (tracked.count) {

src/datatype.c

+22-7
Original file line numberDiff line numberDiff line change
@@ -298,9 +298,10 @@ static jl_datatype_layout_t *jl_get_layout(uint32_t sz,
298298
}
299299

300300
// Determine if homogeneous tuple with fields of type t will have
301-
// a special alignment beyond normal Julia rules.
301+
// a special alignment and vector-ABI beyond normal rules for aggregates.
302302
// Return special alignment if one exists, 0 if normal alignment rules hold.
303303
// A non-zero result *must* match the LLVM rules for a vector type <nfields x t>.
304+
// Matching the compiler's `__attribute__ vector_size` behavior.
304305
// For sake of Ahead-Of-Time (AOT) compilation, this routine has to work
305306
// without LLVM being available.
306307
unsigned jl_special_vector_alignment(size_t nfields, jl_value_t *t)
@@ -315,8 +316,12 @@ unsigned jl_special_vector_alignment(size_t nfields, jl_value_t *t)
315316
// motivating use case comes up for Julia, we reject pointers.
316317
return 0;
317318
size_t elsz = jl_datatype_size(ty);
318-
if (elsz != 1 && elsz != 2 && elsz != 4 && elsz != 8)
319-
// Only handle power-of-two-sized elements (for now)
319+
if (next_power_of_two(elsz) != elsz)
320+
// Only handle power-of-two-sized elements (for now), since other
321+
// lengths may be packed into very complicated arrangements (llvm pads
322+
// extra bits on most platforms when computing alignment but not when
323+
// computing type size, but adds no extra bytes for each element, so
324+
// their effect on offsets are never what you may naturally expect).
320325
return 0;
321326
size_t size = nfields * elsz;
322327
// Use natural alignment for this vector: this matches LLVM and clang.
@@ -707,9 +712,9 @@ void jl_compute_field_offsets(jl_datatype_t *st)
707712
}
708713
else {
709714
fsz = sizeof(void*);
710-
if (fsz > MAX_ALIGN)
711-
fsz = MAX_ALIGN;
712715
al = fsz;
716+
if (al > MAX_ALIGN)
717+
al = MAX_ALIGN;
713718
desc[i].isptr = 1;
714719
zeroinit = 1;
715720
npointers++;
@@ -754,8 +759,6 @@ void jl_compute_field_offsets(jl_datatype_t *st)
754759
if (al > alignm)
755760
alignm = al;
756761
}
757-
if (alignm > MAX_ALIGN)
758-
alignm = MAX_ALIGN; // We cannot guarantee alignments over 16 bytes because that's what our heap is aligned as
759762
if (LLT_ALIGN(sz, alignm) > sz) {
760763
haspadding = 1;
761764
sz = LLT_ALIGN(sz, alignm);
@@ -931,6 +934,18 @@ JL_DLLEXPORT jl_datatype_t *jl_new_primitivetype(jl_value_t *name, jl_module_t *
931934
jl_emptysvec, jl_emptysvec, jl_emptysvec, 0, 0, 0);
932935
uint32_t nbytes = (nbits + 7) / 8;
933936
uint32_t alignm = next_power_of_two(nbytes);
937+
# if defined(_CPU_X86_) && !defined(_OS_WINDOWS_)
938+
// datalayout strings are often weird: on 64-bit they usually follow fairly simple rules,
939+
// but on x86 32 bit platforms, sometimes 5 to 8 byte types are
940+
// 32-bit aligned even though the MAX_ALIGN (for types 9+ bytes) is 16
941+
// (except for f80 which is align 4 on Mingw, Linux, and BSDs--but align 16 on MSVC and Darwin)
942+
// https://llvm.org/doxygen/ARMTargetMachine_8cpp.html#adb29b487708f0dc2a940345b68649270
943+
// https://llvm.org/doxygen/AArch64TargetMachine_8cpp.html#a003a58caf135efbf7273c5ed84e700d7
944+
// https://llvm.org/doxygen/X86TargetMachine_8cpp.html#aefdbcd6131ef195da070cef7fdaf0532
945+
// 32-bit alignment is weird
946+
if (alignm == 8)
947+
alignm = 4;
948+
# endif
934949
if (alignm > MAX_ALIGN)
935950
alignm = MAX_ALIGN;
936951
// memoize isprimitivetype, since it is much easier than checking

test/core.jl

+7
Original file line numberDiff line numberDiff line change
@@ -5668,6 +5668,13 @@ let ni128 = sizeof(FP128test) ÷ sizeof(Int),
56685668
@test reinterpret(UInt128, arr[2].fp) == expected
56695669
end
56705670

5671+
# make sure VecElement Tuple has the C alignment and ABI for supported types
5672+
primitive type Int24 24 end
5673+
@test Base.datatype_alignment(NTuple{10,VecElement{Int16}}) == 32
5674+
@test Base.datatype_alignment(NTuple{10,VecElement{Int24}}) == 4
5675+
@test Base.datatype_alignment(NTuple{10,VecElement{Int64}}) == 128
5676+
@test Base.datatype_alignment(NTuple{10,VecElement{Int128}}) == 256
5677+
56715678
# issue #21516
56725679
struct T21516
56735680
x::Vector{Float64}

0 commit comments

Comments
 (0)