Commit 125b453c58

Jacob Young <jacobly0@users.noreply.github.com>
2023-07-28 18:48:01
llvm: fix SysV C abi for structs smaller than two eightbytes
Closes #16038 Closes #16288
1 parent c80609d
Changed files (7)
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,
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 {
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;
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;
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);
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,
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;