From 50a8124f45cd0994b52af3fd166dd3bda48f4b99 Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Sun, 25 Apr 2021 16:40:41 +0200 Subject: stage1: Change how the Frame alignment is computed The code would previously assume every function would start at addresses being multiples of 16, this is not true beside some specific cases. Moreover LLVM picks different alignment values depending on whether it's trying to generate dense or fast code. Let's use the minimum guaranteed alignment as base value, computed according to how big the opcodes are. The alignment of function pointers is always 1, a safe value that won't cause any error at runtime. Note that this was already the case before this commit, here we're making this choice explicit. Let the 'alignment' field for TypeInfo of fn types reflect the ABI alignment used by the compiler, make this field behave similarly to the 'alignment' one for pointers. --- src/stage1/analyze.cpp | 15 ++++++--------- src/stage1/ir.cpp | 4 ++-- src/stage1/target.cpp | 33 ++++++++++++++++++++++++++++++++- src/stage1/target.hpp | 1 + 4 files changed, 41 insertions(+), 12 deletions(-) (limited to 'src') diff --git a/src/stage1/analyze.cpp b/src/stage1/analyze.cpp index 21ad55b8f7..e1e0c496f6 100644 --- a/src/stage1/analyze.cpp +++ b/src/stage1/analyze.cpp @@ -4770,10 +4770,10 @@ Error type_is_nonnull_ptr2(CodeGen *g, ZigType *type, bool *result) { } static uint32_t get_async_frame_align_bytes(CodeGen *g) { - uint32_t a = g->pointer_size_bytes * 2; - // promises have at least alignment 8 so that we can have 3 extra bits when doing atomicrmw - if (a < 8) a = 8; - return a; + // Due to how the frame structure is built the minimum alignment is the one + // of a usize (or pointer). + // label (grep this): [fn_frame_struct_layout] + return max(g->builtin_types.entry_usize->abi_align, target_fn_align(g->zig_target)); } uint32_t get_ptr_align(CodeGen *g, ZigType *type) { @@ -4789,11 +4789,8 @@ uint32_t get_ptr_align(CodeGen *g, ZigType *type) { return (ptr_type->data.pointer.explicit_alignment == 0) ? get_abi_alignment(g, ptr_type->data.pointer.child_type) : ptr_type->data.pointer.explicit_alignment; } else if (ptr_type->id == ZigTypeIdFn) { - // I tried making this use LLVMABIAlignmentOfType but it trips this assertion in LLVM: - // "Cannot getTypeInfo() on a type that is unsized!" - // when getting the alignment of `?fn() callconv(.C) void`. - // See http://lists.llvm.org/pipermail/llvm-dev/2018-September/126142.html - return (ptr_type->data.fn.fn_type_id.alignment == 0) ? 1 : ptr_type->data.fn.fn_type_id.alignment; + return (ptr_type->data.fn.fn_type_id.alignment == 0) ? + target_fn_ptr_align(g->zig_target) : ptr_type->data.fn.fn_type_id.alignment; } else if (ptr_type->id == ZigTypeIdAnyFrame) { return get_async_frame_align_bytes(g); } else { diff --git a/src/stage1/ir.cpp b/src/stage1/ir.cpp index c59f63399c..176d50dc72 100644 --- a/src/stage1/ir.cpp +++ b/src/stage1/ir.cpp @@ -26079,11 +26079,11 @@ static Error ir_make_type_info_value(IrAnalyze *ira, IrInst* source_instr, ZigTy fields[0]->special = ConstValSpecialStatic; fields[0]->type = get_builtin_type(ira->codegen, "CallingConvention"); bigint_init_unsigned(&fields[0]->data.x_enum_tag, type_entry->data.fn.fn_type_id.cc); - // alignment: u29 + // alignment: comptime_int ensure_field_index(result->type, "alignment", 1); fields[1]->special = ConstValSpecialStatic; fields[1]->type = ira->codegen->builtin_types.entry_num_lit_int; - bigint_init_unsigned(&fields[1]->data.x_bigint, type_entry->data.fn.fn_type_id.alignment); + bigint_init_unsigned(&fields[1]->data.x_bigint, get_ptr_align(ira->codegen, type_entry)); // is_generic: bool ensure_field_index(result->type, "is_generic", 2); bool is_generic = type_entry->data.fn.is_generic; diff --git a/src/stage1/target.cpp b/src/stage1/target.cpp index 028f6e5fa8..6aa3cfcbd0 100644 --- a/src/stage1/target.cpp +++ b/src/stage1/target.cpp @@ -1253,6 +1253,37 @@ bool target_is_ppc(const ZigTarget *target) { target->arch == ZigLLVM_ppc64le; } +// Returns the minimum alignment for every function pointer on the given +// architecture. +unsigned target_fn_ptr_align(const ZigTarget *target) { + // TODO This is a pessimization but is always correct. + return 1; +} + +// Returns the minimum alignment for every function on the given architecture. unsigned target_fn_align(const ZigTarget *target) { - return 16; + switch (target->arch) { + case ZigLLVM_riscv32: + case ZigLLVM_riscv64: + // TODO If the C extension is not present the value is 4. + return 2; + case ZigLLVM_ppc: + case ZigLLVM_ppcle: + case ZigLLVM_ppc64: + case ZigLLVM_ppc64le: + case ZigLLVM_aarch64: + case ZigLLVM_aarch64_be: + case ZigLLVM_aarch64_32: + case ZigLLVM_sparc: + case ZigLLVM_sparcel: + case ZigLLVM_sparcv9: + case ZigLLVM_mips: + case ZigLLVM_mipsel: + case ZigLLVM_mips64: + case ZigLLVM_mips64el: + return 4; + + default: + return 1; + } } diff --git a/src/stage1/target.hpp b/src/stage1/target.hpp index 099c3c1e78..362d63eb32 100644 --- a/src/stage1/target.hpp +++ b/src/stage1/target.hpp @@ -98,6 +98,7 @@ size_t target_libc_count(void); void target_libc_enum(size_t index, ZigTarget *out_target); bool target_libc_needs_crti_crtn(const ZigTarget *target); +unsigned target_fn_ptr_align(const ZigTarget *target); unsigned target_fn_align(const ZigTarget *target); #endif -- cgit v1.2.3 From ae15022406e5d59787195130cfd5261c6336f41c Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Sun, 25 Apr 2021 16:50:41 +0200 Subject: translate-c: Fix casting of function pointers The @ptrCast(X, @alignCast(@alignOf(T), Y)) pattern is only correct if T is not a function type or a pointer, in that case the @alignOf refers to the pointer itself and not to the pointee type. --- src/translate_c.zig | 2 +- src/translate_c/ast.zig | 12 +++++++++++- test/translate_c.zig | 18 +++++++++--------- 3 files changed, 21 insertions(+), 11 deletions(-) (limited to 'src') diff --git a/src/translate_c.zig b/src/translate_c.zig index 60b3961e6e..0a722ab3b4 100644 --- a/src/translate_c.zig +++ b/src/translate_c.zig @@ -3539,7 +3539,7 @@ fn transCPtrCast( expr else blk: { const child_type_node = try transQualType(c, scope, child_type, loc); - const alignof = try Tag.alignof.create(c.arena, child_type_node); + const alignof = try Tag.std_meta_alignment.create(c.arena, child_type_node); const align_cast = try Tag.align_cast.create(c.arena, .{ .lhs = alignof, .rhs = expr }); break :blk align_cast; }; diff --git a/src/translate_c/ast.zig b/src/translate_c/ast.zig index 61d28bb22d..2e75bf1182 100644 --- a/src/translate_c/ast.zig +++ b/src/translate_c/ast.zig @@ -120,8 +120,11 @@ pub const Node = extern union { std_math_Log2Int, /// @intCast(lhs, rhs) int_cast, - /// @rem(lhs, rhs) + /// @import("std").meta.promoteIntLiteral(value, type, radix) std_meta_promoteIntLiteral, + /// @import("std").meta.alignment(value) + std_meta_alignment, + /// @rem(lhs, rhs) rem, /// @divTrunc(lhs, rhs) div_trunc, @@ -260,6 +263,7 @@ pub const Node = extern union { .switch_else, .block_single, .std_meta_sizeof, + .std_meta_alignment, .bool_to_int, .sizeof, .alignof, @@ -876,6 +880,11 @@ fn renderNode(c: *Context, node: Node) Allocator.Error!NodeIndex { const import_node = try renderStdImport(c, "meta", "promoteIntLiteral"); return renderCall(c, import_node, &.{ payload.type, payload.value, payload.radix }); }, + .std_meta_alignment => { + const payload = node.castTag(.std_meta_alignment).?.data; + const import_node = try renderStdImport(c, "meta", "alignment"); + return renderCall(c, import_node, &.{payload}); + }, .std_meta_sizeof => { const payload = node.castTag(.std_meta_sizeof).?.data; const import_node = try renderStdImport(c, "meta", "sizeof"); @@ -2144,6 +2153,7 @@ fn renderNodeGrouped(c: *Context, node: Node) !NodeIndex { .typeof, .typeinfo, .std_meta_sizeof, + .std_meta_alignment, .std_meta_cast, .std_meta_promoteIntLiteral, .std_meta_vector, diff --git a/test/translate_c.zig b/test/translate_c.zig index 846eec0d62..4021210b29 100644 --- a/test/translate_c.zig +++ b/test/translate_c.zig @@ -1363,7 +1363,7 @@ pub fn addCases(cases: *tests.TranslateCContext) void { , &[_][]const u8{ \\pub export fn ptrcast() [*c]f32 { \\ var a: [*c]c_int = undefined; - \\ return @ptrCast([*c]f32, @alignCast(@alignOf(f32), a)); + \\ return @ptrCast([*c]f32, @alignCast(@import("std").meta.alignment(f32), a)); \\} }); @@ -1387,16 +1387,16 @@ pub fn addCases(cases: *tests.TranslateCContext) void { \\pub export fn test_ptr_cast() void { \\ var p: ?*c_void = undefined; \\ { - \\ var to_char: [*c]u8 = @ptrCast([*c]u8, @alignCast(@alignOf(u8), p)); - \\ var to_short: [*c]c_short = @ptrCast([*c]c_short, @alignCast(@alignOf(c_short), p)); - \\ var to_int: [*c]c_int = @ptrCast([*c]c_int, @alignCast(@alignOf(c_int), p)); - \\ var to_longlong: [*c]c_longlong = @ptrCast([*c]c_longlong, @alignCast(@alignOf(c_longlong), p)); + \\ var to_char: [*c]u8 = @ptrCast([*c]u8, @alignCast(@import("std").meta.alignment(u8), p)); + \\ var to_short: [*c]c_short = @ptrCast([*c]c_short, @alignCast(@import("std").meta.alignment(c_short), p)); + \\ var to_int: [*c]c_int = @ptrCast([*c]c_int, @alignCast(@import("std").meta.alignment(c_int), p)); + \\ var to_longlong: [*c]c_longlong = @ptrCast([*c]c_longlong, @alignCast(@import("std").meta.alignment(c_longlong), p)); \\ } \\ { - \\ var to_char: [*c]u8 = @ptrCast([*c]u8, @alignCast(@alignOf(u8), p)); - \\ var to_short: [*c]c_short = @ptrCast([*c]c_short, @alignCast(@alignOf(c_short), p)); - \\ var to_int: [*c]c_int = @ptrCast([*c]c_int, @alignCast(@alignOf(c_int), p)); - \\ var to_longlong: [*c]c_longlong = @ptrCast([*c]c_longlong, @alignCast(@alignOf(c_longlong), p)); + \\ var to_char: [*c]u8 = @ptrCast([*c]u8, @alignCast(@import("std").meta.alignment(u8), p)); + \\ var to_short: [*c]c_short = @ptrCast([*c]c_short, @alignCast(@import("std").meta.alignment(c_short), p)); + \\ var to_int: [*c]c_int = @ptrCast([*c]c_int, @alignCast(@import("std").meta.alignment(c_int), p)); + \\ var to_longlong: [*c]c_longlong = @ptrCast([*c]c_longlong, @alignCast(@import("std").meta.alignment(c_longlong), p)); \\ } \\} }); -- cgit v1.2.3 From 82f1d592fae021fcfc737e3cb6c107b325fcf1ee Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Sun, 25 Apr 2021 19:42:59 +0200 Subject: stage1: Use correct alignment for asyncCall frame --- src/stage1/analyze.cpp | 2 +- src/stage1/analyze.hpp | 1 + src/stage1/ir.cpp | 8 ++++++-- test/compile_errors.zig | 4 +++- 4 files changed, 11 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/stage1/analyze.cpp b/src/stage1/analyze.cpp index e1e0c496f6..f76ea85ca9 100644 --- a/src/stage1/analyze.cpp +++ b/src/stage1/analyze.cpp @@ -4769,7 +4769,7 @@ Error type_is_nonnull_ptr2(CodeGen *g, ZigType *type, bool *result) { return ErrorNone; } -static uint32_t get_async_frame_align_bytes(CodeGen *g) { +uint32_t get_async_frame_align_bytes(CodeGen *g) { // Due to how the frame structure is built the minimum alignment is the one // of a usize (or pointer). // label (grep this): [fn_frame_struct_layout] diff --git a/src/stage1/analyze.hpp b/src/stage1/analyze.hpp index cea48b893c..2815274f63 100644 --- a/src/stage1/analyze.hpp +++ b/src/stage1/analyze.hpp @@ -47,6 +47,7 @@ ZigType *get_test_fn_type(CodeGen *g); ZigType *get_any_frame_type(CodeGen *g, ZigType *result_type); bool handle_is_ptr(CodeGen *g, ZigType *type_entry); Error emit_error_unless_callconv_allowed_for_target(CodeGen *g, AstNode *source_node, CallingConvention cc); +uint32_t get_async_frame_align_bytes(CodeGen *g); bool type_has_bits(CodeGen *g, ZigType *type_entry); Error type_has_bits2(CodeGen *g, ZigType *type_entry, bool *result); diff --git a/src/stage1/ir.cpp b/src/stage1/ir.cpp index 176d50dc72..7edfe37c92 100644 --- a/src/stage1/ir.cpp +++ b/src/stage1/ir.cpp @@ -20659,8 +20659,12 @@ static IrInstGen *analyze_casted_new_stack(IrAnalyze *ira, IrInst* source_instr, get_fn_frame_type(ira->codegen, fn_entry), false); return ir_implicit_cast(ira, new_stack, needed_frame_type); } else { + // XXX The stack alignment is hardcoded to 16 here and in + // std.Target.stack_align. + const uint32_t required_align = is_async_call_builtin ? + get_async_frame_align_bytes(ira->codegen) : 16; ZigType *u8_ptr = get_pointer_to_type_extra(ira->codegen, ira->codegen->builtin_types.entry_u8, - false, false, PtrLenUnknown, target_fn_align(ira->codegen->zig_target), 0, 0, false); + false, false, PtrLenUnknown, required_align, 0, 0, false); ZigType *u8_slice = get_slice_type(ira->codegen, u8_ptr); ira->codegen->need_frame_size_prefix_data = true; return ir_implicit_cast2(ira, new_stack_src, new_stack, u8_slice); @@ -30095,7 +30099,7 @@ static IrInstGen *ir_align_cast(IrAnalyze *ira, IrInstGen *target, uint32_t alig fn_type_id.alignment = align_bytes; result_type = get_fn_type(ira->codegen, &fn_type_id); } else if (target_type->id == ZigTypeIdAnyFrame) { - if (align_bytes >= target_fn_align(ira->codegen->zig_target)) { + if (align_bytes >= get_async_frame_align_bytes(ira->codegen)) { result_type = target_type; } else { ir_add_error(ira, &target->base, buf_sprintf("sub-aligned anyframe not allowed")); diff --git a/test/compile_errors.zig b/test/compile_errors.zig index bfa9b592b4..5b36027248 100644 --- a/test/compile_errors.zig +++ b/test/compile_errors.zig @@ -2136,7 +2136,9 @@ pub fn addCases(cases: *tests.CompileErrorContext) void { \\} \\fn func() callconv(.Async) void {} , &[_][]const u8{ - "tmp.zig:4:21: error: expected type '[]align(16) u8', found '*[64]u8'", + // Split the check in two as the alignment value is target dependent. + "tmp.zig:4:21: error: expected type '[]align(", + ") u8', found '*[64]u8'", }); cases.add("atomic orderings of fence Acquire or stricter", -- cgit v1.2.3