Skip to content

Commit 58ac008

Browse files
gwenzekVexu
authored and
andrewrk
committed
compute LLVMTypes in ParamTypeIterator (#13592)
follow up on #13376 - fixes a bug in the x86_64 C ABI. Co-authored-by: Veikka Tuominen <[email protected]>
1 parent 4428e38 commit 58ac008

File tree

3 files changed

+77
-137
lines changed

3 files changed

+77
-137
lines changed

‎src/arch/x86_64/abi.zig

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -507,3 +507,48 @@ pub const RegisterClass = struct {
507507
break :blk set;
508508
};
509509
};
510+
511+
const testing = std.testing;
512+
const Module = @import("../../Module.zig");
513+
const Value = @import("../../value.zig").Value;
514+
const builtin = @import("builtin");
515+
516+
fn _field(comptime tag: Type.Tag, offset: u32) Module.Struct.Field {
517+
return .{
518+
.ty = Type.initTag(tag),
519+
.default_val = Value.initTag(.unreachable_value),
520+
.abi_align = 0,
521+
.offset = offset,
522+
.is_comptime = false,
523+
};
524+
}
525+
526+
test "C_C_D" {
527+
var fields = Module.Struct.Fields{};
528+
// const C_C_D = extern struct { v1: i8, v2: i8, v3: f64 };
529+
try fields.ensureTotalCapacity(testing.allocator, 3);
530+
defer fields.deinit(testing.allocator);
531+
fields.putAssumeCapacity("v1", _field(.i8, 0));
532+
fields.putAssumeCapacity("v2", _field(.i8, 1));
533+
fields.putAssumeCapacity("v3", _field(.f64, 4));
534+
535+
var C_C_D_struct = Module.Struct{
536+
.fields = fields,
537+
.namespace = undefined,
538+
.owner_decl = undefined,
539+
.zir_index = undefined,
540+
.layout = .Extern,
541+
.status = .fully_resolved,
542+
.known_non_opv = true,
543+
};
544+
var C_C_D = Type.Payload.Struct{ .data = &C_C_D_struct };
545+
546+
try testing.expectEqual(
547+
[_]Class{ .integer, .sse, .none, .none, .none, .none, .none, .none },
548+
classifySystemV(Type.initPayload(&C_C_D.base), builtin.target, .ret),
549+
);
550+
try testing.expectEqual(
551+
[_]Class{ .integer, .sse, .none, .none, .none, .none, .none, .none },
552+
classifySystemV(Type.initPayload(&C_C_D.base), builtin.target, .arg),
553+
);
554+
}

‎src/codegen/llvm.zig

Lines changed: 28 additions & 125 deletions
Original file line numberDiff line numberDiff line change
@@ -1049,60 +1049,20 @@ pub const Object = struct {
10491049
const aggregate = builder.buildInsertValue(partial, len_param, 1, "");
10501050
try args.append(aggregate);
10511051
},
1052-
.multiple_llvm_ints => {
1052+
.multiple_llvm_types => {
10531053
assert(!it.byval_attr);
1054-
const llvm_ints = it.llvm_types_buffer[0..it.llvm_types_len];
1054+
const field_types = it.llvm_types_buffer[0..it.llvm_types_len];
10551055
const param_ty = fn_info.param_types[it.zig_index - 1];
10561056
const param_llvm_ty = try dg.lowerType(param_ty);
10571057
const param_alignment = param_ty.abiAlignment(target);
10581058
const arg_ptr = buildAllocaInner(builder, llvm_func, false, param_llvm_ty, param_alignment, target);
1059-
var field_types_buf: [8]*llvm.Type = undefined;
1060-
const field_types = field_types_buf[0..llvm_ints.len];
1061-
for (llvm_ints) |int_bits, i| {
1062-
field_types[i] = dg.context.intType(int_bits);
1063-
}
1064-
const ints_llvm_ty = dg.context.structType(field_types.ptr, @intCast(c_uint, field_types.len), .False);
1065-
const casted_ptr = builder.buildBitCast(arg_ptr, ints_llvm_ty.pointerType(0), "");
1066-
for (llvm_ints) |_, field_i_usize| {
1059+
const llvm_ty = dg.context.structType(field_types.ptr, @intCast(c_uint, field_types.len), .False);
1060+
const casted_ptr = builder.buildBitCast(arg_ptr, llvm_ty.pointerType(0), "");
1061+
for (field_types) |_, field_i_usize| {
10671062
const field_i = @intCast(c_uint, field_i_usize);
10681063
const param = llvm_func.getParam(llvm_arg_i);
10691064
llvm_arg_i += 1;
1070-
const field_ptr = builder.buildStructGEP(ints_llvm_ty, casted_ptr, field_i, "");
1071-
const store_inst = builder.buildStore(param, field_ptr);
1072-
store_inst.setAlignment(target.cpu.arch.ptrBitWidth() / 8);
1073-
}
1074-
1075-
const is_by_ref = isByRef(param_ty);
1076-
const loaded = if (is_by_ref) arg_ptr else l: {
1077-
const load_inst = builder.buildLoad(param_llvm_ty, arg_ptr, "");
1078-
load_inst.setAlignment(param_alignment);
1079-
break :l load_inst;
1080-
};
1081-
try args.append(loaded);
1082-
},
1083-
.multiple_llvm_float => {
1084-
assert(!it.byval_attr);
1085-
const llvm_floats = it.llvm_types_buffer[0..it.llvm_types_len];
1086-
const param_ty = fn_info.param_types[it.zig_index - 1];
1087-
const param_llvm_ty = try dg.lowerType(param_ty);
1088-
const param_alignment = param_ty.abiAlignment(target);
1089-
const arg_ptr = buildAllocaInner(builder, llvm_func, false, param_llvm_ty, param_alignment, target);
1090-
var field_types_buf: [8]*llvm.Type = undefined;
1091-
const field_types = field_types_buf[0..llvm_floats.len];
1092-
for (llvm_floats) |float_bits, i| {
1093-
switch (float_bits) {
1094-
64 => field_types[i] = dg.context.doubleType(),
1095-
80 => field_types[i] = dg.context.x86FP80Type(),
1096-
else => {},
1097-
}
1098-
}
1099-
const floats_llvm_ty = dg.context.structType(field_types.ptr, @intCast(c_uint, field_types.len), .False);
1100-
const casted_ptr = builder.buildBitCast(arg_ptr, floats_llvm_ty.pointerType(0), "");
1101-
for (llvm_floats) |_, field_i_usize| {
1102-
const field_i = @intCast(c_uint, field_i_usize);
1103-
const param = llvm_func.getParam(llvm_arg_i);
1104-
llvm_arg_i += 1;
1105-
const field_ptr = builder.buildStructGEP(floats_llvm_ty, casted_ptr, field_i, "");
1065+
const field_ptr = builder.buildStructGEP(llvm_ty, casted_ptr, field_i, "");
11061066
const store_inst = builder.buildStore(param, field_ptr);
11071067
store_inst.setAlignment(target.cpu.arch.ptrBitWidth() / 8);
11081068
}
@@ -2626,8 +2586,7 @@ pub const DeclGen = struct {
26262586
// No attributes needed for these.
26272587
.no_bits,
26282588
.abi_sized_int,
2629-
.multiple_llvm_ints,
2630-
.multiple_llvm_float,
2589+
.multiple_llvm_types,
26312590
.as_u16,
26322591
.float_array,
26332592
.i32_array,
@@ -3167,25 +3126,8 @@ pub const DeclGen = struct {
31673126
llvm_params.appendAssumeCapacity(ptr_llvm_ty);
31683127
llvm_params.appendAssumeCapacity(len_llvm_ty);
31693128
},
3170-
.multiple_llvm_ints => {
3171-
const llvm_ints = it.llvm_types_buffer[0..it.llvm_types_len];
3172-
try llvm_params.ensureUnusedCapacity(it.llvm_types_len);
3173-
for (llvm_ints) |int_bits| {
3174-
const big_int_ty = dg.context.intType(int_bits);
3175-
llvm_params.appendAssumeCapacity(big_int_ty);
3176-
}
3177-
},
3178-
.multiple_llvm_float => {
3179-
const llvm_ints = it.llvm_types_buffer[0..it.llvm_types_len];
3180-
try llvm_params.ensureUnusedCapacity(it.llvm_types_len);
3181-
for (llvm_ints) |float_bits| {
3182-
const float_ty = switch (float_bits) {
3183-
64 => dg.context.doubleType(),
3184-
80 => dg.context.x86FP80Type(),
3185-
else => unreachable,
3186-
};
3187-
llvm_params.appendAssumeCapacity(float_ty);
3188-
}
3129+
.multiple_llvm_types => {
3130+
try llvm_params.appendSlice(it.llvm_types_buffer[0..it.llvm_types_len]);
31893131
},
31903132
.as_u16 => {
31913133
try llvm_params.append(dg.context.intType(16));
@@ -4824,10 +4766,10 @@ pub const FuncGen = struct {
48244766
llvm_args.appendAssumeCapacity(ptr);
48254767
llvm_args.appendAssumeCapacity(len);
48264768
},
4827-
.multiple_llvm_ints => {
4769+
.multiple_llvm_types => {
48284770
const arg = args[it.zig_index - 1];
48294771
const param_ty = self.air.typeOf(arg);
4830-
const llvm_ints = it.llvm_types_buffer[0..it.llvm_types_len];
4772+
const llvm_types = it.llvm_types_buffer[0..it.llvm_types_len];
48314773
const llvm_arg = try self.resolveInst(arg);
48324774
const is_by_ref = isByRef(param_ty);
48334775
const arg_ptr = if (is_by_ref) llvm_arg else p: {
@@ -4837,51 +4779,13 @@ pub const FuncGen = struct {
48374779
break :p p;
48384780
};
48394781

4840-
var field_types_buf: [8]*llvm.Type = undefined;
4841-
const field_types = field_types_buf[0..llvm_ints.len];
4842-
for (llvm_ints) |int_bits, i| {
4843-
field_types[i] = self.dg.context.intType(int_bits);
4844-
}
4845-
const ints_llvm_ty = self.dg.context.structType(field_types.ptr, @intCast(c_uint, field_types.len), .False);
4846-
const casted_ptr = self.builder.buildBitCast(arg_ptr, ints_llvm_ty.pointerType(0), "");
4782+
const llvm_ty = self.dg.context.structType(llvm_types.ptr, @intCast(c_uint, llvm_types.len), .False);
4783+
const casted_ptr = self.builder.buildBitCast(arg_ptr, llvm_ty.pointerType(0), "");
48474784
try llvm_args.ensureUnusedCapacity(it.llvm_types_len);
4848-
for (llvm_ints) |_, i_usize| {
4785+
for (llvm_types) |field_ty, i_usize| {
48494786
const i = @intCast(c_uint, i_usize);
4850-
const field_ptr = self.builder.buildStructGEP(ints_llvm_ty, casted_ptr, i, "");
4851-
const load_inst = self.builder.buildLoad(field_types[i], field_ptr, "");
4852-
load_inst.setAlignment(target.cpu.arch.ptrBitWidth() / 8);
4853-
llvm_args.appendAssumeCapacity(load_inst);
4854-
}
4855-
},
4856-
.multiple_llvm_float => {
4857-
const arg = args[it.zig_index - 1];
4858-
const param_ty = self.air.typeOf(arg);
4859-
const llvm_floats = it.llvm_types_buffer[0..it.llvm_types_len];
4860-
const llvm_arg = try self.resolveInst(arg);
4861-
const is_by_ref = isByRef(param_ty);
4862-
const arg_ptr = if (is_by_ref) llvm_arg else p: {
4863-
const p = self.buildAlloca(llvm_arg.typeOf(), null);
4864-
const store_inst = self.builder.buildStore(llvm_arg, p);
4865-
store_inst.setAlignment(param_ty.abiAlignment(target));
4866-
break :p p;
4867-
};
4868-
4869-
var field_types_buf: [8]*llvm.Type = undefined;
4870-
const field_types = field_types_buf[0..llvm_floats.len];
4871-
for (llvm_floats) |float_bits, i| {
4872-
switch (float_bits) {
4873-
64 => field_types[i] = self.dg.context.doubleType(),
4874-
80 => field_types[i] = self.dg.context.x86FP80Type(),
4875-
else => {},
4876-
}
4877-
}
4878-
const floats_llvm_ty = self.dg.context.structType(field_types.ptr, @intCast(c_uint, field_types.len), .False);
4879-
const casted_ptr = self.builder.buildBitCast(arg_ptr, floats_llvm_ty.pointerType(0), "");
4880-
try llvm_args.ensureUnusedCapacity(it.llvm_types_len);
4881-
for (llvm_floats) |_, i_usize| {
4882-
const i = @intCast(c_uint, i_usize);
4883-
const field_ptr = self.builder.buildStructGEP(floats_llvm_ty, casted_ptr, i, "");
4884-
const load_inst = self.builder.buildLoad(field_types[i], field_ptr, "");
4787+
const field_ptr = self.builder.buildStructGEP(llvm_ty, casted_ptr, i, "");
4788+
const load_inst = self.builder.buildLoad(field_ty, field_ptr, "");
48854789
load_inst.setAlignment(target.cpu.arch.ptrBitWidth() / 8);
48864790
llvm_args.appendAssumeCapacity(load_inst);
48874791
}
@@ -10473,16 +10377,15 @@ const ParamTypeIterator = struct {
1047310377
llvm_index: u32,
1047410378
target: std.Target,
1047510379
llvm_types_len: u32,
10476-
llvm_types_buffer: [8]u16,
10380+
llvm_types_buffer: [8]*llvm.Type,
1047710381
byval_attr: bool,
1047810382

1047910383
const Lowering = union(enum) {
1048010384
no_bits,
1048110385
byval,
1048210386
byref,
1048310387
abi_sized_int,
10484-
multiple_llvm_ints,
10485-
multiple_llvm_float,
10388+
multiple_llvm_types,
1048610389
slice,
1048710390
as_u16,
1048810391
float_array: u8,
@@ -10515,7 +10418,7 @@ const ParamTypeIterator = struct {
1051510418
it.zig_index += 1;
1051610419
return .no_bits;
1051710420
}
10518-
10421+
const dg = it.dg;
1051910422
switch (it.fn_info.cc) {
1052010423
.Unspecified, .Inline => {
1052110424
it.zig_index += 1;
@@ -10584,28 +10487,28 @@ const ParamTypeIterator = struct {
1058410487
it.llvm_index += 1;
1058510488
return .byval;
1058610489
}
10587-
var llvm_types_buffer: [8]u16 = undefined;
10490+
var llvm_types_buffer: [8]*llvm.Type = undefined;
1058810491
var llvm_types_index: u32 = 0;
1058910492
for (classes) |class| {
1059010493
switch (class) {
1059110494
.integer => {
10592-
llvm_types_buffer[llvm_types_index] = 64;
10495+
llvm_types_buffer[llvm_types_index] = dg.context.intType(64);
1059310496
llvm_types_index += 1;
1059410497
},
1059510498
.sse => {
10596-
llvm_types_buffer[llvm_types_index] = 64;
10499+
llvm_types_buffer[llvm_types_index] = dg.context.doubleType();
1059710500
llvm_types_index += 1;
1059810501
},
1059910502
.sseup => {
10600-
llvm_types_buffer[llvm_types_index] = 64;
10503+
llvm_types_buffer[llvm_types_index] = dg.context.doubleType();
1060110504
llvm_types_index += 1;
1060210505
},
1060310506
.x87 => {
10604-
llvm_types_buffer[llvm_types_index] = 80;
10507+
llvm_types_buffer[llvm_types_index] = dg.context.x86FP80Type();
1060510508
llvm_types_index += 1;
1060610509
},
1060710510
.x87up => {
10608-
llvm_types_buffer[llvm_types_index] = 80;
10511+
llvm_types_buffer[llvm_types_index] = dg.context.x86FP80Type();
1060910512
llvm_types_index += 1;
1061010513
},
1061110514
.complex_x87 => {
@@ -10625,7 +10528,7 @@ const ParamTypeIterator = struct {
1062510528
it.llvm_types_len = llvm_types_index;
1062610529
it.llvm_index += llvm_types_index;
1062710530
it.zig_index += 1;
10628-
return if (classes[0] == .integer) .multiple_llvm_ints else .multiple_llvm_float;
10531+
return .multiple_llvm_types;
1062910532
},
1063010533
},
1063110534
.wasm32 => {
@@ -10649,8 +10552,8 @@ const ParamTypeIterator = struct {
1064910552
.byval => return .byval,
1065010553
.integer => {
1065110554
it.llvm_types_len = 1;
10652-
it.llvm_types_buffer[0] = 64;
10653-
return .multiple_llvm_ints;
10555+
it.llvm_types_buffer[0] = dg.context.intType(64);
10556+
return .multiple_llvm_types;
1065410557
},
1065510558
.double_integer => return Lowering{ .i64_array = 2 },
1065610559
}

‎test/c_abi/main.zig

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -851,8 +851,6 @@ pub inline fn expectOk(c_err: c_int) !void {
851851
/// Tests for Double + Char struct
852852
const DC = extern struct { v1: f64, v2: u8 };
853853
test "DC: Zig passes to C" {
854-
if (builtin.target.cpu.arch == .x86_64 and builtin.target.os.tag != .windows)
855-
return error.SkipZigTest;
856854
if (comptime builtin.cpu.arch.isMIPS()) return error.SkipZigTest;
857855
if (comptime builtin.cpu.arch.isRISCV()) return error.SkipZigTest;
858856
if (comptime builtin.cpu.arch.isPPC()) return error.SkipZigTest;
@@ -866,8 +864,6 @@ test "DC: Zig returns to C" {
866864
try expectOk(c_assert_ret_DC());
867865
}
868866
test "DC: C passes to Zig" {
869-
if (builtin.target.cpu.arch == .x86_64 and builtin.target.os.tag != .windows)
870-
return error.SkipZigTest;
871867
if (comptime builtin.cpu.arch.isMIPS()) return error.SkipZigTest;
872868
if (comptime builtin.cpu.arch.isRISCV()) return error.SkipZigTest;
873869
if (comptime builtin.cpu.arch.isPPC()) return error.SkipZigTest;
@@ -900,8 +896,7 @@ pub export fn zig_ret_DC() DC {
900896
const CFF = extern struct { v1: u8, v2: f32, v3: f32 };
901897

902898
test "CFF: Zig passes to C" {
903-
if (builtin.target.cpu.arch.isX86() and builtin.target.os.tag != .windows)
904-
return error.SkipZigTest;
899+
if (builtin.target.cpu.arch == .x86) return error.SkipZigTest;
905900
if (comptime builtin.cpu.arch.isMIPS()) return error.SkipZigTest;
906901
if (comptime builtin.cpu.arch.isPPC()) return error.SkipZigTest;
907902
if (comptime builtin.cpu.arch.isPPC64()) return error.SkipZigTest;
@@ -914,8 +909,7 @@ test "CFF: Zig returns to C" {
914909
try expectOk(c_assert_ret_CFF());
915910
}
916911
test "CFF: C passes to Zig" {
917-
if (builtin.target.cpu.arch.isX86() and builtin.target.os.tag != .windows)
918-
return error.SkipZigTest;
912+
if (builtin.target.cpu.arch == .x86) return error.SkipZigTest;
919913
if (comptime builtin.cpu.arch.isMIPS()) return error.SkipZigTest;
920914
if (comptime builtin.cpu.arch.isPPC()) return error.SkipZigTest;
921915
if (comptime builtin.cpu.arch.isPPC64()) return error.SkipZigTest;
@@ -950,8 +944,7 @@ pub export fn zig_ret_CFF() CFF {
950944
const PD = extern struct { v1: ?*anyopaque, v2: f64 };
951945

952946
test "PD: Zig passes to C" {
953-
if (builtin.target.cpu.arch.isX86() and builtin.target.os.tag != .windows)
954-
return error.SkipZigTest;
947+
if (builtin.target.cpu.arch == .x86) return error.SkipZigTest;
955948
if (comptime builtin.cpu.arch.isMIPS()) return error.SkipZigTest;
956949
if (comptime builtin.cpu.arch.isPPC()) return error.SkipZigTest;
957950
if (comptime builtin.cpu.arch.isPPC64()) return error.SkipZigTest;
@@ -964,8 +957,7 @@ test "PD: Zig returns to C" {
964957
try expectOk(c_assert_ret_PD());
965958
}
966959
test "PD: C passes to Zig" {
967-
if (builtin.target.cpu.arch.isX86() and builtin.target.os.tag != .windows)
968-
return error.SkipZigTest;
960+
if (builtin.target.cpu.arch == .x86) return error.SkipZigTest;
969961
if (comptime builtin.cpu.arch.isMIPS()) return error.SkipZigTest;
970962
if (comptime builtin.cpu.arch.isPPC()) return error.SkipZigTest;
971963
if (comptime builtin.cpu.arch.isPPC64()) return error.SkipZigTest;

0 commit comments

Comments
 (0)