diff --git a/src/llvm_abi.cpp b/src/llvm_abi.cpp index 528ddca88..20f6f33fd 100644 --- a/src/llvm_abi.cpp +++ b/src/llvm_abi.cpp @@ -599,11 +599,12 @@ namespace lbAbiAmd64SysV { }; gb_internal void classify_with(LLVMTypeRef t, Array *cls, i64 ix, i64 off); + gb_internal void unify(Array *cls, i64 i, RegClass const newv); gb_internal void fixup(LLVMTypeRef t, Array *cls); gb_internal lbArgType amd64_type(LLVMContextRef c, LLVMTypeRef type, Amd64TypeAttributeKind attribute_kind, ProcCallingConvention calling_convention, bool is_arg, - i32 *int_regs, i32 *sse_regs); - gb_internal Array classify(LLVMTypeRef t); + i32 *int_regs, i32 *sse_regs, Type *source_type); + gb_internal Array classify(LLVMTypeRef t, Type *source_type); gb_internal LLVMTypeRef llreg(LLVMContextRef c, Array const ®_classes, LLVMTypeRef type); gb_internal LB_ABI_COMPUTE_RETURN_TYPE(compute_return_type) { @@ -614,11 +615,11 @@ namespace lbAbiAmd64SysV { return amd64_type(c, return_type, Amd64TypeAttribute_StructRect, ft->calling_convention, false, - nullptr, nullptr); + nullptr, nullptr, nullptr); } gb_internal LB_ABI_INFO(abi_info) { - LLVMContextRef c = m->ctx; + LLVMContextRef c = m->ctx; lbFunctionType *ft = permanent_alloc_item(); ft->ctx = c; ft->calling_convention = calling_convention; @@ -626,14 +627,44 @@ namespace lbAbiAmd64SysV { i32 int_regs = 6; // rdi, rsi, rdx, rcx, r8, r9 i32 sse_regs = 8; // xmm0-xmm7 - ft->args = array_make(lb_function_type_args_allocator(), arg_count); - for (unsigned i = 0; i < arg_count; i++) { - ft->args[i] = amd64_type(c, arg_types[i], Amd64TypeAttribute_ByVal, calling_convention, - true, - &int_regs, &sse_regs); + // The source type of each parameter, where one exists. `arg_types` can carry entries + // with no counterpart. This walks the tuple the way lbAbiArm64 does and hands back nullptr once it runs out. + Entity **params = nullptr; + isize param_count = 0; + if (original_type != nullptr && original_type->kind == Type_Proc && original_type->Proc.params != nullptr) { + params = original_type->Proc.params->Tuple.variables.data; + param_count = original_type->Proc.params->Tuple.variables.count; } - ft->ret = compute_return_type(ft, c, return_type, return_is_defined, return_is_tuple); + ft->args = array_make(lb_function_type_args_allocator(), arg_count); + for (unsigned i = 0, j = 0; i < arg_count; i++, j++) { + while (cast(isize)j < param_count && params[j]->kind != Entity_Variable) { + j++; + } + Type *source_type = cast(isize)j < param_count ? params[j]->type : nullptr; + + ft->args[i] = amd64_type(c, arg_types[i], Amd64TypeAttribute_ByVal, calling_convention, + true, + &int_regs, &sse_regs, source_type); + } + + // A single result can be classified from its source type too. A tuple keeps the lowered + // path: it is split into out-pointers below, and C has no such return shape anyway. + Type *return_source = nullptr; + if (return_is_defined && !return_is_tuple && + original_type != nullptr && original_type->kind == Type_Proc && + original_type->Proc.results != nullptr && + original_type->Proc.results->Tuple.variables.count == 1) { + return_source = original_type->Proc.results->Tuple.variables[0]->type; + } + + if (return_source != nullptr) { + ft->ret = amd64_type(c, return_type, Amd64TypeAttribute_StructRect, calling_convention, + false, + nullptr, nullptr, return_source); + } else { + ft->ret = compute_return_type(ft, c, return_type, return_is_defined, return_is_tuple); + } return ft; } @@ -694,8 +725,8 @@ namespace lbAbiAmd64SysV { gb_internal lbArgType amd64_type(LLVMContextRef c, LLVMTypeRef type, Amd64TypeAttributeKind attribute_kind, ProcCallingConvention calling_convention, bool is_arg, - i32 *int_regs, i32 *sse_regs) { - auto cls = classify(type); + i32 *int_regs, i32 *sse_regs, Type *source_type) { + auto cls = classify(type, source_type); i32 needed_int = 0; i32 needed_sse = 0; for (auto c : cls) { @@ -782,15 +813,158 @@ namespace lbAbiAmd64SysV { return lb_arg_type_direct(type, nullptr, nullptr, attr); } - gb_internal Array classify(LLVMTypeRef t) { + // `classify_with` walks the LOWERED type, and lowering has already destroyed two + // distinctions the ABI rules need: Odin materializes padding as an explicit `[N x i8]` + // member, which is indistinguishable from a real `[N]u8` field, and a `#raw_union` becomes + // an opaque integer, which is indistinguishable from a real integer. §3.2.3 says padding + // contributes no class, and that a union merges the classes of all of its members. + // + // The source type still has both, so classify that instead where it is available. Only the + // kinds handled below are eligible; anything else falls back to the lowered walk, so an + // unrecognised type behaves exactly as it did before. + gb_internal bool source_is_classifiable(Type *t) { + Type *bt = base_type(t); + if (bt == nullptr) { + return false; + } + switch (bt->kind) { + case Type_Basic: + switch (bt->Basic.kind) { + case Basic_bool: case Basic_b8: case Basic_b16: case Basic_b32: case Basic_b64: + case Basic_i8: case Basic_u8: case Basic_i16: case Basic_u16: + case Basic_i32: case Basic_u32: case Basic_i64: case Basic_u64: + case Basic_i128: case Basic_u128: case Basic_int: case Basic_uint: + case Basic_uintptr: case Basic_rawptr: case Basic_rune: + case Basic_f16: case Basic_f32: case Basic_f64: + return true; + // Multi-word, but every word of them is a pointer or an integer, so the leaf rule + // below classifies them correctly without knowing their shape. + case Basic_string: case Basic_cstring: case Basic_any: case Basic_typeid: + return true; + } + return false; + case Type_Pointer: + case Type_MultiPointer: + case Type_Proc: + // Integer-backed, or aggregates of pointers and integers. None of them can contain a + // floating-point member, which is the only thing the leaf rule needs to tell apart. + case Type_Enum: + case Type_BitSet: + case Type_Slice: + case Type_DynamicArray: + return true; + case Type_Array: + return source_is_classifiable(bt->Array.elem); + // Odin matrices are laid out with no padding at all; see the note on + // matrix_type_stride_in_bytes + case Type_Matrix: + return source_is_classifiable(bt->Matrix.elem); + case Type_Struct: + if (bt->Struct.is_packed || bt->Struct.soa_kind != StructSoa_None) { + return false; + } + for (Entity *f : bt->Struct.fields) { + if (!source_is_classifiable(f->type)) { + return false; + } + } + return true; + } + return false; + } + + gb_internal void classify_source(Type *t, Array *cls, i64 ix, i64 off) { + Type *bt = base_type(t); + i64 t_size = type_size_of(bt); + i64 t_align = type_align_of(bt); + + if (t_align != 0 && (off % t_align) != 0) { + i64 e = (off + t_size + 7) / 8; + for (i64 i = off / 8; i < e; i++) { + unify(cls, ix+i, RegClass_Memory); + } + return; + } + + switch (bt->kind) { + case Type_Struct: + // A `#raw_union` has every member at offset zero, and §3.2.3 merges them all -- + // which is what makes `union{f32, u32}` INTEGER while `union{f32, f32}` is SSE. + if (bt->Struct.is_raw_union) { + for (Entity *f : bt->Struct.fields) { + classify_source(f->type, cls, ix, off); + } + } else { + for_array(i, bt->Struct.fields) { + Type *ft = nullptr; + i64 foff = type_offset_of(bt, i, &ft); + classify_source(ft, cls, ix, off + foff); + } + } + break; + case Type_Array: { + Type *elem = bt->Array.elem; + i64 stride = type_size_of(elem); + for (i64 i = 0; i < bt->Array.count; i++) { + classify_source(elem, cls, ix, off + i*stride); + } + break; + } + case Type_Matrix: { + Type *elem = bt->Matrix.elem; + i64 stride = type_size_of(elem); + i64 count = matrix_type_total_internal_elems(bt); + for (i64 i = 0; i < count; i++) { + classify_source(elem, cls, ix, off + i*stride); + } + break; + } + default: + if (is_type_float(bt)) { + switch (t_size) { + case 2: unify(cls, ix + off/8, (off%8 != 0) ? RegClass_SSEHv : RegClass_SSEHs); break; + case 4: unify(cls, ix + off/8, (off%8 == 4) ? RegClass_SSEFv : RegClass_SSEFs); break; + default: unify(cls, ix + off/8, RegClass_SSEDs); break; + } + } else { + i64 s = t_size; + while (s > 0) { + unify(cls, ix + off/8, RegClass_Int); + off += 8; + s -= 8; + } + } + break; + } + } + + gb_internal Array classify(LLVMTypeRef t, Type *source_type) { i64 sz = lb_sizeof(t); i64 words = (sz + 7)/8; auto reg_classes = array_make(heap_allocator(), cast(isize)words); if (words > 4) { all_mem(®_classes); } else { - classify_with(t, ®_classes, 0, 0); + bool from_source = source_type != nullptr && source_is_classifiable(source_type) && + type_size_of(base_type(source_type)) == sz; + if (from_source) { + classify_source(source_type, ®_classes, 0, 0); + } else { + classify_with(t, ®_classes, 0, 0); + } fixup(t, ®_classes); + if (from_source) { + // An eightbyte that ends up NO_CLASS is not passed at all. Only the source walk + // can produce one, the lowered walk classifies padding as INTEGER, and + // nothing downstream has a case for it. + // + // This has to come AFTER `fixup`, which counts eightbytes to apply "larger than + // two eightbytes is MEMORY". Dropping them first makes `#align(32){f32}` look + // like a single SSE eightbyte instead of the memory argument it is. + while (reg_classes.count > 0 && reg_classes[reg_classes.count-1] == RegClass_NoClass) { + array_pop(®_classes); + } + } } return reg_classes; } @@ -1125,6 +1299,10 @@ namespace lbAbiAmd64SysV { namespace lbAbiArm64 { gb_internal Array compute_arg_types(LLVMContextRef c, LLVMTypeRef *arg_types, unsigned arg_count, Type* original_type); + gb_internal bool is_register(LLVMTypeRef type); + gb_internal bool is_homogenous_aggregate(LLVMContextRef c, LLVMTypeRef type, LLVMTypeRef *base_type_, unsigned *member_count_); + gb_internal bool is_homogenous_aggregate_source(LLVMContextRef c, Type *t, LLVMTypeRef *base_type_, unsigned *member_count_); + gb_internal unsigned is_homogenous_aggregate_small_enough(LLVMTypeRef base_type, unsigned member_count); gb_internal LB_ABI_COMPUTE_RETURN_TYPE(compute_return_type); gb_internal bool is_homogenous_aggregate(LLVMContextRef c, LLVMTypeRef type, LLVMTypeRef *base_type_, unsigned *member_count_); @@ -1133,7 +1311,30 @@ namespace lbAbiArm64 { lbFunctionType *ft = permanent_alloc_item(); ft->ctx = c; ft->args = compute_arg_types(c, arg_types, arg_count, original_type); - ft->ret = compute_return_type(ft, c, return_type, return_is_defined, return_is_tuple); + + // The same union case as in compute_arg_types, in return position. A tuple keeps the + // lowered path; C has no such return shape, and the split into out-pointers below is + // driven by the lowered type. + Type *return_source = nullptr; + if (return_is_defined && !return_is_tuple && + original_type != nullptr && original_type->kind == Type_Proc && + original_type->Proc.results != nullptr && + original_type->Proc.results->Tuple.variables.count == 1) { + return_source = original_type->Proc.results->Tuple.variables[0]->type; + } + + LLVMTypeRef ret_base_type = nullptr; + unsigned ret_member_count = 0; + if (return_source != nullptr && + !is_register(return_type) && + !is_homogenous_aggregate(c, return_type, nullptr, nullptr) && + is_homogenous_aggregate_source(c, return_source, &ret_base_type, &ret_member_count) && + is_homogenous_aggregate_small_enough(ret_base_type, ret_member_count)) { + ft->ret = lb_arg_type_direct(return_type, llvm_array_type(ret_base_type, ret_member_count), nullptr, nullptr); + } else { + ft->ret = compute_return_type(ft, c, return_type, return_is_defined, return_is_tuple); + } + ft->calling_convention = calling_convention; return ft; } @@ -1261,6 +1462,77 @@ namespace lbAbiArm64 { return false; } + // §5.9.5 defines a Homogeneous Floating-point Aggregate over Composite Types + // Odin lowers `#raw_union` to an opaque integer, so by the time the lowered type is + // inspected the members are gone and `union{f32, f32}` is indistinguishable from an `i32`. + // The source type still has them. + gb_internal bool is_homogenous_aggregate_source(LLVMContextRef c, Type *t, LLVMTypeRef *base_type_, unsigned *member_count_) { + if (t == nullptr) { + return false; + } + Type *bt = base_type(t); + if (bt == nullptr) { + return false; + } + switch (bt->kind) { + case Type_Basic: + switch (bt->Basic.kind) { + case Basic_f32: + if (base_type_) *base_type_ = LLVMFloatTypeInContext(c); + if (member_count_) *member_count_ = 1; + return true; + case Basic_f64: + if (base_type_) *base_type_ = LLVMDoubleTypeInContext(c); + if (member_count_) *member_count_ = 1; + return true; + } + return false; + case Type_Array: { + LLVMTypeRef elem_base = nullptr; + unsigned elem_count = 0; + if (!is_homogenous_aggregate_source(c, bt->Array.elem, &elem_base, &elem_count)) { + return false; + } + if (base_type_) *base_type_ = elem_base; + if (member_count_) *member_count_ = cast(unsigned)(elem_count * bt->Array.count); + return true; + } + case Type_Struct: { + if (bt->Struct.is_packed || bt->Struct.soa_kind != StructSoa_None) { + return false; + } + LLVMTypeRef found_base = nullptr; + unsigned total = 0; + for (Entity *f : bt->Struct.fields) { + LLVMTypeRef field_base = nullptr; + unsigned field_count = 0; + if (!is_homogenous_aggregate_source(c, f->type, &field_base, &field_count)) { + return false; + } + if (found_base == nullptr) { + found_base = field_base; + total = field_count; + } else if (found_base != field_base) { + return false; + } else { + total = bt->Struct.is_raw_union ? gb_max(total, field_count) : total + field_count; + } + } + if (found_base == nullptr) { + return false; + } + // Rejects anything with padding, matching is_homogenous_struct. + if (type_size_of(bt) != lb_sizeof(found_base) * cast(i64)total) { + return false; + } + if (base_type_) *base_type_ = found_base; + if (member_count_) *member_count_ = total; + return true; + } + } + return false; + } + gb_internal unsigned is_homogenous_aggregate_small_enough(LLVMTypeRef base_type, unsigned member_count) { return (member_count <= 4); } @@ -1325,6 +1597,12 @@ namespace lbAbiArm64 { LLVMTypeRef homo_base_type = {}; unsigned homo_member_count = 0; + // A `#raw_union` lowers to a struct wrapping an opaque integer, so it is not a + // homogeneous aggregate by the lowered type and falls through to the generic size + // path below. §5.9.5 counts a union as a Composite Type, so ask the source type. + LLVMTypeRef src_base_type = nullptr; + unsigned src_member_count = 0; + if (is_register(type)) { args[i] = non_struct(c, type, ptype); } else if (is_homogenous_aggregate(c, type, &homo_base_type, &homo_member_count)) { @@ -1333,6 +1611,9 @@ namespace lbAbiArm64 { } else { args[i] = lb_arg_type_indirect(type, nullptr);; } + } else if (is_homogenous_aggregate_source(c, ptype, &src_base_type, &src_member_count) && + is_homogenous_aggregate_small_enough(src_base_type, src_member_count)) { + args[i] = lb_arg_type_direct(type, llvm_array_type(src_base_type, src_member_count), nullptr, nullptr); } else { i64 size = lb_sizeof(type); if (size <= 16) { diff --git a/tests/issues/run.bat b/tests/issues/run.bat index ff02105d7..d01c85e14 100644 --- a/tests/issues/run.bat +++ b/tests/issues/run.bat @@ -43,6 +43,8 @@ set COMMON=-define:ODIN_TEST_FANCY=false -file -vet -strict-style -ignore-unused ..\..\..\odin check ..\test_issue_7012.odin -no-entry-point %COMMON% || exit /b ..\..\..\odin build ..\test_issue_7037.odin %COMMON% -o:none || exit /b ..\..\..\odin build ..\test_issue_7188.odin %COMMON% || exit /b +clang -c ..\test_issue_sysv_abi.c -o test_issue_sysv_abi_c.o || exit /b +..\..\..\odin test ..\test_issue_sysv_abi.odin %COMMON% || exit /b ..\..\..\odin build ..\test_issue_7073-1.odin %COMMON% 2>&1 | find /c "Error:" | findstr /x "2" || exit /b @echo off diff --git a/tests/issues/run.sh b/tests/issues/run.sh index fe4db8a4b..c4dae65f9 100755 --- a/tests/issues/run.sh +++ b/tests/issues/run.sh @@ -110,6 +110,9 @@ fi clang -c ../test_issue_7010.c -o test_issue_7010_c.o $ODIN test ../test_issue_7010.odin $COMMON +clang -c ../test_issue_sysv_abi.c -o test_issue_sysv_abi_c.o +$ODIN test ../test_issue_sysv_abi.odin $COMMON + clang -c ../test_issue_6809_6816.c -o test_issue_6809_6816_c.o -O3 $ODIN test ../test_issue_6809_6816.odin -o:speed $COMMON diff --git a/tests/issues/test_issue_sysv_abi.c b/tests/issues/test_issue_sysv_abi.c new file mode 100644 index 000000000..0c7de35a1 --- /dev/null +++ b/tests/issues/test_issue_sysv_abi.c @@ -0,0 +1,23 @@ +// Support file for test_issue_sysv_abi.odin +// +// Each callee returns its second argument, so the value that comes back says +// where the struct before it went. If the aggregate consumes the wrong number or +// the wrong file of registers, the following argument is read from the wrong +// place deterministically rather than by scratch-register coincidence. + +typedef struct { long a; float b; } Pad_Int_Float; +typedef struct { float a; double b; } Pad_Float_Double; +typedef struct { float a, b; } No_Pad; +typedef struct { struct { float x; } a; double b; } Nested; +typedef union { float x; float y; } Union_Float; +typedef struct { union { float x; float y; } u; double b; } Union_In_Struct; + +double c_pad_int_float (Pad_Int_Float s, double next) { (void)s; return next; } +double c_pad_float_double(Pad_Float_Double s, double next) { (void)s; return next; } +double c_no_pad (No_Pad s, double next) { (void)s; return next; } +double c_nested (Nested s, double next) { (void)s; return next; } +double c_union_float (Union_Float s, double next) { (void)s; return next; } +double c_union_in_struct (Union_In_Struct s, double next) { (void)s; return next; } + +Pad_Int_Float c_make_pad_int_float(void) { Pad_Int_Float s = {11, 2.5f}; return s; } +Union_Float c_make_union_float(void) { Union_Float s; s.x = 2.5f; return s; } diff --git a/tests/issues/test_issue_sysv_abi.odin b/tests/issues/test_issue_sysv_abi.odin new file mode 100644 index 000000000..1be3e70c3 --- /dev/null +++ b/tests/issues/test_issue_sysv_abi.odin @@ -0,0 +1,64 @@ +// The ABI classifiers ran over the lowered type, where Odin has already turned +// padding into an explicit `[N x i8]` member and a `#raw_union` into an opaque +// integer. SysV contributes no class for padding and merges a union's members, +// and AAPCS64 counts a union as a Composite Type. So a struct with an `f32` +// alone in an eightbyte went to an integer register where C uses SSE, and a +// union of floats never reached a floating-point register at all. +// +// Being an ABI guarantee, must be cross-checked against a c compiler +package test_issues + +import "core:testing" + +Pad_Int_Float :: struct { a: i64, b: f32 } // f32 alone in eightbyte 1 +Pad_Float_Double :: struct { a: f32, b: f64 } // f32 alone in eightbyte 0 +No_Pad :: struct { a: f32, b: f32 } // fills its eightbyte exactly +Nested :: struct { a: struct{ x: f32 }, b: f64 } +Union_Float :: struct #raw_union { x: f32, y: f32 } +Union_In_Struct :: struct { u: Union_Float, b: f64 } + +foreign import lib "build/test_issue_sysv_abi_c.o" + +@(default_calling_convention="c") +foreign lib { + c_pad_int_float :: proc(s: Pad_Int_Float, next: f64) -> f64 --- + c_pad_float_double :: proc(s: Pad_Float_Double, next: f64) -> f64 --- + c_no_pad :: proc(s: No_Pad, next: f64) -> f64 --- + c_nested :: proc(s: Nested, next: f64) -> f64 --- + c_union_float :: proc(s: Union_Float, next: f64) -> f64 --- + c_union_in_struct :: proc(s: Union_In_Struct, next: f64) -> f64 --- + + c_make_pad_int_float :: proc() -> Pad_Int_Float --- + c_make_union_float :: proc() -> Union_Float --- +} + +// The control. It has no padding and no union, so it was correct before the fix +// and must stay correct. Without it, "padding is misclassified" and "f32 pairs +// are broken" would look the same. +@(test) +test_no_padding_control :: proc(t: ^testing.T) { + testing.expect_value(t, c_no_pad(No_Pad{1, 3.5}, 7), f64(7)) +} + +@(test) +test_padded_struct_arguments :: proc(t: ^testing.T) { + testing.expect_value(t, c_pad_int_float(Pad_Int_Float{1, 3.5}, 7), f64(7)) + testing.expect_value(t, c_pad_float_double(Pad_Float_Double{3.5, 2}, 7), f64(7)) + testing.expect_value(t, c_nested(Nested{{3.5}, 2}, 7), f64(7)) +} + +@(test) +test_raw_union_arguments :: proc(t: ^testing.T) { + testing.expect_value(t, c_union_float(Union_Float{x = 3.5}, 7), f64(7)) + testing.expect_value(t, c_union_in_struct(Union_In_Struct{Union_Float{x = 3.5}, 2}, 7), f64(7)) +} + +@(test) +test_returns :: proc(t: ^testing.T) { + s := c_make_pad_int_float() + testing.expect_value(t, s.a, i64(11)) + testing.expect_value(t, s.b, f32(2.5)) + + u := c_make_union_float() + testing.expect_value(t, u.x, f32(2.5)) +}