From 53c86febcbeee858e9c6536c54adf99ee644147e Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Tue, 7 Jun 2022 22:47:08 -0700 Subject: [PATCH] stage2: packed struct fixes for big-endian targets --- src/Sema.zig | 11 +++++-- src/type.zig | 7 ++-- test/behavior/packed-struct.zig | 57 ++++++++++++++++++++++++++------- 3 files changed, 58 insertions(+), 17 deletions(-) diff --git a/src/Sema.zig b/src/Sema.zig index 0cf449991d..8ce9226bd4 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -18708,11 +18708,18 @@ fn structFieldPtrByIndex( // If the field happens to be byte-aligned, simplify the pointer type. // The pointee type bit size must match its ABI byte size so that loads and stores // do not interfere with the surrounding packed bits. - if (parent_align != 0 and ptr_ty_data.bit_offset % 8 == 0) { - const byte_offset = ptr_ty_data.bit_offset / 8; + // We do not attempt this with big-endian targets yet because of nested + // structs and floats. I need to double-check the desired behavior for big endian + // targets before adding the necessary complications to this code. This will not + // cause miscompilations; it only means the field pointer uses bit masking when it + // might not be strictly necessary. + if (parent_align != 0 and ptr_ty_data.bit_offset % 8 == 0 and + target.cpu.arch.endian() == .Little) + { const elem_size_bytes = ptr_ty_data.pointee_type.abiSize(target); const elem_size_bits = ptr_ty_data.pointee_type.bitSize(target); if (elem_size_bytes * 8 == elem_size_bits) { + const byte_offset = ptr_ty_data.bit_offset / 8; const new_align = @as(u32, 1) << @intCast(u5, @ctz(u64, byte_offset | parent_align)); ptr_ty_data.bit_offset = 0; ptr_ty_data.host_size = 0; diff --git a/src/type.zig b/src/type.zig index 8e5eaf0ec7..b6f8db4ca1 100644 --- a/src/type.zig +++ b/src/type.zig @@ -5597,17 +5597,18 @@ pub const Type = extern union { comptime assert(Type.packed_struct_layout_version == 2); var bit_offset: u16 = undefined; + var elem_size_bits: u16 = undefined; var running_bits: u16 = 0; for (struct_obj.fields.values()) |f, i| { if (!f.ty.hasRuntimeBits()) continue; + const field_bits = @intCast(u16, f.ty.bitSize(target)); if (i == field_index) { bit_offset = running_bits; + elem_size_bits = field_bits; } - running_bits += @intCast(u16, f.ty.bitSize(target)); + running_bits += field_bits; } - const host_size = (running_bits + 7) / 8; - _ = host_size; // TODO big-endian const byte_offset = bit_offset / 8; return byte_offset; } diff --git a/test/behavior/packed-struct.zig b/test/behavior/packed-struct.zig index ed06cc486d..2483bbea69 100644 --- a/test/behavior/packed-struct.zig +++ b/test/behavior/packed-struct.zig @@ -289,13 +289,7 @@ test "regular in irregular packed struct" { const Irregular = packed struct { bar: Regular = Regular{}, - - // This field forces the regular packed struct to be a part of single u48 - // and thus it all gets represented as an array of 6 bytes in LLVM _: u24 = 0, - - // This struct on its own can represent its fields directly in LLVM - // with no need to use array of bytes as underlaying representation. pub const Regular = packed struct { a: u16 = 0, b: u8 = 0 }; }; @@ -335,17 +329,44 @@ test "byte-aligned field pointer offsets" { .c = 3, .d = 4, }; - comptime assert(@TypeOf(&a.a) == *align(4) u8); - comptime assert(@TypeOf(&a.b) == *u8); - comptime assert(@TypeOf(&a.c) == *align(2) u8); - comptime assert(@TypeOf(&a.d) == *u8); + switch (comptime builtin.cpu.arch.endian()) { + .Little => { + comptime assert(@TypeOf(&a.a) == *align(4) u8); + comptime assert(@TypeOf(&a.b) == *u8); + comptime assert(@TypeOf(&a.c) == *align(2) u8); + comptime assert(@TypeOf(&a.d) == *u8); + }, + .Big => { + // TODO re-evaluate packed struct endianness + comptime assert(@TypeOf(&a.a) == *align(4:0:4) u8); + comptime assert(@TypeOf(&a.b) == *align(4:8:4) u8); + comptime assert(@TypeOf(&a.c) == *align(4:16:4) u8); + comptime assert(@TypeOf(&a.d) == *align(4:24:4) u8); + }, + } try expect(a.a == 1); try expect(a.b == 2); try expect(a.c == 3); try expect(a.d == 4); + a.a += 1; + try expect(a.a == 2); + try expect(a.b == 2); + try expect(a.c == 3); + try expect(a.d == 4); + a.b += 1; + try expect(a.a == 2); + try expect(a.b == 3); + try expect(a.c == 3); + try expect(a.d == 4); + a.c += 1; + try expect(a.a == 2); + try expect(a.b == 3); + try expect(a.c == 4); + try expect(a.d == 4); + a.d += 1; try expect(a.a == 2); try expect(a.b == 3); @@ -356,11 +377,23 @@ test "byte-aligned field pointer offsets" { .a = 1, .b = 2, }; - comptime assert(@TypeOf(&b.a) == *align(4) u16); - comptime assert(@TypeOf(&b.b) == *u16); + switch (comptime builtin.cpu.arch.endian()) { + .Little => { + comptime assert(@TypeOf(&b.a) == *align(4) u16); + comptime assert(@TypeOf(&b.b) == *u16); + }, + .Big => { + comptime assert(@TypeOf(&b.a) == *align(4:0:4) u16); + comptime assert(@TypeOf(&b.b) == *align(4:16:4) u16); + }, + } try expect(b.a == 1); try expect(b.b == 2); + b.a += 1; + try expect(b.a == 2); + try expect(b.b == 2); + b.b += 1; try expect(b.a == 2); try expect(b.b == 3);