From 342bae02d86d9bd9f2db6ae9489021bab28595ae Mon Sep 17 00:00:00 2001 From: Veikka Tuominen Date: Mon, 16 Jan 2023 19:46:41 +0200 Subject: [PATCH 1/2] Sema: automatically optimize order of struct fields This is a simple starting version of the optimization described in #168 where the fields are just sorted by order of descending alignment. --- src/Module.zig | 42 +++++++++++++++++++++++++++++++++++++++- src/Sema.zig | 36 ++++++++++++++++++++++++++++++++++ src/codegen/llvm.zig | 30 ++++++++++++++-------------- src/type.zig | 6 +++++- test/behavior/struct.zig | 18 +++++++++++++++++ 5 files changed, 115 insertions(+), 17 deletions(-) diff --git a/src/Module.zig b/src/Module.zig index 558ed498ca9d..f2f51907cb61 100644 --- a/src/Module.zig +++ b/src/Module.zig @@ -941,7 +941,8 @@ pub const Struct = struct { owner_decl: Decl.Index, /// Index of the struct_decl ZIR instruction. zir_index: Zir.Inst.Index, - + /// Indexes into `fields` sorted to be most memory efficient. + optimized_order: ?[*]u32 = null, layout: std.builtin.Type.ContainerLayout, /// If the layout is not packed, this is the noreturn type. /// If the layout is packed, this is the backing integer type of the packed struct. @@ -1023,6 +1024,10 @@ pub const Struct = struct { } }; + /// Used in `optimized_order` to indicate field that is not present in the + /// runtime version of the struct. + pub const omitted_field = std.math.maxInt(u32); + pub fn getFullyQualifiedName(s: *Struct, mod: *Module) ![:0]u8 { return mod.declPtr(s.owner_decl).getFullyQualifiedName(mod); } @@ -1098,6 +1103,39 @@ pub const Struct = struct { } unreachable; // index out of bounds } + + pub const RuntimeFieldIterator = struct { + struct_obj: *const Struct, + index: u32 = 0, + + pub const FieldAndIndex = struct { + field: Field, + index: u32, + }; + + pub fn next(it: *RuntimeFieldIterator) ?FieldAndIndex { + while (true) { + var i = it.index; + it.index += 1; + if (it.struct_obj.fields.count() <= i) + return null; + + if (it.struct_obj.optimized_order) |some| { + i = some[i]; + if (i == Module.Struct.omitted_field) return null; + } + const field = it.struct_obj.fields.values()[i]; + + if (!field.is_comptime and field.ty.hasRuntimeBits()) { + return FieldAndIndex{ .index = i, .field = field }; + } + } + } + }; + + pub fn runtimeFieldIterator(s: *const Struct) RuntimeFieldIterator { + return .{ .struct_obj = s }; + } }; /// Represents the data that an enum declaration provides, when the fields @@ -6481,6 +6519,7 @@ pub const Feature = enum { error_return_trace, is_named_enum_value, error_set_has_value, + field_reordering, }; pub fn backendSupportsFeature(mod: Module, feature: Feature) bool { @@ -6493,5 +6532,6 @@ pub fn backendSupportsFeature(mod: Module, feature: Feature) bool { .error_return_trace => mod.comp.bin_file.options.use_llvm, .is_named_enum_value => mod.comp.bin_file.options.use_llvm, .error_set_has_value => mod.comp.bin_file.options.use_llvm, + .field_reordering => mod.comp.bin_file.options.use_llvm, }; } diff --git a/src/Sema.zig b/src/Sema.zig index 3db5eb5ba109..fcf25ab9bfc8 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -29919,6 +29919,42 @@ fn resolveStructLayout(sema: *Sema, ty: Type) CompileError!void { ); return sema.failWithOwnedErrorMsg(msg); } + + if (struct_obj.layout == .Auto and sema.mod.backendSupportsFeature(.field_reordering)) { + const optimized_order = blk: { + const decl = sema.mod.declPtr(struct_obj.owner_decl); + var decl_arena = decl.value_arena.?.promote(sema.mod.gpa); + defer decl.value_arena.?.* = decl_arena.state; + const decl_arena_allocator = decl_arena.allocator(); + + break :blk try decl_arena_allocator.alloc(u32, struct_obj.fields.count()); + }; + + for (struct_obj.fields.values()) |field, i| { + optimized_order[i] = if (field.ty.hasRuntimeBits()) + @intCast(u32, i) + else + Module.Struct.omitted_field; + } + + const AlignSortContext = struct { + struct_obj: *Module.Struct, + sema: *Sema, + + fn lessThan(ctx: @This(), a: u32, b: u32) bool { + if (a == Module.Struct.omitted_field) return false; + if (b == Module.Struct.omitted_field) return true; + const target = ctx.sema.mod.getTarget(); + return ctx.struct_obj.fields.values()[a].ty.abiAlignment(target) > + ctx.struct_obj.fields.values()[b].ty.abiAlignment(target); + } + }; + std.sort.sort(u32, optimized_order, AlignSortContext{ + .struct_obj = struct_obj, + .sema = sema, + }, AlignSortContext.lessThan); + struct_obj.optimized_order = optimized_order.ptr; + } } // otherwise it's a tuple; no need to resolve anything } diff --git a/src/codegen/llvm.zig b/src/codegen/llvm.zig index 8604c7d7f686..b7e1466a2b2a 100644 --- a/src/codegen/llvm.zig +++ b/src/codegen/llvm.zig @@ -2083,15 +2083,15 @@ pub const Object = struct { comptime assert(struct_layout_version == 2); var offset: u64 = 0; - for (fields.values()) |field, i| { - if (field.is_comptime or !field.ty.hasRuntimeBits()) continue; - + var it = ty.castTag(.@"struct").?.data.runtimeFieldIterator(); + while (it.next()) |field_and_index| { + const field = field_and_index.field; const field_size = field.ty.abiSize(target); const field_align = field.alignment(target, layout); const field_offset = std.mem.alignForwardGeneric(u64, offset, field_align); offset = field_offset + field_size; - const field_name = try gpa.dupeZ(u8, fields.keys()[i]); + const field_name = try gpa.dupeZ(u8, fields.keys()[field_and_index.index]); defer gpa.free(field_name); try di_fields.append(gpa, dib.createMemberType( @@ -2985,9 +2985,9 @@ pub const DeclGen = struct { var big_align: u32 = 1; var any_underaligned_fields = false; - for (struct_obj.fields.values()) |field| { - if (field.is_comptime or !field.ty.hasRuntimeBits()) continue; - + var it = struct_obj.runtimeFieldIterator(); + while (it.next()) |field_and_index| { + const field = field_and_index.field; const field_align = field.alignment(target, struct_obj.layout); const field_ty_align = field.ty.abiAlignment(target); any_underaligned_fields = any_underaligned_fields or @@ -3714,9 +3714,9 @@ pub const DeclGen = struct { var big_align: u32 = 0; var need_unnamed = false; - for (struct_obj.fields.values()) |field, i| { - if (field.is_comptime or !field.ty.hasRuntimeBits()) continue; - + var it = struct_obj.runtimeFieldIterator(); + while (it.next()) |field_and_index| { + const field = field_and_index.field; const field_align = field.alignment(target, struct_obj.layout); big_align = @max(big_align, field_align); const prev_offset = offset; @@ -3732,7 +3732,7 @@ pub const DeclGen = struct { const field_llvm_val = try dg.lowerValue(.{ .ty = field.ty, - .val = field_vals[i], + .val = field_vals[field_and_index.index], }); need_unnamed = need_unnamed or dg.isUnnamedType(field.ty, field_llvm_val); @@ -10354,9 +10354,9 @@ fn llvmFieldIndex( assert(layout != .Packed); var llvm_field_index: c_uint = 0; - for (ty.structFields().values()) |field, i| { - if (field.is_comptime or !field.ty.hasRuntimeBits()) continue; - + var it = ty.castTag(.@"struct").?.data.runtimeFieldIterator(); + while (it.next()) |field_and_index| { + const field = field_and_index.field; const field_align = field.alignment(target, layout); big_align = @max(big_align, field_align); const prev_offset = offset; @@ -10367,7 +10367,7 @@ fn llvmFieldIndex( llvm_field_index += 1; } - if (field_index <= i) { + if (field_index == field_and_index.index) { ptr_pl_buf.* = .{ .data = .{ .pointee_type = field.ty, diff --git a/src/type.zig b/src/type.zig index 4c56a7e16758..65391aabfcd4 100644 --- a/src/type.zig +++ b/src/type.zig @@ -5741,10 +5741,14 @@ pub const Type = extern union { target: Target, pub fn next(it: *StructOffsetIterator) ?FieldOffset { - const i = it.field; + var i = it.field; if (it.struct_obj.fields.count() <= i) return null; + if (it.struct_obj.optimized_order) |some| { + i = some[i]; + if (i == Module.Struct.omitted_field) return null; + } const field = it.struct_obj.fields.values()[i]; it.field += 1; diff --git a/test/behavior/struct.zig b/test/behavior/struct.zig index 20a09f92cc69..a32a0ed4950e 100644 --- a/test/behavior/struct.zig +++ b/test/behavior/struct.zig @@ -1555,3 +1555,21 @@ test "optional generic function label struct field" { }; try expect((Options{}).isFoo.?(u8) == 123); } + +test "struct fields get automatically reordered" { + if (builtin.zig_backend != .stage2_llvm) return error.SkipZigTest; // TODO + + const S1 = struct { + a: u32, + b: u32, + c: bool, + d: bool, + }; + const S2 = struct { + a: u32, + b: bool, + c: u32, + d: bool, + }; + try expect(@sizeOf(S1) == @sizeOf(S2)); +} From b2c85464bef931c8263b50b40697a584600ee69e Mon Sep 17 00:00:00 2001 From: Veikka Tuominen Date: Mon, 16 Jan 2023 22:25:34 +0200 Subject: [PATCH 2/2] std.os.linux.io_uring: fix ABI error `register_files_update` was passing a pointer to a Zig struct to the kernel. --- lib/std/os/linux/io_uring.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/std/os/linux/io_uring.zig b/lib/std/os/linux/io_uring.zig index 3bc3fbaf7bf0..61bf39105f2f 100644 --- a/lib/std/os/linux/io_uring.zig +++ b/lib/std/os/linux/io_uring.zig @@ -954,7 +954,7 @@ pub const IO_Uring = struct { pub fn register_files_update(self: *IO_Uring, offset: u32, fds: []const os.fd_t) !void { assert(self.fd >= 0); - const FilesUpdate = struct { + const FilesUpdate = extern struct { offset: u32, resv: u32, fds: u64 align(8),