Merge pull request #8616 from LemonBoy/fn-align

Function pointer alignment

ci/drone/linux_script_test

@@ -22,7 +22,9 @@ case "$1" in
     steps="\
       test-compiler-rt \
       test-minilibc \
-      test-compare-output"
+      test-compare-output \
+      test-translate-c \
+      test-run-translated-c"
     ;;
   '')
     echo "error: expecting test group argument"

lib/std/meta.zig

@@ -117,10 +117,21 @@ test "std.meta.bitCount" {
     testing.expect(bitCount(f32) == 32);
 }
 
+/// Returns the alignment of type T.
+/// Note that if T is a pointer or function type the result is different than
+/// the one returned by @alignOf(T).
+/// If T is a pointer type the alignment of the type it points to is returned.
+/// If T is a function type the alignment a target-dependent value is returned.
 pub fn alignment(comptime T: type) comptime_int {
-    //@alignOf works on non-pointer types
-    const P = if (comptime trait.is(.Pointer)(T)) T else *T;
-    return @typeInfo(P).Pointer.alignment;
+    return switch (@typeInfo(T)) {
+        .Optional => |info| switch (@typeInfo(info.child)) {
+            .Pointer, .Fn => alignment(info.child),
+            else => @alignOf(T),
+        },
+        .Pointer => |info| info.alignment,
+        .Fn => |info| info.alignment,
+        else => @alignOf(T),
+    };
 }
 
 test "std.meta.alignment" {
@@ -129,6 +140,8 @@ test "std.meta.alignment" {
     testing.expect(alignment(*align(2) u8) == 2);
     testing.expect(alignment([]align(1) u8) == 1);
     testing.expect(alignment([]align(2) u8) == 2);
+    testing.expect(alignment(fn () void) > 0);
+    testing.expect(alignment(fn () align(128) void) == 128);
 }
 
 pub fn Child(comptime T: type) type {

src/stage1/analyze.cpp

@@ -4769,11 +4769,11 @@ 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 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;
+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]
+    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 {

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);

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);
@@ -26079,11 +26083,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;
@@ -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"));

src/stage1/target.cpp

@@ -1253,6 +1253,37 @@ bool target_is_ppc(const ZigTarget *target) {
         target->arch == ZigLLVM_ppc64le;
 }
 
-unsigned target_fn_align(const ZigTarget *target) {
-    return 16;
+// 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) {
+    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;
+    }
 }

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

src/translate_c.zig

@@ -3540,7 +3540,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;
         };

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,

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",

test/stage1/behavior/type_info.zig

@@ -306,7 +306,7 @@ test "type info: function type info" {
 fn testFunction() void {
     const fn_info = @typeInfo(@TypeOf(foo));
     expect(fn_info == .Fn);
-    expect(fn_info.Fn.alignment == 0);
+    expect(fn_info.Fn.alignment > 0);
     expect(fn_info.Fn.calling_convention == .C);
     expect(!fn_info.Fn.is_generic);
     expect(fn_info.Fn.args.len == 2);

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));
         \\    }
         \\}
     });