Merge pull request #8616 from LemonBoy/fn-align

Function pointer alignment

5 files changed, 49 insertions, 15 deletions

diff --git a/src/stage1/analyze.cpp b/src/stage1/analyze.cpp
index 21ad55b8f7..f76ea85ca9 100644
--- a/src/stage1/analyze.cpp
+++ b/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 {
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 c59f63399c..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);
@@ -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"));
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