Skip to content

Commit

Permalink
Merge pull request ziglang#16611 from xxxbxxx/packed-struct
Browse files Browse the repository at this point in the history 
codegen: fix various packed struct issues

Closes ziglang#16609
Closes ziglang#15337
  • Loading branch information
@jacobly0
jacobly0 authored Jul 30, 2023
2 parents 7ad4aed + 8c367ef commit 6f0a613
Show file tree
Hide file tree
Showing 5 changed files with 124 additions and 13 deletions.
15 changes: 10 additions & 5 deletions src/arch/wasm/CodeGen.zig
View file
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
View file
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
View file
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
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8797,6 +8797,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 @@ -10646,6 +10655,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 @@ -10657,7 +10667,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
View file
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

0 comments on commit 6f0a613

Please sign in to comment.