From 125b453c58b6d94a628c94d46e5038e06183c54c Mon Sep 17 00:00:00 2001 From: Jacob Young Date: Fri, 28 Jul 2023 12:48:01 -0400 Subject: [PATCH] llvm: fix SysV C abi for structs smaller than two eightbytes Closes #16038 Closes #16288 --- src/codegen/llvm.zig | 41 +++++++++++++++++++++------- src/codegen/llvm/Builder.zig | 4 ++- src/codegen/llvm/bindings.zig | 3 +++ test/behavior/tuple.zig | 1 + test/c_abi/cfuncs.c | 50 ++++++++++++++++++++++++++++++++--- test/c_abi/main.zig | 40 ++++++++++++++++++++++++++++ test/tests.zig | 2 +- 7 files changed, 126 insertions(+), 15 deletions(-) diff --git a/src/codegen/llvm.zig b/src/codegen/llvm.zig index cf6d1ec3ef..0d314aebdf 100644 --- a/src/codegen/llvm.zig +++ b/src/codegen/llvm.zig @@ -5421,10 +5421,13 @@ pub const FuncGen = struct { // In this case the function return type is honoring the calling convention by having // a different LLVM type than the usual one. We solve this here at the callsite // by using our canonical type, then loading it if necessary. - const alignment = Builder.Alignment.fromByteUnits( + const alignment = Builder.Alignment.fromByteUnits(@max( o.target_data.abiAlignmentOfType(abi_ret_ty.toLlvm(&o.builder)), - ); - const rp = try self.buildAlloca(llvm_ret_ty, alignment); + return_type.abiAlignment(mod), + )); + assert(o.target_data.abiSizeOfType(abi_ret_ty.toLlvm(&o.builder)) >= + o.target_data.abiSizeOfType(llvm_ret_ty.toLlvm(&o.builder))); + const rp = try self.buildAlloca(abi_ret_ty, alignment); _ = try self.wip.store(.normal, call, rp, alignment); return if (isByRef(return_type, mod)) rp @@ -11555,8 +11558,17 @@ fn lowerSystemVFnRetTy(o: *Object, fn_info: InternPool.Key.FuncType) Allocator.E .none => break, } } - if (classes[0] == .integer and classes[1] == .none) { - return o.builder.intType(@intCast(return_type.abiSize(mod) * 8)); + const first_non_integer = std.mem.indexOfNone(x86_64_abi.Class, &classes, &.{.integer}); + if (first_non_integer == null or classes[first_non_integer.?] == .none) { + assert(first_non_integer orelse classes.len == types_index); + if (mod.intern_pool.indexToKey(return_type.toIntern()) == .struct_type) { + var struct_it = return_type.iterateStructOffsets(mod); + while (struct_it.next()) |_| {} + assert((std.math.divCeil(u64, struct_it.offset, 8) catch unreachable) == types_index); + if (struct_it.offset % 8 > 0) types_buffer[types_index - 1] = + try o.builder.intType(@intCast(struct_it.offset % 8 * 8)); + } + if (types_index == 1) return types_buffer[0]; } return o.builder.structType(.normal, types_buffer[0..types_index]); } @@ -11807,10 +11819,21 @@ const ParamTypeIterator = struct { .none => break, } } - if (classes[0] == .integer and classes[1] == .none) { - it.zig_index += 1; - it.llvm_index += 1; - return .abi_sized_int; + const first_non_integer = std.mem.indexOfNone(x86_64_abi.Class, &classes, &.{.integer}); + if (first_non_integer == null or classes[first_non_integer.?] == .none) { + assert(first_non_integer orelse classes.len == types_index); + if (types_index == 1) { + it.zig_index += 1; + it.llvm_index += 1; + return .abi_sized_int; + } + if (mod.intern_pool.indexToKey(ty.toIntern()) == .struct_type) { + var struct_it = ty.iterateStructOffsets(mod); + while (struct_it.next()) |_| {} + assert((std.math.divCeil(u64, struct_it.offset, 8) catch unreachable) == types_index); + if (struct_it.offset % 8 > 0) types_buffer[types_index - 1] = + try it.object.builder.intType(@intCast(struct_it.offset % 8 * 8)); + } } it.types_len = types_index; it.types_buffer = types_buffer; diff --git a/src/codegen/llvm/Builder.zig b/src/codegen/llvm/Builder.zig index 3e6cf31d69..cd7a0045aa 100644 --- a/src/codegen/llvm/Builder.zig +++ b/src/codegen/llvm/Builder.zig @@ -572,7 +572,9 @@ pub const Type = enum(u32) { pub fn isSized(self: Type, builder: *const Builder) Allocator.Error!bool { var visited: IsSizedVisited = .{}; - return self.isSizedVisited(&visited, builder); + const result = try self.isSizedVisited(&visited, builder); + if (builder.useLibLlvm()) assert(result == self.toLlvm(builder).isSized().toBool()); + return result; } const FormatData = struct { diff --git a/src/codegen/llvm/bindings.zig b/src/codegen/llvm/bindings.zig index aee9cea384..2fab61d1cf 100644 --- a/src/codegen/llvm/bindings.zig +++ b/src/codegen/llvm/bindings.zig @@ -434,6 +434,9 @@ pub const Type = opaque { Packed: Bool, ) void; + pub const isSized = LLVMTypeIsSized; + extern fn LLVMTypeIsSized(Ty: *Type) Bool; + pub const constGEP = LLVMConstGEP2; extern fn LLVMConstGEP2( Ty: *Type, diff --git a/test/behavior/tuple.zig b/test/behavior/tuple.zig index 757db0aa16..32ebfe89fd 100644 --- a/test/behavior/tuple.zig +++ b/test/behavior/tuple.zig @@ -459,6 +459,7 @@ test "coerce anon tuple to tuple" { if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; // TODO if (builtin.zig_backend == .stage2_sparc64) return error.SkipZigTest; // TODO if (builtin.zig_backend == .stage2_spirv64) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_wasm) return error.SkipZigTest; var x: u8 = 1; var y: u16 = 2; diff --git a/test/c_abi/cfuncs.c b/test/c_abi/cfuncs.c index 262a3a794e..34aab7f934 100644 --- a/test/c_abi/cfuncs.c +++ b/test/c_abi/cfuncs.c @@ -16,8 +16,12 @@ static void assert_or_panic(bool ok) { # define ZIG_PPC32 #endif -#if defined __riscv && defined _ILP32 -# define ZIG_RISCV32 +#ifdef __riscv +# ifdef _ILP32 +# define ZIG_RISCV32 +# else +# define ZIG_RISCV64 +# endif #endif #if defined(__aarch64__) && defined(__linux__) @@ -191,6 +195,15 @@ struct SmallStructInts { void zig_small_struct_ints(struct SmallStructInts); struct SmallStructInts zig_ret_small_struct_ints(); +struct MedStructInts { + int32_t x; + int32_t y; + int32_t z; +}; + +void zig_med_struct_ints(struct MedStructInts); +struct MedStructInts zig_ret_med_struct_ints(); + struct MedStructMixed { uint32_t a; float b; @@ -339,14 +352,22 @@ void run_c_tests(void) { } #endif -#if !defined __i386__ && !defined __arm__ && !defined __mips__ && \ - !defined ZIG_PPC32 && !defined _ARCH_PPC64 +#if !defined __i386__ && !defined __arm__ && !defined __aarch64__ && \ + !defined __mips__ && !defined __powerpc__ && !defined ZIG_RISCV64 { struct SmallStructInts s = {1, 2, 3, 4}; zig_small_struct_ints(s); } #endif +#if !defined __i386__ && !defined __arm__ && !defined __aarch64__ && \ + !defined __mips__ && !defined __powerpc__ && !defined ZIG_RISCV64 + { + struct MedStructInts s = {1, 2, 3}; + zig_med_struct_ints(s); + } +#endif + #ifndef ZIG_NO_I128 { __int128 s = 0; @@ -586,6 +607,27 @@ struct SmallStructInts c_ret_small_struct_ints() { return s; } +void c_med_struct_ints(struct MedStructInts s) { + assert_or_panic(s.x == 1); + assert_or_panic(s.y == 2); + assert_or_panic(s.z == 3); + + struct MedStructInts s2 = zig_ret_med_struct_ints(); + + assert_or_panic(s2.x == 1); + assert_or_panic(s2.y == 2); + assert_or_panic(s2.z == 3); +} + +struct MedStructInts c_ret_med_struct_ints() { + struct MedStructInts s = { + .x = 1, + .y = 2, + .z = 3, + }; + return s; +} + void c_med_struct_mixed(struct MedStructMixed x) { assert_or_panic(x.a == 1234); assert_or_panic(x.b == 100.0f); diff --git a/test/c_abi/main.zig b/test/c_abi/main.zig index 05f7f060ea..13560b25f6 100644 --- a/test/c_abi/main.zig +++ b/test/c_abi/main.zig @@ -393,6 +393,38 @@ export fn zig_small_struct_ints(x: SmallStructInts) void { expect(x.d == 4) catch @panic("test failure"); } +const MedStructInts = extern struct { + x: i32, + y: i32, + z: i32, +}; +extern fn c_med_struct_ints(MedStructInts) void; +extern fn c_ret_med_struct_ints() MedStructInts; + +test "C ABI medium struct of ints" { + if (builtin.cpu.arch == .x86) return error.SkipZigTest; + if (comptime builtin.cpu.arch.isMIPS()) return error.SkipZigTest; + if (comptime builtin.cpu.arch.isPPC()) return error.SkipZigTest; + if (comptime builtin.cpu.arch.isPPC64()) return error.SkipZigTest; + + var s = MedStructInts{ + .x = 1, + .y = 2, + .z = 3, + }; + c_med_struct_ints(s); + var s2 = c_ret_med_struct_ints(); + try expect(s2.x == 1); + try expect(s2.y == 2); + try expect(s2.z == 3); +} + +export fn zig_med_struct_ints(s: MedStructInts) void { + expect(s.x == 1) catch @panic("test failure"); + expect(s.y == 2) catch @panic("test failure"); + expect(s.z == 3) catch @panic("test failure"); +} + const SmallPackedStruct = packed struct { a: u2, b: u2, @@ -691,6 +723,14 @@ export fn zig_ret_small_struct_ints() SmallStructInts { }; } +export fn zig_ret_med_struct_ints() MedStructInts { + return .{ + .x = 1, + .y = 2, + .z = 3, + }; +} + export fn zig_ret_med_struct_mixed() MedStructMixed { return .{ .a = 1234, diff --git a/test/tests.zig b/test/tests.zig index b76f7baf84..bba1b00142 100644 --- a/test/tests.zig +++ b/test/tests.zig @@ -1102,7 +1102,7 @@ pub fn addModuleTests(b: *std.Build, options: ModuleTestOptions) *Step { pub fn addCAbiTests(b: *std.Build, skip_non_native: bool, skip_release: bool) *Step { const step = b.step("test-c-abi", "Run the C ABI tests"); - const optimize_modes: [2]OptimizeMode = .{ .Debug, .ReleaseFast }; + const optimize_modes: [3]OptimizeMode = .{ .Debug, .ReleaseSafe, .ReleaseFast }; for (optimize_modes) |optimize_mode| { if (optimize_mode != .Debug and skip_release) continue;