| author |  Andrew Kelley <andrew@ziglang.org> 2022-06-07 20:51:14-07:00 |
| committer | Andrew Kelley <andrew@ziglang.org> 2022-06-07 21:05:40-07:00 |
| log | 3e30ba3f20dce2d406253de3fc0eb86934a3eaa7 |
| tree | 37cde6ce42dc2e326bd7beb599b6ad0d77051c28 |
| parent | 70dc910086582b028d404d5de5049ceae0a95161 |
stage2: better codegen for byte-aligned packed struct fields
* Sema: handle overaligned packed struct field pointers
* LLVM: handle byte-aligned packed struct field pointers4 files changed, 136 insertions(+), 13 deletions(-)
test
behavior
src/Sema.zig+21-2
| ... | @@ -18678,8 +18678,6 @@ fn structFieldPtrByIndex( | ... | @@ -18678,8 +18678,6 @@ fn structFieldPtrByIndex( |
| 18678 | 18678 | ||
| 18679 | const target = sema.mod.getTarget(); | 18679 | const target = sema.mod.getTarget(); |
| 18680 | 18680 | ||
| 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); |
| 18685 | 18683 | ||
| ... | @@ -18700,6 +18698,27 @@ fn structFieldPtrByIndex( | ... | @@ -18700,6 +18698,27 @@ 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 | if (parent_align != 0 and ptr_ty_data.bit_offset % 8 == 0) { | ||
| 18712 | const byte_offset = ptr_ty_data.bit_offset / 8; | ||
| 18713 | const elem_size_bytes = ptr_ty_data.pointee_type.abiSize(target); | ||
| 18714 | const elem_size_bits = ptr_ty_data.pointee_type.bitSize(target); | ||
| 18715 | if (elem_size_bytes * 8 == elem_size_bits) { | ||
| 18716 | const new_align = @as(u32, 1) << @intCast(u5, @ctz(u64, byte_offset | parent_align)); | ||
| 18717 | ptr_ty_data.bit_offset = 0; | ||
| 18718 | ptr_ty_data.host_size = 0; | ||
| 18719 | ptr_ty_data.@"align" = new_align; | ||
| 18720 | } | ||
| 18721 | } | ||
| 18703 | } else { | 18722 | } else { |
| 18704 | ptr_ty_data.@"align" = field.abi_align; | 18723 | ptr_ty_data.@"align" = field.abi_align; |
| 18705 | } | 18724 | } |
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 pointer | 8320 | const result_ty = self.air.typeOfIndex(inst); |
| 8319 | // to a field of a packed struct are the same. The difference is in the | 8321 | const result_ty_info = result_ty.ptrInfo().data; |
| 8320 | // Zig pointer type which provides information for how to mask and shift | 8322 | 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_size | 8324 | 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 instead | 8327 | // 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+21
| ... | @@ -5591,6 +5591,27 @@ pub const Type = extern union { | ... | @@ -5591,6 +5591,27 @@ pub const Type = extern union { |
| 5591 | } | 5591 | } |
| 5592 | } | 5592 | } |
| 5593 | 5593 | ||
| 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 running_bits: u16 = 0; | ||
| 5601 | for (struct_obj.fields.values()) |f, i| { | ||
| 5602 | if (!f.ty.hasRuntimeBits()) continue; | ||
| 5603 | |||
| 5604 | if (i == field_index) { | ||
| 5605 | bit_offset = running_bits; | ||
| 5606 | } | ||
| 5607 | running_bits += @intCast(u16, f.ty.bitSize(target)); | ||
| 5608 | } | ||
| 5609 | const host_size = (running_bits + 7) / 8; | ||
| 5610 | _ = host_size; // TODO big-endian | ||
| 5611 | const byte_offset = bit_offset / 8; | ||
| 5612 | return byte_offset; | ||
| 5613 | } | ||
| 5614 | |||
| 5594 | pub const FieldOffset = struct { | 5615 | pub const FieldOffset = struct { |
| 5595 | field: usize, | 5616 | field: usize, |
| 5596 | offset: u64, | 5617 | offset: u64, |
test/behavior/packed-struct.zig+65
| ... | @@ -1,5 +1,6 @@ | ... | @@ -1,5 +1,6 @@ |
| 1 | const std = @import("std"); | 1 | const std = @import("std"); |
| 2 | const builtin = @import("builtin"); | 2 | const builtin = @import("builtin"); |
| 3 | const assert = std.debug.assert; | ||
| 3 | const expect = std.testing.expect; | 4 | const expect = std.testing.expect; |
| 4 | const expectEqual = std.testing.expectEqual; | 5 | const expectEqual = std.testing.expectEqual; |
| 5 | const native_endian = builtin.cpu.arch.endian(); | 6 | const native_endian = builtin.cpu.arch.endian(); |
| ... | @@ -305,3 +306,67 @@ test "regular in irregular packed struct" { | ... | @@ -305,3 +306,67 @@ test "regular in irregular packed struct" { |
| 305 | try expectEqual(@as(u16, 235), foo.bar.a); | 306 | try expectEqual(@as(u16, 235), foo.bar.a); |
| 306 | try expectEqual(@as(u8, 42), foo.bar.b); | 307 | try expectEqual(@as(u8, 42), foo.bar.b); |
| 307 | } | 308 | } |
| 309 | |||
| 310 | test "byte-aligned field pointer offsets" { | ||
| 311 | if (builtin.zig_backend == .stage1) return error.SkipZigTest; | ||
| 312 | if (builtin.zig_backend == .stage2_wasm) return error.SkipZigTest; | ||
| 313 | if (builtin.zig_backend == .stage2_c) return error.SkipZigTest; | ||
| 314 | if (builtin.zig_backend == .stage2_x86_64) return error.SkipZigTest; | ||
| 315 | if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest; | ||
| 316 | if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; | ||
| 317 | |||
| 318 | const S = struct { | ||
| 319 | const A = packed struct { | ||
| 320 | a: u8, | ||
| 321 | b: u8, | ||
| 322 | c: u8, | ||
| 323 | d: u8, | ||
| 324 | }; | ||
| 325 | |||
| 326 | const B = packed struct { | ||
| 327 | a: u16, | ||
| 328 | b: u16, | ||
| 329 | }; | ||
| 330 | |||
| 331 | fn doTheTest() !void { | ||
| 332 | var a: A = .{ | ||
| 333 | .a = 1, | ||
| 334 | .b = 2, | ||
| 335 | .c = 3, | ||
| 336 | .d = 4, | ||
| 337 | }; | ||
| 338 | comptime assert(@TypeOf(&a.a) == *align(4) u8); | ||
| 339 | comptime assert(@TypeOf(&a.b) == *u8); | ||
| 340 | comptime assert(@TypeOf(&a.c) == *align(2) u8); | ||
| 341 | comptime assert(@TypeOf(&a.d) == *u8); | ||
| 342 | try expect(a.a == 1); | ||
| 343 | try expect(a.b == 2); | ||
| 344 | try expect(a.c == 3); | ||
| 345 | try expect(a.d == 4); | ||
| 346 | a.a += 1; | ||
| 347 | a.b += 1; | ||
| 348 | a.c += 1; | ||
| 349 | a.d += 1; | ||
| 350 | try expect(a.a == 2); | ||
| 351 | try expect(a.b == 3); | ||
| 352 | try expect(a.c == 4); | ||
| 353 | try expect(a.d == 5); | ||
| 354 | |||
| 355 | var b: B = .{ | ||
| 356 | .a = 1, | ||
| 357 | .b = 2, | ||
| 358 | }; | ||
| 359 | comptime assert(@TypeOf(&b.a) == *align(4) u16); | ||
| 360 | comptime assert(@TypeOf(&b.b) == *u16); | ||
| 361 | try expect(b.a == 1); | ||
| 362 | try expect(b.b == 2); | ||
| 363 | b.a += 1; | ||
| 364 | b.b += 1; | ||
| 365 | try expect(b.a == 2); | ||
| 366 | try expect(b.b == 3); | ||
| 367 | } | ||
| 368 | }; | ||
| 369 | |||
| 370 | try S.doTheTest(); | ||
| 371 | comptime try S.doTheTest(); | ||
| 372 | } |