Commit 53c86febcb

Andrew Kelley <andrew@ziglang.org>
2022-06-08 07:47:08
stage2: packed struct fixes for big-endian targets
1 parent 3e30ba3
Changed files (3)
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;
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;
     }
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);