From b1b3e85dbf2d52b70d8c44c663e83b5c320c6abd Mon Sep 17 00:00:00 2001 From: Matous Kozak <55735845+matouskozak@users.noreply.github.com> Date: Mon, 20 May 2024 16:18:54 +0200 Subject: [PATCH] [mono][swift-interop] Support for Swift frozen struct lowering in mini-JIT/AOT/interpreter (#102143) * refactor + bug fixes for swift struct marshalling * mini struct lowering * interpreter swift struct lowering * enable SwiftAbiStress on Mono --- src/mono/mono/metadata/marshal.c | 72 ++++++++------- src/mono/mono/metadata/marshal.h | 4 +- src/mono/mono/metadata/metadata-internals.h | 1 + src/mono/mono/metadata/metadata.c | 31 +++++++ src/mono/mono/mini/interp/transform.c | 99 +++++++++++++++++++++ src/mono/mono/mini/method-to-ir.c | 78 ++++++++++++++++ src/tests/issues.targets | 3 - 7 files changed, 252 insertions(+), 36 deletions(-) diff --git a/src/mono/mono/metadata/marshal.c b/src/mono/mono/metadata/marshal.c index f82f8cd69fe25..90076ba1994ef 100644 --- a/src/mono/mono/metadata/marshal.c +++ b/src/mono/mono/metadata/marshal.c @@ -3711,7 +3711,7 @@ mono_marshal_get_native_wrapper (MonoMethod *method, gboolean check_exceptions, swift_error_args++; } else if (param_klass == swift_self) { swift_self_args++; - } else if (!m_class_is_blittable (param_klass) || m_class_is_simd_type (param_klass)) { + } else if (!type_is_blittable (method->signature->params [i]) || m_class_is_simd_type (param_klass)) { swift_error_args = swift_self_args = 0; mono_error_set_generic_error (emitted_error, "System", "InvalidProgramException", "Passing non-blittable types to a P/Invoke with the Swift calling convention is unsupported."); break; @@ -6693,7 +6693,8 @@ typedef enum { SWIFT_DOUBLE, } SwiftPhysicalLoweringKind; -static int get_swift_lowering_alignment (SwiftPhysicalLoweringKind kind) { +static int get_swift_lowering_alignment (SwiftPhysicalLoweringKind kind) +{ switch (kind) { case SWIFT_INT64: case SWIFT_DOUBLE: @@ -6705,7 +6706,8 @@ static int get_swift_lowering_alignment (SwiftPhysicalLoweringKind kind) { } } -static void set_lowering_range(guint8* lowered_bytes, guint32 offset, guint32 size, SwiftPhysicalLoweringKind kind) { +static void set_lowering_range(guint8* lowered_bytes, guint32 offset, guint32 size, SwiftPhysicalLoweringKind kind) +{ bool force_opaque = false; if (offset != ALIGN_TO(offset, get_swift_lowering_alignment(kind))) { @@ -6736,7 +6738,8 @@ static void set_lowering_range(guint8* lowered_bytes, guint32 offset, guint32 si static void record_struct_field_physical_lowering (guint8* lowered_bytes, MonoType* type, guint32 offset); -static void record_inlinearray_struct_physical_lowering (guint8* lowered_bytes, MonoClass* klass, guint32 offset) { +static void record_inlinearray_struct_physical_lowering (guint8* lowered_bytes, MonoClass* klass, guint32 offset) +{ // Get the first field and record its physical lowering N times MonoClassField* field = mono_class_get_fields_internal (klass, NULL); MonoType* fieldType = field->type; @@ -6755,17 +6758,21 @@ static void record_struct_physical_lowering (guint8* lowered_bytes, MonoClass* k // For each field, we need to record the physical lowering of it. gpointer iter = NULL; MonoClassField* field; + int type_offset = MONO_ABI_SIZEOF (MonoObject); while ((field = mono_class_get_fields_internal (klass, &iter))) { if (field->type->attrs & FIELD_ATTRIBUTE_STATIC) continue; if (mono_field_is_deleted (field)) continue; - record_struct_field_physical_lowering(lowered_bytes, field->type, offset + m_field_get_offset(field)); + record_struct_field_physical_lowering(lowered_bytes, field->type, (offset + m_field_get_offset(field)) - type_offset); } } -static void record_struct_field_physical_lowering (guint8* lowered_bytes, MonoType* type, guint32 offset) { +static void record_struct_field_physical_lowering (guint8* lowered_bytes, MonoType* type, guint32 offset) +{ + int align; + // Normalize pointer types to IntPtr and resolve generic classes. // We don't need to care about specific pointer types at this ABI level. if (type->type == MONO_TYPE_PTR || type->type == MONO_TYPE_FNPTR) { @@ -6793,7 +6800,7 @@ static void record_struct_field_physical_lowering (guint8* lowered_bytes, MonoTy kind = SWIFT_DOUBLE; } - set_lowering_range(lowered_bytes, offset, mono_type_size(type, NULL), kind); + set_lowering_range(lowered_bytes, offset, mono_type_size(type, &align), kind); } } @@ -6820,13 +6827,13 @@ mono_marshal_get_swift_physical_lowering (MonoType *type, gboolean native_layout } MonoClass *klass = mono_class_from_mono_type_internal (type); - + int vtype_size = mono_class_value_size (klass, NULL); // TODO: We currently don't support vector types, so we can say that the maximum size of a non-by_reference struct // is 4 * PointerSize. // Strictly, this is inaccurate in the case where a struct has a fully-empty 8 bytes of padding using explicit layout, // but that's not possible in the Swift layout algorithm. - if (m_class_get_instance_size(klass) > 4 * TARGET_SIZEOF_VOID_P) { + if (vtype_size > 4 * TARGET_SIZEOF_VOID_P) { lowering.by_reference = TRUE; return lowering; } @@ -6845,8 +6852,7 @@ mono_marshal_get_swift_physical_lowering (MonoType *type, gboolean native_layout GArray* intervals = g_array_new(FALSE, TRUE, sizeof(struct _SwiftInterval)); // Now we'll build the intervals from the lowered_bytes array - int instance_size = m_class_get_instance_size(klass); - for (int i = 0; i < instance_size; ++i) { + for (int i = 0; i < vtype_size; ++i) { // Don't create an interval for empty bytes if (lowered_bytes[i] == SWIFT_EMPTY) { continue; @@ -6874,19 +6880,23 @@ mono_marshal_get_swift_physical_lowering (MonoType *type, gboolean native_layout } // Merge opaque intervals that are in the same pointer-sized block - for (int i = 0; i < intervals->len - 1; ++i) { - struct _SwiftInterval current = g_array_index(intervals, struct _SwiftInterval, i); - struct _SwiftInterval next = g_array_index(intervals, struct _SwiftInterval, i + 1); - - if (current.kind == SWIFT_OPAQUE && next.kind == SWIFT_OPAQUE && current.start / TARGET_SIZEOF_VOID_P == next.start / TARGET_SIZEOF_VOID_P) { - current.size = next.start + next.size - current.start; - g_array_remove_index(intervals, i + 1); - i--; + for (int i = 0; i < intervals->len; ++i) { + struct _SwiftInterval interval = g_array_index(intervals, struct _SwiftInterval, i); + + if (i != 0 && interval.kind == SWIFT_OPAQUE) { + // Merge two opaque intervals when the previous interval ends in the same pointer-sized block + struct _SwiftInterval prevInterval = g_array_index(intervals, struct _SwiftInterval, i - 1); + if (prevInterval.kind == SWIFT_OPAQUE && (prevInterval.start + prevInterval.size) / TARGET_SIZEOF_VOID_P == interval.start / TARGET_SIZEOF_VOID_P) { + (g_array_index(intervals, struct _SwiftInterval, i - 1)).size = interval.start + interval.size - prevInterval.start; + g_array_remove_index(intervals, i); + --i; + continue; + } } } // Now that we have the intervals, we can calculate the lowering - MonoTypeEnum lowered_types[4]; + MonoType *lowered_types[4]; guint32 offsets[4]; guint32 num_lowered_types = 0; @@ -6904,13 +6914,13 @@ mono_marshal_get_swift_physical_lowering (MonoType *type, gboolean native_layout switch (interval.kind) { case SWIFT_INT64: - lowered_types[num_lowered_types++] = MONO_TYPE_I8; + lowered_types[num_lowered_types++] = m_class_get_byval_arg (mono_defaults.int64_class); break; case SWIFT_FLOAT: - lowered_types[num_lowered_types++] = MONO_TYPE_R4; + lowered_types[num_lowered_types++] = m_class_get_byval_arg (mono_defaults.single_class); break; case SWIFT_DOUBLE: - lowered_types[num_lowered_types++] = MONO_TYPE_R8; + lowered_types[num_lowered_types++] = m_class_get_byval_arg (mono_defaults.double_class); break; case SWIFT_OPAQUE: { @@ -6943,20 +6953,20 @@ mono_marshal_get_swift_physical_lowering (MonoType *type, gboolean native_layout offsets[num_lowered_types] = opaque_interval_start; - if (remaining_interval_size > 8 && (opaque_interval_start % 8 == 0)) { - lowered_types[num_lowered_types] = MONO_TYPE_I8; + if (remaining_interval_size > 4 && (opaque_interval_start % 8 == 0)) { + lowered_types[num_lowered_types] = m_class_get_byval_arg (mono_defaults.int64_class); remaining_interval_size -= 8; opaque_interval_start += 8; - } else if (remaining_interval_size > 4 && (opaque_interval_start % 4 == 0)) { - lowered_types[num_lowered_types] = MONO_TYPE_I4; + } else if (remaining_interval_size > 2 && (opaque_interval_start % 4 == 0)) { + lowered_types[num_lowered_types] = m_class_get_byval_arg (mono_defaults.int32_class); remaining_interval_size -= 4; opaque_interval_start += 4; - } else if (remaining_interval_size > 2 && (opaque_interval_start % 2 == 0)) { - lowered_types[num_lowered_types] = MONO_TYPE_I2; + } else if (remaining_interval_size > 1 && (opaque_interval_start % 2 == 0)) { + lowered_types[num_lowered_types] = m_class_get_byval_arg (mono_defaults.int16_class); remaining_interval_size -= 2; opaque_interval_start += 2; } else { - lowered_types[num_lowered_types] = MONO_TYPE_U1; + lowered_types[num_lowered_types] = m_class_get_byval_arg (mono_defaults.byte_class); remaining_interval_size -= 1; opaque_interval_start += 1; } @@ -6967,7 +6977,7 @@ mono_marshal_get_swift_physical_lowering (MonoType *type, gboolean native_layout } } - memcpy(lowering.lowered_elements, lowered_types, num_lowered_types * sizeof(MonoTypeEnum)); + memcpy(lowering.lowered_elements, lowered_types, num_lowered_types * sizeof(MonoType*)); memcpy(lowering.offsets, offsets, num_lowered_types * sizeof(guint32)); lowering.num_lowered_elements = num_lowered_types; lowering.by_reference = FALSE; diff --git a/src/mono/mono/metadata/marshal.h b/src/mono/mono/metadata/marshal.h index d55fd6757d18b..47d47a15db446 100644 --- a/src/mono/mono/metadata/marshal.h +++ b/src/mono/mono/metadata/marshal.h @@ -745,8 +745,8 @@ GENERATE_TRY_GET_CLASS_WITH_CACHE_DECL (swift_error) typedef struct { gboolean by_reference; - int num_lowered_elements; - MonoTypeEnum lowered_elements[4]; + uint32_t num_lowered_elements; + MonoType *lowered_elements[4]; uint32_t offsets[4]; } SwiftPhysicalLowering; diff --git a/src/mono/mono/metadata/metadata-internals.h b/src/mono/mono/metadata/metadata-internals.h index fac7b69e385e9..b35d96457fd9e 100644 --- a/src/mono/mono/metadata/metadata-internals.h +++ b/src/mono/mono/metadata/metadata-internals.h @@ -1010,6 +1010,7 @@ MonoMethodSignature *mono_metadata_signature_dup_mempool (MonoMemPool *mp, Mono MonoMethodSignature *mono_metadata_signature_dup_mem_manager (MonoMemoryManager *mem_manager, MonoMethodSignature *sig); MonoMethodSignature *mono_metadata_signature_dup_add_this (MonoImage *image, MonoMethodSignature *sig, MonoClass *klass); MonoMethodSignature *mono_metadata_signature_dup_delegate_invoke_to_target (MonoMethodSignature *sig); +MonoMethodSignature *mono_metadata_signature_dup_new_params (MonoMemPool *mp, MonoMethodSignature *sig, uint32_t num_params, MonoType **new_params); MonoGenericInst * mono_get_shared_generic_inst (MonoGenericContainer *container); diff --git a/src/mono/mono/metadata/metadata.c b/src/mono/mono/metadata/metadata.c index 3b05c485137b9..0bb9ded0b66e4 100644 --- a/src/mono/mono/metadata/metadata.c +++ b/src/mono/mono/metadata/metadata.c @@ -2546,6 +2546,37 @@ mono_metadata_signature_dup_delegate_invoke_to_target (MonoMethodSignature *sig) return res; } +/** + * mono_metadata_signature_dup_new_params: + * @param mp The memory pool to allocate the duplicated signature from. + * @param sig The original method signature. + * @param num_params The number parameters in the new signature. + * @param new_params An array of MonoType pointers representing the new parameters. + * + * Duplicate an existing \c MonoMethodSignature but with a new set of parameters. + * This is a Mono runtime internal function. + * + * @return the new \c MonoMethodSignature structure. + */ +MonoMethodSignature* +mono_metadata_signature_dup_new_params (MonoMemPool *mp, MonoMethodSignature *sig, uint32_t num_params, MonoType **new_params) +{ + size_t new_sig_size = MONO_SIZEOF_METHOD_SIGNATURE + num_params * sizeof (MonoType*); + if (sig->ret) + new_sig_size += mono_sizeof_type (sig->ret); + + MonoMethodSignature *res = (MonoMethodSignature *)mono_mempool_alloc0 (mp, (unsigned int)new_sig_size); + memcpy (res, sig, MONO_SIZEOF_METHOD_SIGNATURE); + res->param_count = GUINT32_TO_UINT16 (num_params); + + for (uint16_t i = 0; i < res->param_count; i++) { + res->params [i] = new_params [i]; + } + res->ret = sig->ret; + + return res; +} + /* * mono_metadata_signature_size: * diff --git a/src/mono/mono/mini/interp/transform.c b/src/mono/mono/mini/interp/transform.c index d8d06e74ac75d..88f386802cc2a 100644 --- a/src/mono/mono/mini/interp/transform.c +++ b/src/mono/mono/mini/interp/transform.c @@ -3320,6 +3320,95 @@ interp_try_devirt (MonoClass *this_klass, MonoMethod *target_method) return NULL; } +static MonoMethodSignature* +interp_emit_swiftcall_struct_lowering (TransformData *td, MonoMethodSignature *csignature) +{ + // P/Invoke calls shouldn't contain 'this' + g_assert (!csignature->hasthis); + + /* + * Argument reordering here doesn't handle on the fly offset allocation + * and requires the full var offset allocator pass that is only ran for optimized code + */ + g_assert (td->optimized); + + MonoMethodSignature *new_csignature; + // Save the function pointer + StackInfo sp_fp = td->sp [-1]; + --td->sp; + + // Save the old arguments + td->sp -= csignature->param_count; + StackInfo *sp_old_params = (StackInfo*) mono_mempool_alloc (td->mempool, sizeof (StackInfo) * csignature->param_count); + for (int i = 0; i < csignature->param_count; ++i) + sp_old_params [i] = td->sp [i]; + + GArray *new_params = g_array_sized_new (FALSE, FALSE, sizeof (MonoType*), csignature->param_count); + uint32_t new_param_count = 0; + int align; + MonoClass *swift_self = mono_class_try_get_swift_self_class (); + MonoClass *swift_error = mono_class_try_get_swift_error_class (); + /* + * Go through the lowered arguments, if the argument is a struct, + * we need to replace it with a sequence of lowered arguments. + * Also record the updated parameters for the new signature. + */ + for (int idx_param = 0; idx_param < csignature->param_count; ++idx_param) { + MonoType *ptype = csignature->params [idx_param]; + MonoClass *klass = mono_class_from_mono_type_internal (ptype); + // SwiftSelf and SwiftError are special cases where we need to preserve the class information for the codegen to handle them correctly. + if (mono_type_is_struct (ptype) && !(klass == swift_self || klass == swift_error)) { + SwiftPhysicalLowering lowered_swift_struct = mono_marshal_get_swift_physical_lowering (ptype, FALSE); + if (!lowered_swift_struct.by_reference) { + for (uint32_t idx_lowered = 0; idx_lowered < lowered_swift_struct.num_lowered_elements; ++idx_lowered) { + int mt_lowered = mono_mint_type (lowered_swift_struct.lowered_elements [idx_lowered]); + int lowered_elem_size = mono_type_size (lowered_swift_struct.lowered_elements [idx_lowered], &align); + // Load the lowered elements of the struct + interp_add_ins (td, MINT_MOV_SRC_OFF); + interp_ins_set_sreg (td->last_ins, sp_old_params [idx_param].var); + td->last_ins->data [0] = (guint16) lowered_swift_struct.offsets [idx_lowered]; + td->last_ins->data [1] = GINT_TO_UINT16 (mt_lowered); + td->last_ins->data [2] = GINT_TO_UINT16 (lowered_elem_size); + push_mono_type (td, lowered_swift_struct.lowered_elements [idx_lowered], mt_lowered, mono_class_from_mono_type_internal (lowered_swift_struct.lowered_elements [idx_lowered])); + interp_ins_set_dreg (td->last_ins, td->sp [-1].var); + + ++new_param_count; + g_array_append_val (new_params, lowered_swift_struct.lowered_elements [idx_lowered]); + } + } else { + // For structs that cannot be lowered, we change the argument to byref type + ptype = mono_class_get_byref_type (mono_defaults.typed_reference_class); + // Load the address of the struct + interp_add_ins (td, MINT_LDLOCA_S); + interp_ins_set_sreg (td->last_ins, sp_old_params [idx_param].var); + push_simple_type (td, STACK_TYPE_I); + interp_ins_set_dreg (td->last_ins, td->sp [-1].var); + + ++new_param_count; + g_array_append_val (new_params, ptype); + } + } else { + // Copy over non-struct arguments + memcpy (td->sp, &sp_old_params [idx_param], sizeof (StackInfo)); + ++td->sp; + + ++new_param_count; + g_array_append_val (new_params, ptype); + } + } + // Restore the function pointer + memcpy (td->sp, &sp_fp, sizeof (StackInfo)); + ++td->sp; + + // Create a new dummy signature with the lowered arguments + new_csignature = mono_metadata_signature_dup_new_params (td->mempool, csignature, new_param_count, (MonoType**)new_params->data); + + // Deallocate temp array + g_array_free (new_params, TRUE); + + return new_csignature; +} + /* Return FALSE if error, including inline failure */ static gboolean interp_transform_call (TransformData *td, MonoMethod *method, MonoMethod *target_method, MonoGenericContext *generic_context, MonoClass *constrained_class, gboolean readonly, MonoError *error, gboolean check_visibility, gboolean save_last_error, gboolean tailcall) @@ -3404,6 +3493,16 @@ interp_transform_call (TransformData *td, MonoMethod *method, MonoMethod *target return FALSE; } +#ifdef MONO_ARCH_HAVE_SWIFTCALL + /* + * We need to modify the signature of the swiftcall calli to account for the lowering of Swift structs. + * This is done by replacing struct arguments on stack with a lowered sequence and updating the signature. + */ + if (csignature->pinvoke && mono_method_signature_has_ext_callconv (csignature, MONO_EXT_CALLCONV_SWIFTCALL)) { + csignature = interp_emit_swiftcall_struct_lowering (td, csignature); + } +#endif + if (check_visibility && target_method && !mono_method_can_access_method (method, target_method)) interp_generate_mae_throw (td, method, target_method); diff --git a/src/mono/mono/mini/method-to-ir.c b/src/mono/mono/mini/method-to-ir.c index 40905c0ceb48f..90a8f3f341e8f 100644 --- a/src/mono/mono/mini/method-to-ir.c +++ b/src/mono/mono/mini/method-to-ir.c @@ -7514,6 +7514,84 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b if (cfg->gsharedvt_min && mini_is_gsharedvt_variable_signature (fsig)) GSHAREDVT_FAILURE (il_op); +#ifdef MONO_ARCH_HAVE_SWIFTCALL + /* + * We need to modify the signature of the swiftcall calli to account for the lowering of Swift structs. + * This is done by replacing struct arguments on stack with a lowered sequence and updating the signature. + */ + if (fsig->pinvoke && mono_method_signature_has_ext_callconv (fsig, MONO_EXT_CALLCONV_SWIFTCALL)) { + g_assert (!fsig->hasthis); // Swift P/Invoke calls shouldn't contain 'this' + + n = fsig->param_count; + sp -= n; + // Save the old arguments + MonoInst **old_params = (MonoInst**) mono_mempool_alloc (cfg->mempool, sizeof (MonoInst*) * n); + for (int idx_param = 0; idx_param < n; ++idx_param) { + old_params [idx_param] = sp [idx_param]; + } + + GArray *new_params = g_array_sized_new (FALSE, FALSE, sizeof (MonoType*), n); + uint32_t new_param_count = 0; + MonoClass *swift_self = mono_class_try_get_swift_self_class (); + MonoClass *swift_error = mono_class_try_get_swift_error_class (); + /* + * Go through the lowered arguments, if the argument is a struct, + * we need to replace it with a sequence of lowered arguments. + * Also record the updated parameters for the new signature. + */ + for (int idx_param = 0; idx_param < n; ++idx_param) { + MonoType *ptype = fsig->params [idx_param]; + MonoClass *klass = mono_class_from_mono_type_internal (ptype); + + // SwiftSelf and SwiftError are special cases where we need to preserve the class information for the codegen to handle them correctly. + if (mono_type_is_struct (ptype) && !(klass == swift_self || klass == swift_error)) { + SwiftPhysicalLowering lowered_swift_struct = mono_marshal_get_swift_physical_lowering (ptype, FALSE); + if (!lowered_swift_struct.by_reference) { + // Create a new local variable to store the base address of the struct + MonoInst *struct_base_address = mono_compile_create_var (cfg, mono_get_int_type (), OP_LOCAL); + CHECK_ARG (idx_param); + NEW_ARGLOADA (cfg, struct_base_address, idx_param); + MONO_ADD_INS (cfg->cbb, struct_base_address); + + for (uint32_t idx_lowered = 0; idx_lowered < lowered_swift_struct.num_lowered_elements; ++idx_lowered) { + MonoInst *lowered_arg = NULL; + // Load the lowered elements of the struct + lowered_arg = mini_emit_memory_load (cfg, lowered_swift_struct.lowered_elements [idx_lowered], struct_base_address, lowered_swift_struct.offsets [idx_lowered], 0); + *sp++ = lowered_arg; + + ++new_param_count; + g_array_append_val (new_params, lowered_swift_struct.lowered_elements [idx_lowered]); + } + } else { + // For structs that cannot be lowered, we change the argument to byref type + ptype = mono_class_get_byref_type (mono_defaults.typed_reference_class); + // Load the address of the struct + MonoInst *struct_base_address = mono_compile_create_var (cfg, mono_get_int_type (), OP_LOCAL); + CHECK_ARG (idx_param); + NEW_ARGLOADA (cfg, struct_base_address, idx_param); + MONO_ADD_INS (cfg->cbb, struct_base_address); + *sp++ = struct_base_address; + + ++new_param_count; + g_array_append_val (new_params, ptype); + } + } else { + // Copy over non-struct arguments + *sp++ = old_params [idx_param]; + + ++new_param_count; + g_array_append_val (new_params, ptype); + } + } + + // Create a new dummy signature with the lowered arguments + fsig = mono_metadata_signature_dup_new_params (cfg->mempool, fsig, new_param_count, (MonoType**)new_params->data); + + // Deallocate temp array + g_array_free (new_params, TRUE); + } +#endif + if (method->dynamic && fsig->pinvoke) { MonoInst *args [3]; diff --git a/src/tests/issues.targets b/src/tests/issues.targets index a8b94f6576cf2..d8e74aacc8009 100644 --- a/src/tests/issues.targets +++ b/src/tests/issues.targets @@ -1879,9 +1879,6 @@ https://github.com/dotnet/runtime/issues/98628 - - https://github.com/dotnet/runtime/issues/93631: Swift frozen struct support is not implemented on Mono yet - https://github.com/dotnet/runtime/issues/93631: Swift frozen struct support is not implemented on Mono yet