From d664b6963ea0d064eaf32021496d72593aaaaa90 Mon Sep 17 00:00:00 2001 From: Matous Kozak Date: Fri, 10 May 2024 10:39:01 +0200 Subject: [PATCH 01/13] refactor + bug fixes for swift struct marshalling --- src/mono/mono/metadata/marshal.c | 64 ++++++++++++++++++-------------- src/mono/mono/metadata/marshal.h | 4 +- 2 files changed, 39 insertions(+), 29 deletions(-) diff --git a/src/mono/mono/metadata/marshal.c b/src/mono/mono/metadata/marshal.c index aad740a31d90e5..d045bf2211aa4b 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; @@ -6665,13 +6665,14 @@ 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); } } @@ -6703,7 +6704,8 @@ 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); + int align; + set_lowering_range(lowered_bytes, offset, mono_type_size(type, &align), kind); } } @@ -6730,13 +6732,18 @@ mono_marshal_get_swift_physical_lowering (MonoType *type, gboolean native_layout } MonoClass *klass = mono_class_from_mono_type_internal (type); - + // SwiftSelf and SwiftError are special cases where we need to preserve the class information for the codegen to handle them correctly. + if (klass == mono_class_try_get_swift_self_class () || klass == mono_class_try_get_swift_error_class ()) { + lowering.by_reference = TRUE; + return lowering; + } + 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; } @@ -6755,8 +6762,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; @@ -6784,19 +6790,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; @@ -6814,13 +6824,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: { @@ -6853,20 +6863,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; } @@ -6877,7 +6887,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 8d545fcc25dea6..92452082b0335f 100644 --- a/src/mono/mono/metadata/marshal.h +++ b/src/mono/mono/metadata/marshal.h @@ -744,8 +744,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; From b305cb380a7d41a74c2b4c82bfcbc87a91d7473f Mon Sep 17 00:00:00 2001 From: Matous Kozak Date: Fri, 10 May 2024 10:39:34 +0200 Subject: [PATCH 02/13] mini struct lowering --- src/mono/mono/mini/method-to-ir.c | 64 +++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/src/mono/mono/mini/method-to-ir.c b/src/mono/mono/mini/method-to-ir.c index 40905c0ceb48f5..e53e100bdc2b84 100644 --- a/src/mono/mono/mini/method-to-ir.c +++ b/src/mono/mono/mini/method-to-ir.c @@ -7514,6 +7514,70 @@ 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); + /* + * 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_args = g_newa (MonoInst*, n); + for (int idx_param = 0; idx_param < n; ++idx_param) { + old_args [idx_param] = sp [idx_param]; + } + + GArray *new_params = g_array_new (FALSE, FALSE, sizeof (MonoType *)); + uint32_t new_param_count = 0; + /* + * 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]; + if (mono_type_is_struct (ptype)) { + 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); //ins_flag + *sp++ = lowered_arg; + + ++new_param_count; + g_array_append_val (new_params, lowered_swift_struct.lowered_elements [idx_lowered]); + } + continue; + } + } + // For non-struct and by-ref arguments, just copy them over + *sp++ = old_args [idx_param]; + + ++new_param_count; + g_array_append_val (new_params, ptype); + } + + // Create a new dummy signature with the lowered arguments + MonoMethodSignature *swiftcall_signature = mono_metadata_signature_alloc (m_class_get_image (method->klass), new_param_count); + for (uint32_t idx_param = 0; idx_param < new_param_count; ++idx_param) { + swiftcall_signature->params [idx_param] = g_array_index (new_params, MonoType *, idx_param); + } + swiftcall_signature->ret = fsig->ret; + swiftcall_signature->pinvoke = fsig->pinvoke; + swiftcall_signature->ext_callconv = fsig->ext_callconv; + + fsig = swiftcall_signature; + } + if (method->dynamic && fsig->pinvoke) { MonoInst *args [3]; From 2764076cbfdefd6402d95fee54829d80789d4715 Mon Sep 17 00:00:00 2001 From: Matous Kozak Date: Fri, 10 May 2024 10:40:12 +0200 Subject: [PATCH 03/13] interpreter swift struct lowering --- src/mono/mono/mini/interp/transform.c | 72 +++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/src/mono/mono/mini/interp/transform.c b/src/mono/mono/mini/interp/transform.c index be1d722f146408..f8e509198c3f1f 100644 --- a/src/mono/mono/mini/interp/transform.c +++ b/src/mono/mono/mini/interp/transform.c @@ -3404,6 +3404,78 @@ 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)) { + g_assert (!csignature->hasthis); // P/Invoke calls shouldn't contain 'this' + + // 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); + memcpy (sp_old_params, td->sp, sizeof (StackInfo) * csignature->param_count); + + GArray *new_params = g_array_new (FALSE, FALSE, sizeof (MonoType *)); + uint32_t new_param_count = 0; + int align; + /* + * 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]; + if (mono_type_is_struct (ptype)) { + 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] = 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]); + } + continue; + } + } + // For non-struct and by-ref arguments, just copy them over + 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 + MonoMethodSignature *swiftcall_signature = mono_metadata_signature_alloc (m_class_get_image (method->klass), new_param_count); + for (uint32_t idx_param = 0; idx_param < new_param_count; ++idx_param) { + swiftcall_signature->params [idx_param] = g_array_index (new_params, MonoType *, idx_param); + } + swiftcall_signature->ret = csignature->ret; + swiftcall_signature->pinvoke = csignature->pinvoke; + swiftcall_signature->ext_callconv = csignature->ext_callconv; + + csignature = swiftcall_signature; + } +#endif + if (check_visibility && target_method && !mono_method_can_access_method (method, target_method)) interp_generate_mae_throw (td, method, target_method); From 5294446fc148db074b0d19fb01962dc25d330e4b Mon Sep 17 00:00:00 2001 From: Matous Kozak Date: Mon, 13 May 2024 07:24:45 +0200 Subject: [PATCH 04/13] enable SwiftAbiStress on Mono --- src/tests/issues.targets | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/tests/issues.targets b/src/tests/issues.targets index 8289fcefb924fb..2dd122d2a642bd 100644 --- a/src/tests/issues.targets +++ b/src/tests/issues.targets @@ -1919,9 +1919,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 From 91a5e7ad867d7fb9d356c1549e9c24535dce69aa Mon Sep 17 00:00:00 2001 From: Matous Kozak Date: Tue, 14 May 2024 08:52:04 +0000 Subject: [PATCH 05/13] explicit handling of byref swift structs --- src/mono/mono/mini/interp/transform.c | 27 ++++++++++++++++++-------- src/mono/mono/mini/method-to-ir.c | 28 +++++++++++++++++++-------- 2 files changed, 39 insertions(+), 16 deletions(-) diff --git a/src/mono/mono/mini/interp/transform.c b/src/mono/mono/mini/interp/transform.c index f8e509198c3f1f..ed76ddfb63dc15 100644 --- a/src/mono/mono/mini/interp/transform.c +++ b/src/mono/mono/mini/interp/transform.c @@ -3421,7 +3421,7 @@ interp_transform_call (TransformData *td, MonoMethod *method, MonoMethod *target StackInfo *sp_old_params = (StackInfo*) mono_mempool_alloc (td->mempool, sizeof (StackInfo) * csignature->param_count); memcpy (sp_old_params, td->sp, sizeof (StackInfo) * csignature->param_count); - GArray *new_params = g_array_new (FALSE, FALSE, sizeof (MonoType *)); + GArray *new_params = g_array_new (FALSE, FALSE, sizeof (MonoType*)); uint32_t new_param_count = 0; int align; /* @@ -3449,15 +3449,26 @@ interp_transform_call (TransformData *td, MonoMethod *method, MonoMethod *target ++new_param_count; g_array_append_val (new_params, lowered_swift_struct.lowered_elements [idx_lowered]); } - continue; + } 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); } - } - // For non-struct and by-ref arguments, just copy them over - memcpy (td->sp, &sp_old_params [idx_param], sizeof (StackInfo)); - ++td->sp; + } 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); + ++new_param_count; + g_array_append_val (new_params, ptype); + } } // Restore the function pointer memcpy (td->sp, &sp_fp, sizeof (StackInfo)); diff --git a/src/mono/mono/mini/method-to-ir.c b/src/mono/mono/mini/method-to-ir.c index e53e100bdc2b84..2109cbd391680a 100644 --- a/src/mono/mono/mini/method-to-ir.c +++ b/src/mono/mono/mini/method-to-ir.c @@ -7529,7 +7529,7 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b old_args [idx_param] = sp [idx_param]; } - GArray *new_params = g_array_new (FALSE, FALSE, sizeof (MonoType *)); + GArray *new_params = g_array_new (FALSE, FALSE, sizeof (MonoType*)); uint32_t new_param_count = 0; /* * Go through the lowered arguments, if the argument is a struct, @@ -7550,20 +7550,32 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b 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); //ins_flag + 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]); } - continue; + } 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); } - } - // For non-struct and by-ref arguments, just copy them over - *sp++ = old_args [idx_param]; + } else { + // Copy over non-struct arguments + *sp++ = old_args [idx_param]; - ++new_param_count; - g_array_append_val (new_params, ptype); + ++new_param_count; + g_array_append_val (new_params, ptype); + } } // Create a new dummy signature with the lowered arguments From 49d5afb53617d7b63338280b735d6dcc07a5184e Mon Sep 17 00:00:00 2001 From: Matous Kozak Date: Tue, 14 May 2024 14:21:21 +0200 Subject: [PATCH 06/13] move SwiftSelf and SwiftError checks outside Swift struct lowering algorithm --- src/mono/mono/metadata/marshal.c | 5 ----- src/mono/mono/mini/interp/transform.c | 6 +++++- src/mono/mono/mini/method-to-ir.c | 9 ++++++++- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/mono/mono/metadata/marshal.c b/src/mono/mono/metadata/marshal.c index d045bf2211aa4b..c3caa56c3cf174 100644 --- a/src/mono/mono/metadata/marshal.c +++ b/src/mono/mono/metadata/marshal.c @@ -6732,11 +6732,6 @@ mono_marshal_get_swift_physical_lowering (MonoType *type, gboolean native_layout } MonoClass *klass = mono_class_from_mono_type_internal (type); - // SwiftSelf and SwiftError are special cases where we need to preserve the class information for the codegen to handle them correctly. - if (klass == mono_class_try_get_swift_self_class () || klass == mono_class_try_get_swift_error_class ()) { - lowering.by_reference = TRUE; - return lowering; - } 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. diff --git a/src/mono/mono/mini/interp/transform.c b/src/mono/mono/mini/interp/transform.c index ed76ddfb63dc15..96725147273886 100644 --- a/src/mono/mono/mini/interp/transform.c +++ b/src/mono/mono/mini/interp/transform.c @@ -3424,6 +3424,8 @@ interp_transform_call (TransformData *td, MonoMethod *method, MonoMethod *target GArray *new_params = g_array_new (FALSE, FALSE, sizeof (MonoType*)); 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. @@ -3431,7 +3433,9 @@ interp_transform_call (TransformData *td, MonoMethod *method, MonoMethod *target */ for (int idx_param = 0; idx_param < csignature->param_count; ++idx_param) { MonoType *ptype = csignature->params [idx_param]; - if (mono_type_is_struct (ptype)) { + 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) { diff --git a/src/mono/mono/mini/method-to-ir.c b/src/mono/mono/mini/method-to-ir.c index 2109cbd391680a..79ff08871a9a97 100644 --- a/src/mono/mono/mini/method-to-ir.c +++ b/src/mono/mono/mini/method-to-ir.c @@ -7514,6 +7514,7 @@ 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. @@ -7531,6 +7532,8 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b GArray *new_params = g_array_new (FALSE, FALSE, sizeof (MonoType*)); 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. @@ -7538,7 +7541,10 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b */ for (int idx_param = 0; idx_param < n; ++idx_param) { MonoType *ptype = fsig->params [idx_param]; - if (mono_type_is_struct (ptype)) { + 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 @@ -7589,6 +7595,7 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b fsig = swiftcall_signature; } +#endif if (method->dynamic && fsig->pinvoke) { MonoInst *args [3]; From b564238ceb794f4c49fa4bd9209a29c83605e7c9 Mon Sep 17 00:00:00 2001 From: Matous Kozak Date: Tue, 14 May 2024 19:47:28 +0200 Subject: [PATCH 07/13] Adding a helper function for new signature The helper function duplicates signature metadata and adds new parameters to the signature. + pre-allocating new_params to the old params size --- src/mono/mono/metadata/metadata-internals.h | 1 + src/mono/mono/metadata/metadata.c | 31 +++++++++++++++++++++ src/mono/mono/mini/interp/transform.c | 12 ++------ src/mono/mono/mini/method-to-ir.c | 14 ++-------- 4 files changed, 37 insertions(+), 21 deletions(-) diff --git a/src/mono/mono/metadata/metadata-internals.h b/src/mono/mono/metadata/metadata-internals.h index 8c1dfd803b578d..a2c84bc0708c46 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 c517703415eab9..ef7dc36da307d1 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, new_sig_size); + memcpy (res, sig, MONO_SIZEOF_METHOD_SIGNATURE); + res->param_count = GUINT32_TO_UINT16 (num_params); + + for (int i = 0; i < num_params; 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 96725147273886..91253e79054934 100644 --- a/src/mono/mono/mini/interp/transform.c +++ b/src/mono/mono/mini/interp/transform.c @@ -3421,7 +3421,7 @@ interp_transform_call (TransformData *td, MonoMethod *method, MonoMethod *target StackInfo *sp_old_params = (StackInfo*) mono_mempool_alloc (td->mempool, sizeof (StackInfo) * csignature->param_count); memcpy (sp_old_params, td->sp, sizeof (StackInfo) * csignature->param_count); - GArray *new_params = g_array_new (FALSE, FALSE, sizeof (MonoType*)); + 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 (); @@ -3479,15 +3479,7 @@ interp_transform_call (TransformData *td, MonoMethod *method, MonoMethod *target ++td->sp; // Create a new dummy signature with the lowered arguments - MonoMethodSignature *swiftcall_signature = mono_metadata_signature_alloc (m_class_get_image (method->klass), new_param_count); - for (uint32_t idx_param = 0; idx_param < new_param_count; ++idx_param) { - swiftcall_signature->params [idx_param] = g_array_index (new_params, MonoType *, idx_param); - } - swiftcall_signature->ret = csignature->ret; - swiftcall_signature->pinvoke = csignature->pinvoke; - swiftcall_signature->ext_callconv = csignature->ext_callconv; - - csignature = swiftcall_signature; + csignature = mono_metadata_signature_dup_new_params (td->mempool, csignature, new_param_count, (MonoType**)new_params->data); } #endif diff --git a/src/mono/mono/mini/method-to-ir.c b/src/mono/mono/mini/method-to-ir.c index 79ff08871a9a97..989bada468a419 100644 --- a/src/mono/mono/mini/method-to-ir.c +++ b/src/mono/mono/mini/method-to-ir.c @@ -7530,7 +7530,7 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b old_args [idx_param] = sp [idx_param]; } - GArray *new_params = g_array_new (FALSE, FALSE, sizeof (MonoType*)); + 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 (); @@ -7584,16 +7584,8 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b } } - // Create a new dummy signature with the lowered arguments - MonoMethodSignature *swiftcall_signature = mono_metadata_signature_alloc (m_class_get_image (method->klass), new_param_count); - for (uint32_t idx_param = 0; idx_param < new_param_count; ++idx_param) { - swiftcall_signature->params [idx_param] = g_array_index (new_params, MonoType *, idx_param); - } - swiftcall_signature->ret = fsig->ret; - swiftcall_signature->pinvoke = fsig->pinvoke; - swiftcall_signature->ext_callconv = fsig->ext_callconv; - - fsig = swiftcall_signature; + // 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); } #endif From 96a6fd39f1408269f9ac7215229b74ca6365005e Mon Sep 17 00:00:00 2001 From: Matous Kozak Date: Tue, 14 May 2024 20:44:09 +0200 Subject: [PATCH 08/13] extract swift struct lowering to separate function Only for interpreter at the moment because mono_method_to_ir is more complicated due to NEW_ARGLOADA macros --- src/mono/mono/mini/interp/transform.c | 146 ++++++++++++++------------ 1 file changed, 76 insertions(+), 70 deletions(-) diff --git a/src/mono/mono/mini/interp/transform.c b/src/mono/mono/mini/interp/transform.c index 91253e79054934..9107e2a4ac1115 100644 --- a/src/mono/mono/mini/interp/transform.c +++ b/src/mono/mono/mini/interp/transform.c @@ -3320,6 +3320,81 @@ interp_try_devirt (MonoClass *this_klass, MonoMethod *target_method) return NULL; } +static MonoMethodSignature* +interp_emit_swiftcall_struct_lowering (TransformData *td, MonoMethodSignature *csignature) +{ + g_assert (!csignature->hasthis); // P/Invoke calls shouldn't contain 'this' + + // 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); + memcpy (sp_old_params, td->sp, sizeof (StackInfo) * csignature->param_count); + + 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] = 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 + return mono_metadata_signature_dup_new_params (td->mempool, csignature, new_param_count, (MonoType**)new_params->data); +} + /* 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) @@ -3410,76 +3485,7 @@ interp_transform_call (TransformData *td, MonoMethod *method, MonoMethod *target * 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)) { - g_assert (!csignature->hasthis); // P/Invoke calls shouldn't contain 'this' - - // 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); - memcpy (sp_old_params, td->sp, sizeof (StackInfo) * csignature->param_count); - - 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] = 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 - csignature = mono_metadata_signature_dup_new_params (td->mempool, csignature, new_param_count, (MonoType**)new_params->data); + csignature = interp_emit_swiftcall_struct_lowering (td, csignature); } #endif From ba829c881bce7cf69e3d686b1bea95ccbc0fe7d6 Mon Sep 17 00:00:00 2001 From: Matous Kozak Date: Wed, 15 May 2024 15:02:02 +0200 Subject: [PATCH 09/13] fix build warnings in signature alloc helper func --- src/mono/mono/metadata/metadata.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/mono/mono/metadata/metadata.c b/src/mono/mono/metadata/metadata.c index ef7dc36da307d1..333c7603fd8c1b 100644 --- a/src/mono/mono/metadata/metadata.c +++ b/src/mono/mono/metadata/metadata.c @@ -2565,11 +2565,11 @@ mono_metadata_signature_dup_new_params (MonoMemPool *mp, MonoMethodSignature *si if (sig->ret) new_sig_size += mono_sizeof_type (sig->ret); - MonoMethodSignature *res = (MonoMethodSignature *)mono_mempool_alloc0 (mp, new_sig_size); + 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 (int i = 0; i < num_params; i++) { + for (uint16_t i = 0; i < res->param_count; i++) { res->params [i] = new_params [i]; } res->ret = sig->ret; From 241765b5c8302a14e771f32dd6128c2f89442aba Mon Sep 17 00:00:00 2001 From: Matous Kozak Date: Wed, 15 May 2024 18:24:53 +0200 Subject: [PATCH 10/13] deallocate temporary arrays --- src/mono/mono/mini/interp/transform.c | 11 +++++++++-- src/mono/mono/mini/method-to-ir.c | 11 +++++++---- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/mono/mono/mini/interp/transform.c b/src/mono/mono/mini/interp/transform.c index 9107e2a4ac1115..ecd98554e5ea21 100644 --- a/src/mono/mono/mini/interp/transform.c +++ b/src/mono/mono/mini/interp/transform.c @@ -3323,6 +3323,7 @@ interp_try_devirt (MonoClass *this_klass, MonoMethod *target_method) static MonoMethodSignature* interp_emit_swiftcall_struct_lowering (TransformData *td, MonoMethodSignature *csignature) { + MonoMethodSignature *new_csignature; g_assert (!csignature->hasthis); // P/Invoke calls shouldn't contain 'this' // Save the function pointer @@ -3332,7 +3333,8 @@ interp_emit_swiftcall_struct_lowering (TransformData *td, MonoMethodSignature *c // Save the old arguments td->sp -= csignature->param_count; StackInfo *sp_old_params = (StackInfo*) mono_mempool_alloc (td->mempool, sizeof (StackInfo) * csignature->param_count); - memcpy (sp_old_params, td->sp, 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; @@ -3392,7 +3394,12 @@ interp_emit_swiftcall_struct_lowering (TransformData *td, MonoMethodSignature *c ++td->sp; // Create a new dummy signature with the lowered arguments - return mono_metadata_signature_dup_new_params (td->mempool, csignature, new_param_count, (MonoType**)new_params->data); + 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 */ diff --git a/src/mono/mono/mini/method-to-ir.c b/src/mono/mono/mini/method-to-ir.c index 989bada468a419..90a8f3f341e8f8 100644 --- a/src/mono/mono/mini/method-to-ir.c +++ b/src/mono/mono/mini/method-to-ir.c @@ -7525,9 +7525,9 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b n = fsig->param_count; sp -= n; // Save the old arguments - MonoInst **old_args = g_newa (MonoInst*, n); + MonoInst **old_params = (MonoInst**) mono_mempool_alloc (cfg->mempool, sizeof (MonoInst*) * n); for (int idx_param = 0; idx_param < n; ++idx_param) { - old_args [idx_param] = sp [idx_param]; + old_params [idx_param] = sp [idx_param]; } GArray *new_params = g_array_sized_new (FALSE, FALSE, sizeof (MonoType*), n); @@ -7577,7 +7577,7 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b } } else { // Copy over non-struct arguments - *sp++ = old_args [idx_param]; + *sp++ = old_params [idx_param]; ++new_param_count; g_array_append_val (new_params, ptype); @@ -7585,7 +7585,10 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b } // 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); + 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 From 0638729412fc8ff8b3fd7c17f1cae87057e55e7e Mon Sep 17 00:00:00 2001 From: Matous Kozak Date: Wed, 15 May 2024 18:54:00 +0200 Subject: [PATCH 11/13] fix cast to guint16 in transform.c --- src/mono/mono/mini/interp/transform.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mono/mono/mini/interp/transform.c b/src/mono/mono/mini/interp/transform.c index ecd98554e5ea21..2011e4a1189189 100644 --- a/src/mono/mono/mini/interp/transform.c +++ b/src/mono/mono/mini/interp/transform.c @@ -3359,7 +3359,7 @@ interp_emit_swiftcall_struct_lowering (TransformData *td, MonoMethodSignature *c // 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] = lowered_swift_struct.offsets [idx_lowered]; + 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])); From 2376d0562cdf13e8b231896834b74bb4ebbac2ac Mon Sep 17 00:00:00 2001 From: Matous Kozak Date: Fri, 17 May 2024 10:32:13 +0200 Subject: [PATCH 12/13] fix style in marshaling of swift structs --- src/mono/mono/metadata/marshal.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/mono/mono/metadata/marshal.c b/src/mono/mono/metadata/marshal.c index c3caa56c3cf174..27bf4bd180d522 100644 --- a/src/mono/mono/metadata/marshal.c +++ b/src/mono/mono/metadata/marshal.c @@ -6603,7 +6603,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: @@ -6615,7 +6616,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))) { @@ -6646,7 +6648,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; @@ -6676,7 +6679,10 @@ static void record_struct_physical_lowering (guint8* lowered_bytes, MonoClass* k } } -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) { @@ -6704,7 +6710,6 @@ static void record_struct_field_physical_lowering (guint8* lowered_bytes, MonoTy kind = SWIFT_DOUBLE; } - int align; set_lowering_range(lowered_bytes, offset, mono_type_size(type, &align), kind); } } From f0e8da44c56c463ea64157ed7a1ea1313895be32 Mon Sep 17 00:00:00 2001 From: Matous Kozak Date: Fri, 17 May 2024 10:55:27 +0200 Subject: [PATCH 13/13] assert td->optimized for interp struct lowering --- src/mono/mono/mini/interp/transform.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/mono/mono/mini/interp/transform.c b/src/mono/mono/mini/interp/transform.c index 2011e4a1189189..7f3e4e659da11c 100644 --- a/src/mono/mono/mini/interp/transform.c +++ b/src/mono/mono/mini/interp/transform.c @@ -3323,9 +3323,16 @@ interp_try_devirt (MonoClass *this_klass, MonoMethod *target_method) static MonoMethodSignature* interp_emit_swiftcall_struct_lowering (TransformData *td, MonoMethodSignature *csignature) { - MonoMethodSignature *new_csignature; - g_assert (!csignature->hasthis); // P/Invoke calls shouldn't contain 'this' + // 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;