Skip to content

packed struct fixes #16611

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions src/arch/wasm/CodeGen.zig
Original file line number Diff line number Diff line change
Expand Up @@ -3675,18 +3675,20 @@ fn airStructFieldPtr(func: *CodeGen, inst: Air.Inst.Index) InnerError!void {
const extra = func.air.extraData(Air.StructField, ty_pl.payload);

const struct_ptr = try func.resolveInst(extra.data.struct_operand);
const struct_ty = func.typeOf(extra.data.struct_operand).childType(mod);
const result = try func.structFieldPtr(inst, extra.data.struct_operand, struct_ptr, struct_ty, extra.data.field_index);
const struct_ptr_ty = func.typeOf(extra.data.struct_operand);
const struct_ty = struct_ptr_ty.childType(mod);
const result = try func.structFieldPtr(inst, extra.data.struct_operand, struct_ptr, struct_ptr_ty, struct_ty, extra.data.field_index);
func.finishAir(inst, result, &.{extra.data.struct_operand});
}

fn airStructFieldPtrIndex(func: *CodeGen, inst: Air.Inst.Index, index: u32) InnerError!void {
const mod = func.bin_file.base.options.module.?;
const ty_op = func.air.instructions.items(.data)[inst].ty_op;
const struct_ptr = try func.resolveInst(ty_op.operand);
const struct_ty = func.typeOf(ty_op.operand).childType(mod);
const struct_ptr_ty = func.typeOf(ty_op.operand);
const struct_ty = struct_ptr_ty.childType(mod);

const result = try func.structFieldPtr(inst, ty_op.operand, struct_ptr, struct_ty, index);
const result = try func.structFieldPtr(inst, ty_op.operand, struct_ptr, struct_ptr_ty, struct_ty, index);
func.finishAir(inst, result, &.{ty_op.operand});
}

Expand All @@ -3695,18 +3697,21 @@ fn structFieldPtr(
inst: Air.Inst.Index,
ref: Air.Inst.Ref,
struct_ptr: WValue,
struct_ptr_ty: Type,
struct_ty: Type,
index: u32,
) InnerError!WValue {
const mod = func.bin_file.base.options.module.?;
const result_ty = func.typeOfIndex(inst);
const struct_ptr_ty_info = struct_ptr_ty.ptrInfo(mod);

const offset = switch (struct_ty.containerLayout(mod)) {
.Packed => switch (struct_ty.zigTypeTag(mod)) {
.Struct => offset: {
if (result_ty.ptrInfo(mod).packed_offset.host_size != 0) {
break :offset @as(u32, 0);
}
break :offset struct_ty.packedStructFieldByteOffset(index, mod);
break :offset struct_ty.packedStructFieldByteOffset(index, mod) + @divExact(struct_ptr_ty_info.packed_offset.bit_offset, 8);
},
.Union => 0,
else => unreachable,
Expand Down
4 changes: 3 additions & 1 deletion src/arch/x86_64/CodeGen.zig
Original file line number Diff line number Diff line change
Expand Up @@ -5556,12 +5556,14 @@ fn fieldPtr(self: *Self, inst: Air.Inst.Index, operand: Air.Inst.Ref, index: u32
const mod = self.bin_file.options.module.?;
const ptr_field_ty = self.typeOfIndex(inst);
const ptr_container_ty = self.typeOf(operand);
const ptr_container_ty_info = ptr_container_ty.ptrInfo(mod);
const container_ty = ptr_container_ty.childType(mod);

const field_offset: i32 = @intCast(switch (container_ty.containerLayout(mod)) {
.Auto, .Extern => container_ty.structFieldOffset(index, mod),
.Packed => if (container_ty.zigTypeTag(mod) == .Struct and
ptr_field_ty.ptrInfo(mod).packed_offset.host_size == 0)
container_ty.packedStructFieldByteOffset(index, mod)
container_ty.packedStructFieldByteOffset(index, mod) + @divExact(ptr_container_ty_info.packed_offset.bit_offset, 8)
else
0,
});
Expand Down
11 changes: 6 additions & 5 deletions src/codegen/c.zig
Original file line number Diff line number Diff line change
Expand Up @@ -661,7 +661,7 @@ pub const DeclGen = struct {
try dg.renderCType(writer, ptr_cty);
try writer.writeByte(')');
}
switch (fieldLocation(base_ty, ptr_ty, @as(u32, @intCast(field.index)), mod)) {
switch (fieldLocation(ptr_base_ty, ptr_ty, @as(u32, @intCast(field.index)), mod)) {
.begin => try dg.renderParentPtr(writer, field.base, location),
.field => |name| {
try writer.writeAll("&(");
Expand Down Expand Up @@ -5187,7 +5187,7 @@ fn airOptionalPayloadPtrSet(f: *Function, inst: Air.Inst.Index) !CValue {
}

fn fieldLocation(
container_ty: Type,
container_ptr_ty: Type,
field_ptr_ty: Type,
field_index: u32,
mod: *Module,
Expand All @@ -5198,6 +5198,7 @@ fn fieldLocation(
end: void,
} {
const ip = &mod.intern_pool;
const container_ty = container_ptr_ty.childType(mod);
return switch (container_ty.zigTypeTag(mod)) {
.Struct => switch (container_ty.containerLayout(mod)) {
.Auto, .Extern => for (field_index..container_ty.structFieldCount(mod)) |next_field_index| {
Expand All @@ -5211,7 +5212,7 @@ fn fieldLocation(
.{ .identifier = ip.stringToSlice(container_ty.structFieldName(next_field_index, mod)) } };
} else if (container_ty.hasRuntimeBitsIgnoreComptime(mod)) .end else .begin,
.Packed => if (field_ptr_ty.ptrInfo(mod).packed_offset.host_size == 0)
.{ .byte_offset = container_ty.packedStructFieldByteOffset(field_index, mod) }
.{ .byte_offset = container_ty.packedStructFieldByteOffset(field_index, mod) + @divExact(container_ptr_ty.ptrInfo(mod).packed_offset.bit_offset, 8) }
else
.begin,
},
Expand Down Expand Up @@ -5282,7 +5283,7 @@ fn airFieldParentPtr(f: *Function, inst: Air.Inst.Index) !CValue {
try f.renderType(writer, container_ptr_ty);
try writer.writeByte(')');

switch (fieldLocation(container_ty, field_ptr_ty, extra.field_index, mod)) {
switch (fieldLocation(container_ptr_ty, field_ptr_ty, extra.field_index, mod)) {
.begin => try f.writeCValue(writer, field_ptr_val, .Initializer),
.field => |field| {
const u8_ptr_ty = try mod.adjustPtrTypeChild(field_ptr_ty, Type.u8);
Expand Down Expand Up @@ -5339,7 +5340,7 @@ fn fieldPtr(
try f.renderType(writer, field_ptr_ty);
try writer.writeByte(')');

switch (fieldLocation(container_ty, field_ptr_ty, field_index, mod)) {
switch (fieldLocation(container_ptr_ty, field_ptr_ty, field_index, mod)) {
.begin => try f.writeCValue(writer, container_ptr_val, .Initializer),
.field => |field| {
try writer.writeByte('&');
Expand Down
12 changes: 11 additions & 1 deletion src/codegen/llvm.zig
Original file line number Diff line number Diff line change
Expand Up @@ -8753,6 +8753,15 @@ pub const FuncGen = struct {

const val_is_undef = if (try self.air.value(bin_op.rhs, mod)) |val| val.isUndefDeep(mod) else false;
if (val_is_undef) {
const ptr_info = ptr_ty.ptrInfo(mod);
const needs_bitmask = (ptr_info.packed_offset.host_size != 0);
if (needs_bitmask) {
// TODO: only some bits are to be undef, we cannot write with a simple memset.
// meanwhile, ignore the write rather than stomping over valid bits.
// https://github.com/ziglang/zig/issues/15337
return .none;
}

// Even if safety is disabled, we still emit a memset to undefined since it conveys
// extra information to LLVM. However, safety makes the difference between using
// 0xaa or actual undefined for the fill byte.
Expand Down Expand Up @@ -10602,6 +10611,7 @@ pub const FuncGen = struct {
.Packed => {
const result_ty = self.typeOfIndex(inst);
const result_ty_info = result_ty.ptrInfo(mod);
const struct_ptr_ty_info = struct_ptr_ty.ptrInfo(mod);

if (result_ty_info.packed_offset.host_size != 0) {
// From LLVM's perspective, a pointer to a packed struct and a pointer
Expand All @@ -10613,7 +10623,7 @@ pub const FuncGen = struct {

// We have a pointer to a packed struct field that happens to be byte-aligned.
// Offset our operand pointer by the correct number of bytes.
const byte_offset = struct_ty.packedStructFieldByteOffset(field_index, mod);
const byte_offset = struct_ty.packedStructFieldByteOffset(field_index, mod) + @divExact(struct_ptr_ty_info.packed_offset.bit_offset, 8);
if (byte_offset == 0) return struct_ptr;
const usize_ty = try o.lowerType(Type.usize);
const llvm_index = try o.builder.intValue(usize_ty, byte_offset);
Expand Down
95 changes: 94 additions & 1 deletion test/behavior/packed-struct.zig
Original file line number Diff line number Diff line change
Expand Up @@ -528,6 +528,100 @@ test "nested packed struct field access test" {
try std.testing.expect(arg.g.i == 8);
}

test "nested packed struct at non-zero offset" {
if (builtin.zig_backend == .stage2_sparc64) return error.SkipZigTest; // TODO
if (builtin.zig_backend == .stage2_spirv64) return error.SkipZigTest;

const Pair = packed struct(u24) {
a: u16 = 0,
b: u8 = 0,
};
const A = packed struct {
p1: Pair,
p2: Pair,
};

var k: u8 = 123;
var v: A = .{
.p1 = .{ .a = k + 1, .b = k },
.p2 = .{ .a = k + 1, .b = k },
};

try expect(v.p1.a == k + 1 and v.p1.b == k);
try expect(v.p2.a == k + 1 and v.p2.b == k);

v.p2.a -= v.p1.a;
v.p2.b -= v.p1.b;
try expect(v.p2.a == 0 and v.p2.b == 0);
try expect(v.p1.a == k + 1 and v.p1.b == k);
}

test "nested packed struct at non-zero offset 2" {
if (builtin.zig_backend == .stage2_x86_64) return error.SkipZigTest; // TODO
if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest;
if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest;
if (builtin.zig_backend == .stage2_sparc64) return error.SkipZigTest; // TODO
if (builtin.zig_backend == .stage2_spirv64) return error.SkipZigTest;
if (builtin.zig_backend == .stage2_wasm) return error.SkipZigTest; // TODO
if (builtin.zig_backend == .stage2_c) return error.SkipZigTest;

const S = struct {
const Pair = packed struct(u40) {
a: u32 = 0,
b: u8 = 0,
};
const A = packed struct {
p1: Pair,
p2: Pair,
c: C,
};
const C = packed struct {
p1: Pair,
pad1: u5,
p2: Pair,
pad2: u3,
last: i16,
};

fn doTheTest() !void {
var k: u8 = 123;
var v: A = .{
.p1 = .{ .a = k + 1, .b = k },
.p2 = .{ .a = k + 1, .b = k },
.c = .{
.pad1 = 11,
.pad2 = 2,
.p1 = .{ .a = k + 1, .b = k },
.p2 = .{ .a = k + 1, .b = k },
.last = -12345,
},
};

try expect(v.p1.a == k + 1 and v.p1.b == k);
try expect(v.p2.a == k + 1 and v.p2.b == k);
try expect(v.c.p2.a == k + 1 and v.c.p2.b == k);
try expect(v.c.p2.a == k + 1 and v.c.p2.b == k);
try expect(v.c.last == -12345);
try expect(v.c.pad1 == 11 and v.c.pad2 == 2);

v.p2.a -= v.p1.a;
v.p2.b -= v.p1.b;
v.c.p2.a -= v.c.p1.a;
v.c.p2.b -= v.c.p1.b;
v.c.last -|= 32000;
try expect(v.p2.a == 0 and v.p2.b == 0);
try expect(v.p1.a == k + 1 and v.p1.b == k);
try expect(v.c.p2.a == 0 and v.c.p2.b == 0);
try expect(v.c.p1.a == k + 1 and v.c.p1.b == k);
try expect(v.c.last == -32768);
try expect(v.c.pad1 == 11 and v.c.pad2 == 2);
}
};

try S.doTheTest();
try comptime S.doTheTest();
}

test "runtime init of unnamed packed struct type" {
if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest;
if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest;
Expand Down Expand Up @@ -632,7 +726,6 @@ test "pointer to container level packed struct field" {
test "store undefined to packed result location" {
if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest;
if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest;
if (builtin.zig_backend == .stage2_llvm) return error.SkipZigTest;
if (builtin.zig_backend == .stage2_spirv64) return error.SkipZigTest;
if (builtin.zig_backend == .stage2_wasm) return error.SkipZigTest;

Expand Down