Skip to content

[Mono] [swift-interop] Add support for reverse pinvoke argument lowering #104437

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 19 commits into from
Jul 15, 2024
Merged

[Mono] [swift-interop] Add support for reverse pinvoke argument lowering #104437

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
f4a08ad
Add support for reverse pinvoke argument lowering
jkurdek Jul 4, 2024
f113643
Fix ILGeneration tests errors
jkurdek Jul 5, 2024
ad59f6d
Fix build errors
jkurdek Jul 5, 2024
d703585
Reworked structure reassemble algorithm to work on sig
jkurdek Jul 5, 2024
4af9745
Fix x64 test errors
jkurdek Jul 5, 2024
bbf5612
Merge branch 'dotnet:main' into reverse-pinvokes-args-lowering
jkurdek Jul 8, 2024
b40a633
Add support for SwiftIndirectResult
jkurdek Jul 8, 2024
6ebc9bd
Remove duplicate call to lowering algorithm
jkurdek Jul 8, 2024
12ff9d8
Refactor emitting swift lowered struct
jkurdek Jul 8, 2024
3912225
Fix formatting
jkurdek Jul 8, 2024
e6401cb
Remove extra newline
jkurdek Jul 8, 2024
f95d478
Fix SwiftIndirectResult errors
jkurdek Jul 8, 2024
9e1f6e2
Address review comments
jkurdek Jul 9, 2024
b7e8f35
Fix loop indent
jkurdek Jul 10, 2024
a176c16
Update src/mono/mono/metadata/marshal-lightweight.c
jkurdek Jul 11, 2024
cb449f9
Remove passing MonoImage
jkurdek Jul 11, 2024
022eab3
Fix formatting around conditional blocks
jkurdek Jul 12, 2024
96a2cb9
Fix missing whitespace
jkurdek Jul 12, 2024
903d7b6
Merge branch 'main' into reverse-pinvokes-args-lowering
jkurdek Jul 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 57 additions & 8 deletions src/mono/mono/metadata/marshal-lightweight.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2715,13 +2715,47 @@ emit_managed_wrapper_validate_signature (MonoMethodSignature* sig, MonoMarshalSp
return TRUE;
}

static void
emit_swift_lowered_struct_load (MonoMethodBuilder *mb, MonoMethodSignature *csig, SwiftPhysicalLowering swift_lowering, int tmp_local, uint32_t csig_argnum)
{
guint8 stind_op;
uint32_t offset = 0;

for (uint32_t idx_lowered = 0; idx_lowered < swift_lowering.num_lowered_elements; idx_lowered++) {
offset = swift_lowering.offsets [idx_lowered];
mono_mb_emit_ldloc_addr (mb, tmp_local);
mono_mb_emit_icon (mb, offset);
mono_mb_emit_byte (mb, CEE_ADD);

mono_mb_emit_ldarg (mb, csig_argnum + idx_lowered);
stind_op = mono_type_to_stind (csig->params [csig_argnum + idx_lowered]);
mono_mb_emit_byte (mb, stind_op);
}
}

/* Swift struct lowering handling causes csig to have additional arguments.
* This function returns the index of the argument in the csig that corresponds to the argument in the original signature.
*/
static int
get_csig_argnum (int i, EmitMarshalContext* m)
{
if (m->swift_sig_to_csig_mp) {
g_assert (i < m->sig->param_count);
int csig_argnum = m->swift_sig_to_csig_mp [i];
g_assert (csig_argnum >= 0 && csig_argnum < m->csig->param_count);
return csig_argnum;
}
return i;
}

static void
emit_managed_wrapper_ilgen (MonoMethodBuilder *mb, MonoMethodSignature *invoke_sig, MonoMarshalSpec **mspecs, EmitMarshalContext* m, MonoMethod *method, MonoGCHandle target_handle, gboolean runtime_init_callback, MonoError *error)
{
MonoMethodSignature *sig, *csig;
int i, *tmp_locals;
gboolean closed = FALSE;
GCUnsafeTransitionBuilder gc_unsafe_builder = {0,};
SwiftPhysicalLowering *swift_lowering = m->swift_lowering;

sig = m->sig;
csig = m->csig;
Expand Down Expand Up @@ -2798,19 +2832,31 @@ emit_managed_wrapper_ilgen (MonoMethodBuilder *mb, MonoMethodSignature *invoke_s
for (i = 0; i < sig->param_count; i ++) {
MonoType *t = sig->params [i];
MonoMarshalSpec *spec = mspecs [i + 1];
int csig_argnum = get_csig_argnum (i, m);

if (spec && spec->native == MONO_NATIVE_CUSTOM) {
tmp_locals [i] = mono_emit_marshal (m, i, t, mspecs [i + 1], 0, &csig->params [i], MARSHAL_ACTION_MANAGED_CONV_IN);
tmp_locals [i] = mono_emit_marshal (m, csig_argnum, t, mspecs [i + 1], 0, &csig->params [csig_argnum], MARSHAL_ACTION_MANAGED_CONV_IN);
} else {
switch (t->type) {
case MONO_TYPE_VALUETYPE:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you need a MONO_TYPE_GENERICINST case to handle SwiftSelf<T> or other generics?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe currently we do not plan to support SwiftSelf<T> in reverse pinvokes #103576 (comment)

cc: @kotlarmilos

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#103576 (comment)

if (mono_method_signature_has_ext_callconv (csig, MONO_EXT_CALLCONV_SWIFTCALL)) {
if (swift_lowering [i].num_lowered_elements > 0) {
tmp_locals [i] = mono_mb_add_local (mb, sig->params [i]);
emit_swift_lowered_struct_load (mb, csig, swift_lowering [i], tmp_locals [i], csig_argnum);
break;
} else if (swift_lowering [i].by_reference) {
/* Structs passed by reference are handled during arg loading emission */
tmp_locals [i] = 0;
break;
}
} /* else fallthru */
case MONO_TYPE_OBJECT:
case MONO_TYPE_CLASS:
case MONO_TYPE_VALUETYPE:
case MONO_TYPE_ARRAY:
case MONO_TYPE_SZARRAY:
case MONO_TYPE_STRING:
case MONO_TYPE_BOOLEAN:
tmp_locals [i] = mono_emit_marshal (m, i, t, mspecs [i + 1], 0, &csig->params [i], MARSHAL_ACTION_MANAGED_CONV_IN);
tmp_locals [i] = mono_emit_marshal (m, csig_argnum, t, mspecs [i + 1], 0, &csig->params [csig_argnum], MARSHAL_ACTION_MANAGED_CONV_IN);
break;
default:
tmp_locals [i] = 0;
Expand All @@ -2836,15 +2882,18 @@ emit_managed_wrapper_ilgen (MonoMethodBuilder *mb, MonoMethodSignature *invoke_s

for (i = 0; i < sig->param_count; i++) {
MonoType *t = sig->params [i];

if (tmp_locals [i]) {
int csig_argnum = get_csig_argnum (i, m);
if (mono_method_signature_has_ext_callconv (csig, MONO_EXT_CALLCONV_SWIFTCALL) && swift_lowering [i].by_reference) {
mono_mb_emit_ldarg (mb, csig_argnum);
MonoClass* klass = mono_class_from_mono_type_internal (sig->params [i]);
mono_mb_emit_op (mb, CEE_LDOBJ, klass);
} else if (tmp_locals [i]) {
if (m_type_is_byref (t))
mono_mb_emit_ldloc_addr (mb, tmp_locals [i]);
else
mono_mb_emit_ldloc (mb, tmp_locals [i]);
}
else
mono_mb_emit_ldarg (mb, i);
} else
mono_mb_emit_ldarg (mb, csig_argnum);
}

/* ret = method (...) */
Expand Down
57 changes: 50 additions & 7 deletions src/mono/mono/metadata/marshal.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4103,6 +4103,8 @@ marshal_get_managed_wrapper (MonoMethod *method, MonoClass *delegate_klass, Mono
int i;
EmitMarshalContext m;
gboolean marshalling_enabled = FALSE;
int *swift_sig_to_csig_mp = NULL;
SwiftPhysicalLowering *swift_lowering = NULL;

g_assert (method != NULL);
error_init (error);
Expand Down Expand Up @@ -4183,6 +4185,50 @@ marshal_get_managed_wrapper (MonoMethod *method, MonoClass *delegate_klass, Mono
csig->hasthis = 0;
csig->pinvoke = 1;

if (invoke)
mono_marshal_set_callconv_from_modopt (invoke, csig, TRUE);
else
mono_marshal_set_callconv_from_unmanaged_callers_only_attribute (method, csig);

if (mono_method_signature_has_ext_callconv (csig, MONO_EXT_CALLCONV_SWIFTCALL)) {
MonoClass *swift_self = mono_class_try_get_swift_self_class ();
MonoClass *swift_error = mono_class_try_get_swift_error_class ();
MonoClass *swift_indirect_result = mono_class_try_get_swift_indirect_result_class ();
swift_lowering = g_newa (SwiftPhysicalLowering, sig->param_count);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where are these deallocated?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. Added deallocation for new_params. swift_lowering and swift_sig_to_csig_mp are allocated on stack, so we don't deallocate explicetely them right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Why they are allocated differently?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

swift_lowering and swift_sig_to_csig_mp are going to have exactly sig->param_count elements. In some places across marshalling code such arrays are allocated on stack.

runtime/src/mono/mono/metadata/marshal-lightweight.c

Line 2797 in f9eda07

tmp_locals = g_newa (int, sig->param_count);

swift_sig_to_csig_mp is an array of ints so I guess it ok. We could make the swift_lowering heap allocated, but I don't know whether this is necessary.

The new_params can be up to 4 times longer than theswift_lowering and swift_sig_to_csig_mp, as it includes the lowered params. So I following what was done in method-to-ir.

runtime/src/mono/mono/mini/method-to-ir.c

Line 7533 in f9eda07

GArray *new_params = g_array_sized_new (FALSE, FALSE, sizeof (MonoType*), n);

swift_sig_to_csig_mp = g_newa (int, sig->param_count);
GArray *new_params = g_array_sized_new (FALSE, FALSE, sizeof (MonoType*), csig->param_count);
int new_param_count = 0;


for (i = 0; i < csig->param_count; i++) {
swift_lowering [i] = (SwiftPhysicalLowering){0};
swift_sig_to_csig_mp [i] = new_param_count;
MonoType *ptype = csig->params [i];
MonoClass *klass = mono_class_from_mono_type_internal (ptype);

if (mono_type_is_struct (ptype) && !(klass == swift_self || klass == swift_error || klass == swift_indirect_result)) {
SwiftPhysicalLowering lowered_swift_struct = mono_marshal_get_swift_physical_lowering (ptype, FALSE);
swift_lowering [i] = lowered_swift_struct;
if (!lowered_swift_struct.by_reference) {
for (uint32_t idx_lowered = 0; idx_lowered < lowered_swift_struct.num_lowered_elements; idx_lowered++) {
g_array_append_val (new_params, lowered_swift_struct.lowered_elements [idx_lowered]);
new_param_count++;
}
} else {
ptype = mono_class_get_byref_type (klass);
g_array_append_val (new_params, ptype);
new_param_count++;
}
} else {
g_array_append_val (new_params, ptype);
new_param_count++;
}
}

csig = mono_metadata_signature_dup_new_params (NULL, m_method_get_mem_manager (method), csig, new_param_count, (MonoType**)new_params->data);
g_array_free (new_params, TRUE);
}

if (!marshalling_enabled)
csig->marshalling_disabled = 1;

Expand All @@ -4194,11 +4240,8 @@ marshal_get_managed_wrapper (MonoMethod *method, MonoClass *delegate_klass, Mono
m.csig = csig;
m.image = get_method_image (method);
m.runtime_marshalling_enabled = marshalling_enabled;

if (invoke)
mono_marshal_set_callconv_from_modopt (invoke, csig, TRUE);
else
mono_marshal_set_callconv_from_unmanaged_callers_only_attribute(method, csig);
m.swift_lowering = swift_lowering;
m.swift_sig_to_csig_mp = swift_sig_to_csig_mp;

/* The attribute is only available in Net 2.0 */
if (delegate_klass && mono_class_try_get_unmanaged_function_pointer_attribute_class ()) {
Expand Down Expand Up @@ -4274,10 +4317,10 @@ marshal_get_managed_wrapper (MonoMethod *method, MonoClass *delegate_klass, Mono
info->d.native_to_managed.klass = delegate_klass;

res = mono_mb_create_and_cache_full (cache, method,
mb, csig, sig->param_count + 16,
mb, csig, csig->param_count + 16,
info, NULL);
} else {
res = mono_mb_create (mb, csig, sig->param_count + 16, NULL);
res = mono_mb_create (mb, csig, csig->param_count + 16, NULL);
}
}

Expand Down
22 changes: 12 additions & 10 deletions src/mono/mono/metadata/marshal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,16 @@ typedef const gunichar2 *mono_bstr_const;

GENERATE_TRY_GET_CLASS_WITH_CACHE_DECL(stringbuilder)

typedef struct {
gboolean by_reference;
uint32_t num_lowered_elements;
MonoType *lowered_elements[4];
uint32_t offsets[4];
} SwiftPhysicalLowering;

SwiftPhysicalLowering
mono_marshal_get_swift_physical_lowering (MonoType *type, gboolean native_layout);


/*
* This structure holds the state kept by the emit_ marshalling functions.
Expand All @@ -53,6 +63,8 @@ typedef struct {
MonoMethodSignature *csig; /* Might need to be changed due to MarshalAs directives */
MonoImage *image; /* The image to use for looking up custom marshallers */
gboolean runtime_marshalling_enabled;
SwiftPhysicalLowering *swift_lowering;
int *swift_sig_to_csig_mp;
} EmitMarshalContext;

typedef enum {
Expand Down Expand Up @@ -744,14 +756,4 @@ GENERATE_TRY_GET_CLASS_WITH_CACHE_DECL (swift_self)
GENERATE_TRY_GET_CLASS_WITH_CACHE_DECL (swift_error)
GENERATE_TRY_GET_CLASS_WITH_CACHE_DECL (swift_indirect_result)

typedef struct {
gboolean by_reference;
uint32_t num_lowered_elements;
MonoType *lowered_elements[4];
uint32_t offsets[4];
} SwiftPhysicalLowering;

SwiftPhysicalLowering
mono_marshal_get_swift_physical_lowering (MonoType *type, gboolean native_layout);

#endif /* __MONO_MARSHAL_H__ */
Loading
Loading