From 6e313eb1107d4f5d7b0ada0a67c810ce90e79bf5 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Sat, 16 Jul 2022 16:27:37 -0700 Subject: [PATCH] stage2: agree with LLVM that `@alignOf(u128)` is 8 on x86_64 and similar targets. --- lib/std/target.zig | 7 ++++--- src/Module.zig | 30 +++++++++++++++++++++++++----- src/Sema.zig | 5 +---- src/codegen/llvm.zig | 14 ++++++++------ src/type.zig | 13 +++++++++++-- test/behavior/align.zig | 29 +++++++++++++++++++++-------- 6 files changed, 70 insertions(+), 28 deletions(-) diff --git a/lib/std/target.zig b/lib/std/target.zig index f77f73bce0..155c59bdfc 100644 --- a/lib/std/target.zig +++ b/lib/std/target.zig @@ -1806,9 +1806,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. @@ -1819,6 +1819,7 @@ pub const Target = struct { .mips64, .mips64el, .sparc64, + => 8, // Even LLVMABIAlignmentOfType(i128) agrees on these targets. .aarch64, diff --git a/src/Module.zig b/src/Module.zig index 995fdda7ea..6d2180e8e7 100644 --- a/src/Module.zig +++ b/src/Module.zig @@ -935,13 +935,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; + }, + } } }; diff --git a/src/Sema.zig b/src/Sema.zig index d6ac3d0276..32b95a4c21 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -14380,10 +14380,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, diff --git a/src/codegen/llvm.zig b/src/codegen/llvm.zig index 0586c99432..a9a343439d 100644 --- a/src/codegen/llvm.zig +++ b/src/codegen/llvm.zig @@ -1841,6 +1841,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); @@ -1854,7 +1855,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; @@ -2499,7 +2500,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; @@ -2757,7 +2758,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); const field_ty_align = field.ty.abiAlignment(target); any_underaligned_fields = any_underaligned_fields or field_align < field_ty_align; @@ -3433,7 +3434,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); @@ -9376,13 +9377,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); diff --git a/src/type.zig b/src/type.zig index 700f524556..4432ae9cf3 100644 --- a/src/type.zig +++ b/src/type.zig @@ -3017,6 +3017,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 }; }, @@ -5490,7 +5499,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_safety_tagged, .union_tagged => { const union_obj = ty.cast(Payload.Union).?.data; @@ -5597,7 +5606,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); diff --git a/test/behavior/align.zig b/test/behavior/align.zig index 26e3d91373..4a824bc9cf 100644 --- a/test/behavior/align.zig +++ b/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); } }