diff --git a/src/llvm_backend_const.cpp b/src/llvm_backend_const.cpp index aff877edc..4b762d894 100644 --- a/src/llvm_backend_const.cpp +++ b/src/llvm_backend_const.cpp @@ -726,13 +726,18 @@ gb_internal LLVMValueRef lb_fill_fixed_capacity_dynamic_array(lbModule *m, i64 e LLVMValueRef array_len = lb_const_int(m, t_int, elem_count).value; isize svalue_count = 0; - LLVMValueRef svalues[3] = {}; + LLVMValueRef svalues[4] = {}; svalues[svalue_count++] = array_backing; + i64 size = type_size_of(bt); i64 padding = bt->FixedCapacityDynamicArray.padding_needed; if (padding > 0) { svalues[svalue_count++] = LLVMConstNull(lb_type_padding_filler(m, padding, 1)); } svalues[svalue_count++] = array_len; + i64 tail = size - type_size_of(elem_type)*capacity - padding - build_context.int_size; + if (tail > 0) { + svalues[svalue_count++] = LLVMConstNull(lb_type_padding_filler(m, tail, 1)); + } return llvm_const_named_struct(m, original_type, svalues, svalue_count); } diff --git a/src/llvm_backend_general.cpp b/src/llvm_backend_general.cpp index eb0d57064..0108c2185 100644 --- a/src/llvm_backend_general.cpp +++ b/src/llvm_backend_general.cpp @@ -3199,18 +3199,25 @@ gb_internal LLVMTypeRef lb_type_internal(lbModule *m, Type *type) { { unsigned field_count = 0; - LLVMTypeRef fields[3] = {}; + LLVMTypeRef fields[4] = {}; m->internal_type_level += 1; fields[field_count++] = llvm_array_type(lb_type(m, type->FixedCapacityDynamicArray.elem), type->FixedCapacityDynamicArray.capacity); m->internal_type_level -= 1; - gb_unused(type_size_of(type)); - if (type->FixedCapacityDynamicArray.padding_needed > 0) { - fields[field_count++] = lb_type_padding_filler(m, type->FixedCapacityDynamicArray.padding_needed, 1); // padding + i64 size = type_size_of(type); + i64 padding = type->FixedCapacityDynamicArray.padding_needed; + if (padding > 0) { + fields[field_count++] = lb_type_padding_filler(m, padding, 1); // padding } fields[field_count++] = lb_type(m, t_int); // len + // an over-aligned elem (e.g. a matrix) needs more tail padding than LLVM's own alignment gives + i64 tail = size - type_size_of(type->FixedCapacityDynamicArray.elem)*type->FixedCapacityDynamicArray.capacity - padding - build_context.int_size; + if (tail > 0) { + fields[field_count++] = lb_type_padding_filler(m, tail, 1); + } + return LLVMStructTypeInContext(ctx, fields, field_count, false); } break; diff --git a/src/types.cpp b/src/types.cpp index 3a8268271..7861a4f01 100644 --- a/src/types.cpp +++ b/src/types.cpp @@ -1608,35 +1608,6 @@ gb_internal bool is_type_matrix(Type *t) { return t->kind == Type_Matrix; } -gb_internal i64 matrix_align_of(Type *t, struct TypePath *tp) { - t = base_type(t); - GB_ASSERT(t->kind == Type_Matrix); - - Type *elem = t->Matrix.elem; - i64 row_count = gb_max(t->Matrix.row_count, 1); - i64 column_count = gb_max(t->Matrix.column_count, 1); - - bool pop = type_path_push(tp, elem); - if (tp->failure) { - return FAILURE_ALIGNMENT; - } - - i64 elem_align = type_align_of_internal(elem, tp); - if (pop) type_path_pop(tp); - - i64 elem_size = type_size_of(elem); - - - // NOTE(bill, 2021-10-25): The alignment strategy here is to have zero padding - // It would be better for performance to pad each column so that each column - // could be maximally aligned but as a compromise, having no padding will be - // beneficial to third libraries that assume no padding - - gb_unused(row_count); gb_unused(column_count); gb_unused(elem_size); - return gb_clamp(elem_align, 1, build_context.max_simd_align); -} - - gb_internal i64 matrix_type_stride_in_bytes(Type *t, struct TypePath *tp) { t = base_type(t); GB_ASSERT(t->kind == Type_Matrix); @@ -1676,6 +1647,27 @@ gb_internal i64 matrix_type_stride_in_elems(Type *t) { return stride/gb_max(1, type_size_of(t->Matrix.elem)); } +gb_internal i64 matrix_align_of(Type *t, struct TypePath *tp) { + t = base_type(t); + GB_ASSERT(t->kind == Type_Matrix); + + Type *elem = t->Matrix.elem; + + bool pop = type_path_push(tp, elem); + if (tp->failure) { + return FAILURE_ALIGNMENT; + } + + i64 elem_align = type_align_of_internal(elem, tp); + if (pop) type_path_pop(tp); + + // NOTE(bill): Matrices have no padding, so align to the largest power of two dividing the stride, + // i.e. each column (row if #row_major) is aligned, capped at 16 so the layout is the same across targets + i64 stride = matrix_type_stride_in_bytes(t, tp); + i64 align = stride & -stride; + return gb_clamp(align, elem_align, gb_min(16, build_context.max_simd_align)); +} + gb_internal i64 matrix_type_total_internal_elems(Type *t) { t = base_type(t); diff --git a/tests/abi/gen.odin b/tests/abi/gen.odin index 6303ac09c..083e5ceff 100644 --- a/tests/abi/gen.odin +++ b/tests/abi/gen.odin @@ -805,7 +805,8 @@ build :: proc() { ) // --- matrix, which lowers to an array with its own alignment - // a matrix aligns to its element, so the counterpart is a plain array + // a matrix aligns to the largest power of two dividing its column (row if + // #row_major) up to 16, so the counterpart is an over-aligned plain array { fields := make([]Leaf, 4) for i in 0 ..< 4 { @@ -819,9 +820,46 @@ build :: proc() { add( "m22_f32", "struct { m: matrix[2,2]f32 }", - "struct { float m[4]; }", + "struct { _Alignas(8) float m[4]; }", + fields, + ) + } + // NOT at offset 0, where the alignment moves the matrix and changes `size_of` + { + fields := make([]Leaf, 17) + fields[0] = leaf("a", "f32", 0) + for i in 0 ..< 16 { + fields[i + 1] = leaf2( + tp("m[%d, %d]", i % 4, i / 4), + tp("m[%d]", i), + "f32", + val(i + 1, "f32"), + ) + } + add( + "m44_off", + "struct { a: f32, m: matrix[4,4]f32 }", + "struct { float a; _Alignas(16) float m[16]; }", + fields, + ) + } + // #row_major: a 16-byte row aligns where a 12-byte column would not + { + fields := make([]Leaf, 13) + fields[0] = leaf("a", "f32", 0) + for i in 0 ..< 12 { + fields[i + 1] = leaf2( + tp("m[%d, %d]", i / 4, i % 4), + tp("m[%d]", i), + "f32", + val(i + 1, "f32"), + ) + } + add( + "mr34_off", + "struct { a: f32, m: #row_major matrix[3,4]f32 }", + "struct { float a; _Alignas(16) float m[12]; }", fields, - tier = TIER_GNU, ) } diff --git a/tests/issues/test_issue_7336.odin b/tests/issues/test_issue_7336.odin index 305b4a974..7ea02ba83 100644 --- a/tests/issues/test_issue_7336.odin +++ b/tests/issues/test_issue_7336.odin @@ -13,8 +13,9 @@ test_offset :: proc(t: ^testing.T) { s := new(S) defer free(s) - // 4*4 f32 + dynamic len == 72 - expect: uintptr = 4 * 4 * 4 + size_of(int) + // 4*4 f32 + dynamic len, rounded up to the matrix's alignment + A :: align_of(matrix[4, 4]f32) + expect: uintptr = (4 * 4 * 4 + size_of(int) + A - 1) &~ (A - 1) testing.expect(t, offset_of(S, b) == expect) testing.expect(t, uintptr(rawptr(&s.b)) - uintptr(rawptr(s)) == expect) }