authorgravatar for andrew@ziglang.orgAndrew Kelley <andrew@ziglang.org> 2022-06-08 15:24:17-04:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2022-06-08 15:24:17-04:00
log868a39b29697025e9857112e20b77878802c00f7
treecb4d2149d74f2f758970e86ce5dd1a71c2bd80f8
parentf5d97e5e4865d454f468c352e671d2e4a15cf4e4
parent53c86febcbeee858e9c6536c54adf99ee644147e
signaturebadge-question-mark Signed by PGP key 4AEE18F83AFDEB23

Merge pull request #11821 from ziglang/stage2-packed-structs

stage2: better codegen for byte-aligned packed struct fields

4 files changed, 183 insertions(+), 19 deletions(-)

src/Sema.zig+28-2
...@@ -18678,8 +18678,6 @@ fn structFieldPtrByIndex(...@@ -18678,8 +18678,6 @@ fn structFieldPtrByIndex(
1867818678
18679 const target = sema.mod.getTarget();18679 const target = sema.mod.getTarget();
1868018680
18681 // TODO handle when the struct pointer is overaligned, we should return a potentially
18682 // over-aligned field pointer too.
18683 if (struct_obj.layout == .Packed) {18681 if (struct_obj.layout == .Packed) {
18684 comptime assert(Type.packed_struct_layout_version == 2);18682 comptime assert(Type.packed_struct_layout_version == 2);
1868518683
...@@ -18700,6 +18698,34 @@ fn structFieldPtrByIndex(...@@ -18700,6 +18698,34 @@ fn structFieldPtrByIndex(
18700 ptr_ty_data.host_size = struct_ptr_ty_info.host_size;18698 ptr_ty_data.host_size = struct_ptr_ty_info.host_size;
18701 ptr_ty_data.bit_offset += struct_ptr_ty_info.bit_offset;18699 ptr_ty_data.bit_offset += struct_ptr_ty_info.bit_offset;
18702 }18700 }
18701
18702 const parent_align = if (struct_ptr_ty_info.@"align" != 0)
18703 struct_ptr_ty_info.@"align"
18704 else
18705 struct_ptr_ty_info.pointee_type.abiAlignment(target);
18706 ptr_ty_data.@"align" = parent_align;
18707
18708 // If the field happens to be byte-aligned, simplify the pointer type.
18709 // The pointee type bit size must match its ABI byte size so that loads and stores
18710 // do not interfere with the surrounding packed bits.
18711 // We do not attempt this with big-endian targets yet because of nested
18712 // structs and floats. I need to double-check the desired behavior for big endian
18713 // targets before adding the necessary complications to this code. This will not
18714 // cause miscompilations; it only means the field pointer uses bit masking when it
18715 // might not be strictly necessary.
18716 if (parent_align != 0 and ptr_ty_data.bit_offset % 8 == 0 and
18717 target.cpu.arch.endian() == .Little)
18718 {
18719 const elem_size_bytes = ptr_ty_data.pointee_type.abiSize(target);
18720 const elem_size_bits = ptr_ty_data.pointee_type.bitSize(target);
18721 if (elem_size_bytes * 8 == elem_size_bits) {
18722 const byte_offset = ptr_ty_data.bit_offset / 8;
18723 const new_align = @as(u32, 1) << @intCast(u5, @ctz(u64, byte_offset | parent_align));
18724 ptr_ty_data.bit_offset = 0;
18725 ptr_ty_data.host_size = 0;
18726 ptr_ty_data.@"align" = new_align;
18727 }
18728 }
18703 } else {18729 } else {
18704 ptr_ty_data.@"align" = field.abi_align;18730 ptr_ty_data.@"align" = field.abi_align;
18705 }18731 }
src/codegen/llvm.zig+29-11
...@@ -8311,23 +8311,41 @@ pub const FuncGen = struct {...@@ -8311,23 +8311,41 @@ pub const FuncGen = struct {
8311 field_index: u32,8311 field_index: u32,
8312 ) !?*const llvm.Value {8312 ) !?*const llvm.Value {
8313 if (self.liveness.isUnused(inst)) return null;8313 if (self.liveness.isUnused(inst)) return null;
8314
8315 const target = self.dg.object.target;
8314 const struct_ty = struct_ptr_ty.childType();8316 const struct_ty = struct_ptr_ty.childType();
8315 switch (struct_ty.zigTypeTag()) {8317 switch (struct_ty.zigTypeTag()) {
8316 .Struct => switch (struct_ty.containerLayout()) {8318 .Struct => switch (struct_ty.containerLayout()) {
8317 .Packed => {8319 .Packed => {
8318 // From LLVM's perspective, a pointer to a packed struct and a pointer8320 const result_ty = self.air.typeOfIndex(inst);
8319 // to a field of a packed struct are the same. The difference is in the8321 const result_ty_info = result_ty.ptrInfo().data;
8320 // Zig pointer type which provides information for how to mask and shift8322 const result_llvm_ty = try self.dg.lowerType(result_ty);
8321 // out the relevant bits when accessing the pointee.8323
8322 // Here we perform a bitcast because we want to use the host_size8324 if (result_ty_info.host_size != 0) {
8323 // as the llvm pointer element type.8325 // From LLVM's perspective, a pointer to a packed struct and a pointer
8324 const result_llvm_ty = try self.dg.lowerType(self.air.typeOfIndex(inst));8326 // to a field of a packed struct are the same. The difference is in the
8325 // TODO this can be removed if we change host_size to be bits instead8327 // Zig pointer type which provides information for how to mask and shift
8326 // of bytes.8328 // out the relevant bits when accessing the pointee.
8327 return self.builder.buildBitCast(struct_ptr, result_llvm_ty, "");8329 // Here we perform a bitcast because we want to use the host_size
8330 // as the llvm pointer element type.
8331 return self.builder.buildBitCast(struct_ptr, result_llvm_ty, "");
8332 }
8333
8334 // We have a pointer to a packed struct field that happens to be byte-aligned.
8335 // Offset our operand pointer by the correct number of bytes.
8336 const byte_offset = struct_ty.packedStructFieldByteOffset(field_index, target);
8337 if (byte_offset == 0) {
8338 return self.builder.buildBitCast(struct_ptr, result_llvm_ty, "");
8339 }
8340 const llvm_bytes_ptr_ty = self.context.intType(8).pointerType(0);
8341 const ptr_as_bytes = self.builder.buildBitCast(struct_ptr, llvm_bytes_ptr_ty, "");
8342 const llvm_usize = try self.dg.lowerType(Type.usize);
8343 const llvm_index = llvm_usize.constInt(byte_offset, .False);
8344 const indices: [1]*const llvm.Value = .{llvm_index};
8345 const new_ptr = self.builder.buildInBoundsGEP(ptr_as_bytes, &indices, indices.len, "");
8346 return self.builder.buildBitCast(new_ptr, result_llvm_ty, "");
8328 },8347 },
8329 else => {8348 else => {
8330 const target = self.dg.module.getTarget();
8331 var ty_buf: Type.Payload.Pointer = undefined;8349 var ty_buf: Type.Payload.Pointer = undefined;
8332 if (llvmFieldIndex(struct_ty, field_index, target, &ty_buf)) |llvm_field_index| {8350 if (llvmFieldIndex(struct_ty, field_index, target, &ty_buf)) |llvm_field_index| {
8333 return self.builder.buildStructGEP(struct_ptr, llvm_field_index, "");8351 return self.builder.buildStructGEP(struct_ptr, llvm_field_index, "");
src/type.zig+22
...@@ -5591,6 +5591,28 @@ pub const Type = extern union {...@@ -5591,6 +5591,28 @@ pub const Type = extern union {
5591 }5591 }
5592 }5592 }
55935593
5594 pub fn packedStructFieldByteOffset(ty: Type, field_index: usize, target: Target) u32 {
5595 const struct_obj = ty.castTag(.@"struct").?.data;
5596 assert(struct_obj.layout == .Packed);
5597 comptime assert(Type.packed_struct_layout_version == 2);
5598
5599 var bit_offset: u16 = undefined;
5600 var elem_size_bits: u16 = undefined;
5601 var running_bits: u16 = 0;
5602 for (struct_obj.fields.values()) |f, i| {
5603 if (!f.ty.hasRuntimeBits()) continue;
5604
5605 const field_bits = @intCast(u16, f.ty.bitSize(target));
5606 if (i == field_index) {
5607 bit_offset = running_bits;
5608 elem_size_bits = field_bits;
5609 }
5610 running_bits += field_bits;
5611 }
5612 const byte_offset = bit_offset / 8;
5613 return byte_offset;
5614 }
5615
5594 pub const FieldOffset = struct {5616 pub const FieldOffset = struct {
5595 field: usize,5617 field: usize,
5596 offset: u64,5618 offset: u64,
test/behavior/packed-struct.zig+104-6
...@@ -1,5 +1,6 @@...@@ -1,5 +1,6 @@
1const std = @import("std");1const std = @import("std");
2const builtin = @import("builtin");2const builtin = @import("builtin");
3const assert = std.debug.assert;
3const expect = std.testing.expect;4const expect = std.testing.expect;
4const expectEqual = std.testing.expectEqual;5const expectEqual = std.testing.expectEqual;
5const native_endian = builtin.cpu.arch.endian();6const native_endian = builtin.cpu.arch.endian();
...@@ -288,13 +289,7 @@ test "regular in irregular packed struct" {...@@ -288,13 +289,7 @@ test "regular in irregular packed struct" {
288289
289 const Irregular = packed struct {290 const Irregular = packed struct {
290 bar: Regular = Regular{},291 bar: Regular = Regular{},
291
292 // This field forces the regular packed struct to be a part of single u48
293 // and thus it all gets represented as an array of 6 bytes in LLVM
294 _: u24 = 0,292 _: u24 = 0,
295
296 // This struct on its own can represent its fields directly in LLVM
297 // with no need to use array of bytes as underlaying representation.
298 pub const Regular = packed struct { a: u16 = 0, b: u8 = 0 };293 pub const Regular = packed struct { a: u16 = 0, b: u8 = 0 };
299 };294 };
300295
...@@ -305,3 +300,106 @@ test "regular in irregular packed struct" {...@@ -305,3 +300,106 @@ test "regular in irregular packed struct" {
305 try expectEqual(@as(u16, 235), foo.bar.a);300 try expectEqual(@as(u16, 235), foo.bar.a);
306 try expectEqual(@as(u8, 42), foo.bar.b);301 try expectEqual(@as(u8, 42), foo.bar.b);
307}302}
303
304test "byte-aligned field pointer offsets" {
305 if (builtin.zig_backend == .stage1) return error.SkipZigTest;
306 if (builtin.zig_backend == .stage2_wasm) return error.SkipZigTest;
307 if (builtin.zig_backend == .stage2_c) return error.SkipZigTest;
308 if (builtin.zig_backend == .stage2_x86_64) return error.SkipZigTest;
309 if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest;
310 if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest;
311
312 const S = struct {
313 const A = packed struct {
314 a: u8,
315 b: u8,
316 c: u8,
317 d: u8,
318 };
319
320 const B = packed struct {
321 a: u16,
322 b: u16,
323 };
324
325 fn doTheTest() !void {
326 var a: A = .{
327 .a = 1,
328 .b = 2,
329 .c = 3,
330 .d = 4,
331 };
332 switch (comptime builtin.cpu.arch.endian()) {
333 .Little => {
334 comptime assert(@TypeOf(&a.a) == *align(4) u8);
335 comptime assert(@TypeOf(&a.b) == *u8);
336 comptime assert(@TypeOf(&a.c) == *align(2) u8);
337 comptime assert(@TypeOf(&a.d) == *u8);
338 },
339 .Big => {
340 // TODO re-evaluate packed struct endianness
341 comptime assert(@TypeOf(&a.a) == *align(4:0:4) u8);
342 comptime assert(@TypeOf(&a.b) == *align(4:8:4) u8);
343 comptime assert(@TypeOf(&a.c) == *align(4:16:4) u8);
344 comptime assert(@TypeOf(&a.d) == *align(4:24:4) u8);
345 },
346 }
347 try expect(a.a == 1);
348 try expect(a.b == 2);
349 try expect(a.c == 3);
350 try expect(a.d == 4);
351
352 a.a += 1;
353 try expect(a.a == 2);
354 try expect(a.b == 2);
355 try expect(a.c == 3);
356 try expect(a.d == 4);
357
358 a.b += 1;
359 try expect(a.a == 2);
360 try expect(a.b == 3);
361 try expect(a.c == 3);
362 try expect(a.d == 4);
363
364 a.c += 1;
365 try expect(a.a == 2);
366 try expect(a.b == 3);
367 try expect(a.c == 4);
368 try expect(a.d == 4);
369
370 a.d += 1;
371 try expect(a.a == 2);
372 try expect(a.b == 3);
373 try expect(a.c == 4);
374 try expect(a.d == 5);
375
376 var b: B = .{
377 .a = 1,
378 .b = 2,
379 };
380 switch (comptime builtin.cpu.arch.endian()) {
381 .Little => {
382 comptime assert(@TypeOf(&b.a) == *align(4) u16);
383 comptime assert(@TypeOf(&b.b) == *u16);
384 },
385 .Big => {
386 comptime assert(@TypeOf(&b.a) == *align(4:0:4) u16);
387 comptime assert(@TypeOf(&b.b) == *align(4:16:4) u16);
388 },
389 }
390 try expect(b.a == 1);
391 try expect(b.b == 2);
392
393 b.a += 1;
394 try expect(b.a == 2);
395 try expect(b.b == 2);
396
397 b.b += 1;
398 try expect(b.a == 2);
399 try expect(b.b == 3);
400 }
401 };
402
403 try S.doTheTest();
404 comptime try S.doTheTest();
405}