stage2: agree with LLVM that @alignOf(u128) is 8

on x86_64 and similar targets.

Author: andrewrk

lib/std/target.zig

@@ -1805,9 +1805,9 @@ pub const Target = struct {
                 else => 4,
             },
 
-            // For x86_64, LLVMABIAlignmentOfType(i128) reports 8. However I think 16
-            // is a better number for two reasons:
-            // 1. Better machine code when loading into SIMD register.
+            // For these, LLVMABIAlignmentOfType(i128) reports 8. Note that 16
+            // is a relevant number in three cases:
+            // 1. Different machine code instruction when loading into SIMD register.
             // 2. The C ABI wants 16 for extern structs.
             // 3. 16-byte cmpxchg needs 16-byte alignment.
             // Same logic for riscv64, powerpc64, mips64, sparc64.
@@ -1818,6 +1818,7 @@ pub const Target = struct {
             .mips64,
             .mips64el,
             .sparc64,
+            => 8,
 
             // Even LLVMABIAlignmentOfType(i128) agrees on these targets.
             .aarch64,

src/Module.zig

@@ -934,13 +934,33 @@ pub const Struct = struct {
         /// If true then `default_val` is the comptime field value.
         is_comptime: bool,
 
-        /// Returns the field alignment, assuming the struct is not packed.
-        pub fn normalAlignment(field: Field, target: Target) u32 {
-            if (field.abi_align == 0) {
-                return field.ty.abiAlignment(target);
-            } else {
+        /// Returns the field alignment. If the struct is packed, returns 0.
+        pub fn alignment(
+            field: Field,
+            target: Target,
+            layout: std.builtin.Type.ContainerLayout,
+        ) u32 {
+            if (field.abi_align != 0) {
+                assert(layout != .Packed);
                 return field.abi_align;
             }
+
+            switch (layout) {
+                .Packed => return 0,
+                .Auto => return field.ty.abiAlignment(target),
+                .Extern => {
+                    // This logic is duplicated in Type.abiAlignmentAdvanced.
+                    const ty_abi_align = field.ty.abiAlignment(target);
+
+                    if (field.ty.isAbiInt() and field.ty.intInfo(target).bits >= 128) {
+                        // The C ABI requires 128 bit integer fields of structs
+                        // to be 16-bytes aligned.
+                        return @maximum(ty_abi_align, 16);
+                    }
+
+                    return ty_abi_align;
+                },
+            }
         }
     };
 

src/Sema.zig

@@ -13003,10 +13003,7 @@ fn zirTypeInfo(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Ai
                     else
                         field.default_val;
                     const default_val_ptr = try sema.optRefValue(block, src, field.ty, opt_default_val);
-                    const alignment = switch (layout) {
-                        .Auto, .Extern => field.normalAlignment(target),
-                        .Packed => 0,
-                    };
+                    const alignment = field.alignment(target, layout);
 
                     struct_field_fields.* = .{
                         // name: []const u8,

src/codegen/llvm.zig

@@ -1777,6 +1777,7 @@ pub const Object = struct {
                 }
 
                 const fields = ty.structFields();
+                const layout = ty.containerLayout();
 
                 var di_fields: std.ArrayListUnmanaged(*llvm.DIType) = .{};
                 defer di_fields.deinit(gpa);
@@ -1790,7 +1791,7 @@ pub const Object = struct {
                     if (field.is_comptime or !field.ty.hasRuntimeBitsIgnoreComptime()) continue;
 
                     const field_size = field.ty.abiSize(target);
-                    const field_align = field.normalAlignment(target);
+                    const field_align = field.alignment(target, layout);
                     const field_offset = std.mem.alignForwardGeneric(u64, offset, field_align);
                     offset = field_offset + field_size;
 
@@ -2425,7 +2426,7 @@ pub const DeclGen = struct {
 
     fn lowerType(dg: *DeclGen, t: Type) Allocator.Error!*const llvm.Type {
         const llvm_ty = try lowerTypeInner(dg, t);
-        if (std.debug.runtime_safety and false) check: {
+        if (std.debug.runtime_safety) check: {
             if (t.zigTypeTag() == .Opaque) break :check;
             if (!t.hasRuntimeBits()) break :check;
             if (!llvm_ty.isSized().toBool()) break :check;
@@ -2684,7 +2685,7 @@ pub const DeclGen = struct {
                 for (struct_obj.fields.values()) |field| {
                     if (field.is_comptime or !field.ty.hasRuntimeBitsIgnoreComptime()) continue;
 
-                    const field_align = field.normalAlignment(target);
+                    const field_align = field.alignment(target, struct_obj.layout);
                     big_align = @maximum(big_align, field_align);
                     const prev_offset = offset;
                     offset = std.mem.alignForwardGeneric(u64, offset, field_align);
@@ -3344,7 +3345,7 @@ pub const DeclGen = struct {
                 for (struct_obj.fields.values()) |field, i| {
                     if (field.is_comptime or !field.ty.hasRuntimeBitsIgnoreComptime()) continue;
 
-                    const field_align = field.normalAlignment(target);
+                    const field_align = field.alignment(target, struct_obj.layout);
                     big_align = @maximum(big_align, field_align);
                     const prev_offset = offset;
                     offset = std.mem.alignForwardGeneric(u64, offset, field_align);
@@ -8884,13 +8885,14 @@ fn llvmFieldIndex(
         }
         return null;
     }
-    assert(ty.containerLayout() != .Packed);
+    const layout = ty.containerLayout();
+    assert(layout != .Packed);
 
     var llvm_field_index: c_uint = 0;
     for (ty.structFields().values()) |field, i| {
         if (field.is_comptime or !field.ty.hasRuntimeBitsIgnoreComptime()) continue;
 
-        const field_align = field.normalAlignment(target);
+        const field_align = field.alignment(target, layout);
         big_align = @maximum(big_align, field_align);
         const prev_offset = offset;
         offset = std.mem.alignForwardGeneric(u64, offset, field_align);

src/type.zig

@@ -3008,6 +3008,15 @@ pub const Type = extern union {
                         },
                     };
                     big_align = @maximum(big_align, field_align);
+
+                    // This logic is duplicated in Module.Struct.Field.alignment.
+                    if (struct_obj.layout == .Extern) {
+                        if (field.ty.isAbiInt() and field.ty.intInfo(target).bits >= 128) {
+                            // The C ABI requires 128 bit integer fields of structs
+                            // to be 16-bytes aligned.
+                            big_align = @maximum(big_align, 16);
+                        }
+                    }
                 }
                 return AbiAlignmentAdvanced{ .scalar = big_align };
             },
@@ -5428,7 +5437,7 @@ pub const Type = extern union {
             .@"struct" => {
                 const struct_obj = ty.castTag(.@"struct").?.data;
                 assert(struct_obj.layout != .Packed);
-                return struct_obj.fields.values()[index].normalAlignment(target);
+                return struct_obj.fields.values()[index].alignment(target, struct_obj.layout);
             },
             .@"union", .union_tagged => {
                 const union_obj = ty.cast(Payload.Union).?.data;
@@ -5535,7 +5544,7 @@ pub const Type = extern union {
             if (!field.ty.hasRuntimeBits() or field.is_comptime)
                 return FieldOffset{ .field = it.field, .offset = it.offset };
 
-            const field_align = field.normalAlignment(it.target);
+            const field_align = field.alignment(it.target, it.struct_obj.layout);
             it.big_align = @maximum(it.big_align, field_align);
             it.offset = std.mem.alignForwardGeneric(u64, it.offset, field_align);
             defer it.offset += field.ty.abiSize(it.target);

test/behavior/align.zig

@@ -143,6 +143,19 @@ test "alignment and size of structs with 128-bit fields" {
         .riscv64,
         .sparc64,
         .x86_64,
+        => .{
+            .a_align = 8,
+            .a_size = 16,
+
+            .b_align = 16,
+            .b_size = 32,
+
+            .u128_align = 8,
+            .u128_size = 16,
+            .u129_align = 8,
+            .u129_size = 24,
+        },
+
         .aarch64,
         .aarch64_be,
         .aarch64_32,
@@ -166,17 +179,17 @@ test "alignment and size of structs with 128-bit fields" {
         else => return error.SkipZigTest,
     };
     comptime {
-        std.debug.assert(@alignOf(A) == expected.a_align);
-        std.debug.assert(@sizeOf(A) == expected.a_size);
+        assert(@alignOf(A) == expected.a_align);
+        assert(@sizeOf(A) == expected.a_size);
 
-        std.debug.assert(@alignOf(B) == expected.b_align);
-        std.debug.assert(@sizeOf(B) == expected.b_size);
+        assert(@alignOf(B) == expected.b_align);
+        assert(@sizeOf(B) == expected.b_size);
 
-        std.debug.assert(@alignOf(u128) == expected.u128_align);
-        std.debug.assert(@sizeOf(u128) == expected.u128_size);
+        assert(@alignOf(u128) == expected.u128_align);
+        assert(@sizeOf(u128) == expected.u128_size);
 
-        std.debug.assert(@alignOf(u129) == expected.u129_align);
-        std.debug.assert(@sizeOf(u129) == expected.u129_size);
+        assert(@alignOf(u129) == expected.u129_align);
+        assert(@sizeOf(u129) == expected.u129_size);
     }
 }