Commit 8c367ef99aae05ddba800a6c01cb07677e2f512c | zig

author	Xavier Bouchoux <xavierb@gmail.com> 2023-07-29 11:50:25+02:00
committer	Xavier Bouchoux <xavierb@gmail.com> 2023-07-29 18:16:13+02:00
log	8c367ef99aae05ddba800a6c01cb07677e2f512c
tree	a9fecf3efe8e4ed6285f4de7d11ad02edb86e3d1
parent	46abf2045476a32b6f4dd939679c0fbc7a639133

codegen: fix access to byte-aligned nested packed struct elems

When acessing a packed struct member via a byte aligned ptr (from the optimisation in Sema.structFieldPtrByIndex())
the codegen must apply the parent ptr packed_offset in addition to the field offset itself.

resolves https://github.com/ziglang/zig/issues/16609

5 files changed, 115 insertions(+), 12 deletions(-)

src
- arch
  - wasm
    CodeGen.zig
  - x86_64
    CodeGen.zig
- codegen
  - c.zig
  - llvm.zig
test
- behavior
  - packed-struct.zig

src/arch/wasm/CodeGen.zig+10-5

 @@ -3675,8 +3675,9 @@ 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 struct_ptr_ty = func.typeOf(extra.data.struct_operand);
-    const result = try func.structFieldPtr(inst, extra.data.struct_operand, struct_ptr, struct_ty, extra.data.field_index);
+    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});
 }
 @@ -3684,9 +3685,10 @@ fn airStructFieldPtrIndex(func: *CodeGen, inst: Air.Inst.Index, index: u32) Inne
     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});
 }
 @@ -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,

src/arch/x86_64/CodeGen.zig+3-1

 @@ -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,
     });

src/codegen/c.zig+6-5

...	@@ -661,7 +661,7 @@ pub const DeclGen = struct {	...	@@ -661,7 +661,7 @@ pub const DeclGen = struct {
661	try dg.renderCType(writer, ptr_cty);	661	try dg.renderCType(writer, ptr_cty);
662	try writer.writeByte(')');	662	try writer.writeByte(')');
663	}	663	}
664	switch (fieldLocation(base_ty, ptr_ty, @as(u32, @intCast(field.index)), mod)) {	664	switch (fieldLocation(ptr_base_ty, ptr_ty, @as(u32, @intCast(field.index)), mod)) {
665	.begin => try dg.renderParentPtr(writer, field.base, location),	665	.begin => try dg.renderParentPtr(writer, field.base, location),
666	.field => \|name\| {	666	.field => \|name\| {
667	try writer.writeAll("&(");	667	try writer.writeAll("&(");
...	@@ -5187,7 +5187,7 @@ fn airOptionalPayloadPtrSet(f: *Function, inst: Air.Inst.Index) !CValue {	...	@@ -5187,7 +5187,7 @@ fn airOptionalPayloadPtrSet(f: *Function, inst: Air.Inst.Index) !CValue {
5187	}	5187	}
5188		5188
5189	fn fieldLocation(	5189	fn fieldLocation(
5190	container_ty: Type,	5190	container_ptr_ty: Type,
5191	field_ptr_ty: Type,	5191	field_ptr_ty: Type,
5192	field_index: u32,	5192	field_index: u32,
5193	mod: *Module,	5193	mod: *Module,
...	@@ -5198,6 +5198,7 @@ fn fieldLocation(	...	@@ -5198,6 +5198,7 @@ fn fieldLocation(
5198	end: void,	5198	end: void,
5199	} {	5199	} {
5200	const ip = &mod.intern_pool;	5200	const ip = &mod.intern_pool;
		5201	const container_ty = container_ptr_ty.childType(mod);
5201	return switch (container_ty.zigTypeTag(mod)) {	5202	return switch (container_ty.zigTypeTag(mod)) {
5202	.Struct => switch (container_ty.containerLayout(mod)) {	5203	.Struct => switch (container_ty.containerLayout(mod)) {
5203	.Auto, .Extern => for (field_index..container_ty.structFieldCount(mod)) \|next_field_index\| {	5204	.Auto, .Extern => for (field_index..container_ty.structFieldCount(mod)) \|next_field_index\| {
...	@@ -5211,7 +5212,7 @@ fn fieldLocation(	...	@@ -5211,7 +5212,7 @@ fn fieldLocation(
5211	.{ .identifier = ip.stringToSlice(container_ty.structFieldName(next_field_index, mod)) } };	5212	.{ .identifier = ip.stringToSlice(container_ty.structFieldName(next_field_index, mod)) } };
5212	} else if (container_ty.hasRuntimeBitsIgnoreComptime(mod)) .end else .begin,	5213	} else if (container_ty.hasRuntimeBitsIgnoreComptime(mod)) .end else .begin,
5213	.Packed => if (field_ptr_ty.ptrInfo(mod).packed_offset.host_size == 0)	5214	.Packed => if (field_ptr_ty.ptrInfo(mod).packed_offset.host_size == 0)
5214	.{ .byte_offset = container_ty.packedStructFieldByteOffset(field_index, mod) }	5215	.{ .byte_offset = container_ty.packedStructFieldByteOffset(field_index, mod) + @divExact(container_ptr_ty.ptrInfo(mod).packed_offset.bit_offset, 8) }
5215	else	5216	else
5216	.begin,	5217	.begin,
5217	},	5218	},
...	@@ -5282,7 +5283,7 @@ fn airFieldParentPtr(f: *Function, inst: Air.Inst.Index) !CValue {	...	@@ -5282,7 +5283,7 @@ fn airFieldParentPtr(f: *Function, inst: Air.Inst.Index) !CValue {
5282	try f.renderType(writer, container_ptr_ty);	5283	try f.renderType(writer, container_ptr_ty);
5283	try writer.writeByte(')');	5284	try writer.writeByte(')');
5284		5285
5285	switch (fieldLocation(container_ty, field_ptr_ty, extra.field_index, mod)) {	5286	switch (fieldLocation(container_ptr_ty, field_ptr_ty, extra.field_index, mod)) {
5286	.begin => try f.writeCValue(writer, field_ptr_val, .Initializer),	5287	.begin => try f.writeCValue(writer, field_ptr_val, .Initializer),
5287	.field => \|field\| {	5288	.field => \|field\| {
5288	const u8_ptr_ty = try mod.adjustPtrTypeChild(field_ptr_ty, Type.u8);	5289	const u8_ptr_ty = try mod.adjustPtrTypeChild(field_ptr_ty, Type.u8);
...	@@ -5339,7 +5340,7 @@ fn fieldPtr(	...	@@ -5339,7 +5340,7 @@ fn fieldPtr(
5339	try f.renderType(writer, field_ptr_ty);	5340	try f.renderType(writer, field_ptr_ty);
5340	try writer.writeByte(')');	5341	try writer.writeByte(')');
5341		5342
5342	switch (fieldLocation(container_ty, field_ptr_ty, field_index, mod)) {	5343	switch (fieldLocation(container_ptr_ty, field_ptr_ty, field_index, mod)) {
5343	.begin => try f.writeCValue(writer, container_ptr_val, .Initializer),	5344	.begin => try f.writeCValue(writer, container_ptr_val, .Initializer),
5344	.field => \|field\| {	5345	.field => \|field\| {
5345	try writer.writeByte('&');	5346	try writer.writeByte('&');

src/codegen/llvm.zig+2-1

test/behavior/packed-struct.zig+94

...	@@ -528,6 +528,100 @@ test "nested packed struct field access test" {	...	@@ -528,6 +528,100 @@ test "nested packed struct field access test" {
528	try std.testing.expect(arg.g.i == 8);	528	try std.testing.expect(arg.g.i == 8);
529	}	529	}
530		530
		531	test "nested packed struct at non-zero offset" {
		532	if (builtin.zig_backend == .stage2_sparc64) return error.SkipZigTest; // TODO
		533	if (builtin.zig_backend == .stage2_spirv64) return error.SkipZigTest;
		534
		535	const Pair = packed struct(u24) {
		536	a: u16 = 0,
		537	b: u8 = 0,
		538	};
		539	const A = packed struct {
		540	p1: Pair,
		541	p2: Pair,
		542	};
		543
		544	var k: u8 = 123;
		545	var v: A = .{
		546	.p1 = .{ .a = k + 1, .b = k },
		547	.p2 = .{ .a = k + 1, .b = k },
		548	};
		549
		550	try expect(v.p1.a == k + 1 and v.p1.b == k);
		551	try expect(v.p2.a == k + 1 and v.p2.b == k);
		552
		553	v.p2.a -= v.p1.a;
		554	v.p2.b -= v.p1.b;
		555	try expect(v.p2.a == 0 and v.p2.b == 0);
		556	try expect(v.p1.a == k + 1 and v.p1.b == k);
		557	}
		558
		559	test "nested packed struct at non-zero offset 2" {
		560	if (builtin.zig_backend == .stage2_x86_64) return error.SkipZigTest; // TODO
		561	if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest;
		562	if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest;
		563	if (builtin.zig_backend == .stage2_sparc64) return error.SkipZigTest; // TODO
		564	if (builtin.zig_backend == .stage2_spirv64) return error.SkipZigTest;
		565	if (builtin.zig_backend == .stage2_wasm) return error.SkipZigTest; // TODO
		566	if (builtin.zig_backend == .stage2_c) return error.SkipZigTest;
		567
		568	const S = struct {
		569	const Pair = packed struct(u40) {
		570	a: u32 = 0,
		571	b: u8 = 0,
		572	};
		573	const A = packed struct {
		574	p1: Pair,
		575	p2: Pair,
		576	c: C,
		577	};
		578	const C = packed struct {
		579	p1: Pair,
		580	pad1: u5,
		581	p2: Pair,
		582	pad2: u3,
		583	last: i16,
		584	};
		585
		586	fn doTheTest() !void {
		587	var k: u8 = 123;
		588	var v: A = .{
		589	.p1 = .{ .a = k + 1, .b = k },
		590	.p2 = .{ .a = k + 1, .b = k },
		591	.c = .{
		592	.pad1 = 11,
		593	.pad2 = 2,
		594	.p1 = .{ .a = k + 1, .b = k },
		595	.p2 = .{ .a = k + 1, .b = k },
		596	.last = -12345,
		597	},
		598	};
		599
		600	try expect(v.p1.a == k + 1 and v.p1.b == k);
		601	try expect(v.p2.a == k + 1 and v.p2.b == k);
		602	try expect(v.c.p2.a == k + 1 and v.c.p2.b == k);
		603	try expect(v.c.p2.a == k + 1 and v.c.p2.b == k);
		604	try expect(v.c.last == -12345);
		605	try expect(v.c.pad1 == 11 and v.c.pad2 == 2);
		606
		607	v.p2.a -= v.p1.a;
		608	v.p2.b -= v.p1.b;
		609	v.c.p2.a -= v.c.p1.a;
		610	v.c.p2.b -= v.c.p1.b;
		611	v.c.last -\|= 32000;
		612	try expect(v.p2.a == 0 and v.p2.b == 0);
		613	try expect(v.p1.a == k + 1 and v.p1.b == k);
		614	try expect(v.c.p2.a == 0 and v.c.p2.b == 0);
		615	try expect(v.c.p1.a == k + 1 and v.c.p1.b == k);
		616	try expect(v.c.last == -32768);
		617	try expect(v.c.pad1 == 11 and v.c.pad2 == 2);
		618	}
		619	};
		620
		621	try S.doTheTest();
		622	try comptime S.doTheTest();
		623	}
		624
531	test "runtime init of unnamed packed struct type" {	625	test "runtime init of unnamed packed struct type" {
532	if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest;	626	if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest;
533	if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest;	627	if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest;

...	@@ -10611,6 +10611,7 @@ pub const FuncGen = struct {	...	@@ -10611,6 +10611,7 @@ pub const FuncGen = struct {
10611	.Packed => {	10611	.Packed => {
10612	const result_ty = self.typeOfIndex(inst);	10612	const result_ty = self.typeOfIndex(inst);
10613	const result_ty_info = result_ty.ptrInfo(mod);	10613	const result_ty_info = result_ty.ptrInfo(mod);
		10614	const struct_ptr_ty_info = struct_ptr_ty.ptrInfo(mod);
10614		10615
10615	if (result_ty_info.packed_offset.host_size != 0) {	10616	if (result_ty_info.packed_offset.host_size != 0) {
10616	// From LLVM's perspective, a pointer to a packed struct and a pointer	10617	// From LLVM's perspective, a pointer to a packed struct and a pointer
...	@@ -10622,7 +10623,7 @@ pub const FuncGen = struct {	...	@@ -10622,7 +10623,7 @@ pub const FuncGen = struct {
10622		10623
10623	// We have a pointer to a packed struct field that happens to be byte-aligned.	10624	// We have a pointer to a packed struct field that happens to be byte-aligned.
10624	// Offset our operand pointer by the correct number of bytes.	10625	// Offset our operand pointer by the correct number of bytes.
10625	const byte_offset = struct_ty.packedStructFieldByteOffset(field_index, mod);	10626	const byte_offset = struct_ty.packedStructFieldByteOffset(field_index, mod) + @divExact(struct_ptr_ty_info.packed_offset.bit_offset, 8);
10626	if (byte_offset == 0) return struct_ptr;	10627	if (byte_offset == 0) return struct_ptr;
10627	const usize_ty = try o.lowerType(Type.usize);	10628	const usize_ty = try o.lowerType(Type.usize);
10628	const llvm_index = try o.builder.intValue(usize_ty, byte_offset);	10629	const llvm_index = try o.builder.intValue(usize_ty, byte_offset);