Skip to content
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

Change callvirt into calli for virtual delegates #83461

Merged
merged 27 commits into from
Apr 26, 2023
Merged

Change callvirt into calli for virtual delegates #83461

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
a175351
Changed how NaN values are compared due to their multiple bit represe…
Nov 24, 2022
f9ed58c
Merge branch 'dotnet:main' into main
LeVladIonescu Nov 24, 2022
a381c06
Fixed typo
Nov 24, 2022
d91a78f
Merge branch 'dotnet:main' into main
LeVladIonescu Nov 25, 2022
1291fe3
Merge branch 'dotnet:main' into main
LeVladIonescu Dec 5, 2022
9a58bb3
Merge branch 'dotnet:main' into main
LeVladIonescu Dec 16, 2022
8db950e
Merge branch 'dotnet:main' into main
LeVladIonescu Jan 2, 2023
f587d24
Merge branch 'dotnet:main' into main
LeVladIonescu Jan 5, 2023
f5dbeaf
Merge branch 'dotnet:main' into main
LeVladIonescu Feb 1, 2023
287419e
Merge branch 'dotnet:main' into main
LeVladIonescu Feb 3, 2023
fbc1526
Work to make JIT virtual delegates not depend on the target_method
Mar 15, 2023
afd58dd
Merge branch 'dotnet:main' into delegates_issue
LeVladIonescu Mar 15, 2023
45f4845
Merge branch 'dotnet:main' into delegates_issue
LeVladIonescu Mar 15, 2023
91c3920
Switched order of how MonoObject is passed to the icall
Mar 16, 2023
81973db
Initialise pointer for build warning
Mar 16, 2023
3b0d6cb
Check return value in the test
Mar 16, 2023
f67f5fb
Retrieved method in icall and changed test location
Mar 17, 2023
dc2c525
Added check for unbox & rgctx trampolines and modified caching
Mar 31, 2023
4ba3366
Deleted unused vars
Mar 31, 2023
7b65607
Added test for lazy fetch trampoline (rgctx)
Apr 3, 2023
e7e8913
Merge branch 'dotnet:main' into delegates_issue
LeVladIonescu Apr 3, 2023
d6e1d3d
Added boxing for valuetype ref
Apr 12, 2023
0074bea
Added RequiresProcessIsolation property for tests in merged directory
Apr 13, 2023
be6daed
Using get_ftnptr callback and moving boxing inside the icall
Apr 19, 2023
596837e
Merge branch 'dotnet:main' into delegates_issue
LeVladIonescu Apr 20, 2023
414f451
Merge branch 'dotnet:main' into delegates_issue
LeVladIonescu Apr 24, 2023
1d2c4fb
Rremoved add_delegate_trampoline since it's not used anymore
Apr 24, 2023
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
1 change: 1 addition & 0 deletions src/mono/mono/metadata/jit-icall-reg.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,7 @@ MONO_JIT_ICALL (mono_gc_wbarrier_generic_nostore_internal) \
MONO_JIT_ICALL (mono_gc_wbarrier_range_copy) \
MONO_JIT_ICALL (mono_gchandle_get_target_internal) \
MONO_JIT_ICALL (mono_generic_class_init) \
MONO_JIT_ICALL (mono_get_addr_compiled_method) \
MONO_JIT_ICALL (mono_get_assembly_object) \
MONO_JIT_ICALL (mono_get_method_object) \
MONO_JIT_ICALL (mono_get_native_calli_wrapper) \
Expand Down
13 changes: 8 additions & 5 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 @@ -1990,7 +1990,7 @@ emit_delegate_end_invoke_ilgen (MonoMethodBuilder *mb, MonoMethodSignature *sig)
}

static void
emit_delegate_invoke_internal_ilgen (MonoMethodBuilder *mb, MonoMethodSignature *sig, MonoMethodSignature *invoke_sig, gboolean static_method_with_first_arg_bound, gboolean callvirt, gboolean closed_over_null, MonoMethod *method, MonoMethod *target_method, MonoClass *target_class, MonoGenericContext *ctx, MonoGenericContainer *container)
emit_delegate_invoke_internal_ilgen (MonoMethodBuilder *mb, MonoMethodSignature *sig, MonoMethodSignature *invoke_sig, MonoMethodSignature *target_method_sig, gboolean static_method_with_first_arg_bound, gboolean callvirt, gboolean closed_over_null, MonoMethod *method, MonoMethod *target_method, MonoClass *target_class, MonoGenericContext *ctx, MonoGenericContainer *container)
{
int local_i, local_len, local_delegates, local_d, local_target, local_res = 0;
int pos0, pos1, pos2;
Expand Down Expand Up @@ -2095,11 +2095,14 @@ emit_delegate_invoke_internal_ilgen (MonoMethodBuilder *mb, MonoMethodSignature
mono_mb_emit_ldarg (mb, i + 1);
Copy link
Contributor

Choose a reason for hiding this comment

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

This optimized version depends on target_method as well, so it might need to be removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are suggesting to retrieve the method the way it was done before ? i.e. in the il based on its field address inside MonoDelegate class and not in the call?

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean that optimized version needs to be removed since it will make the wrapper depend on target_method.

mono_mb_emit_op (mb, CEE_CALL, target_method);
} else {
for (i = 1; i <= sig->param_count; ++i)
mono_mb_emit_ldarg (mb, i);
mono_mb_emit_ldarg (mb, 1);
mono_mb_emit_op (mb, CEE_CASTCLASS, target_class);
for (i = 1; i < sig->param_count; ++i)
mono_mb_emit_ldarg (mb, i + 1);
mono_mb_emit_op (mb, CEE_CALLVIRT, target_method);
mono_mb_emit_ldarg (mb, 0);
mono_mb_emit_ldflda (mb, MONO_STRUCT_OFFSET (MonoDelegate, method));
mono_mb_emit_byte (mb, CEE_LDIND_I);
mono_mb_emit_icall (mb, mono_get_addr_compiled_method);
lambdageek marked this conversation as resolved.
Show resolved Hide resolved
mono_mb_emit_op (mb, CEE_CALLI, target_method_sig);
}
} else {
mono_mb_emit_byte (mb, CEE_LDNULL);
Expand Down
35 changes: 33 additions & 2 deletions src/mono/mono/metadata/marshal.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,7 @@ mono_marshal_init (void)
register_icall (monoeg_g_free, mono_icall_sig_void_ptr, FALSE);
register_icall (mono_object_isinst_icall, mono_icall_sig_object_object_ptr, TRUE);
register_icall (mono_struct_delete_old, mono_icall_sig_void_ptr_ptr, FALSE);
register_icall (mono_get_addr_compiled_method, mono_icall_sig_ptr_object_ptr, FALSE);
register_icall (mono_delegate_begin_invoke, mono_icall_sig_object_object_ptr, FALSE);
register_icall (mono_delegate_end_invoke, mono_icall_sig_object_object_ptr, FALSE);
register_icall (mono_gc_wbarrier_generic_nostore_internal, mono_icall_sig_void_ptr, TRUE);
Expand Down Expand Up @@ -2085,7 +2086,7 @@ free_signature_pointer_pair (SignaturePointerPair *pair)
MonoMethod *
mono_marshal_get_delegate_invoke_internal (MonoMethod *method, gboolean callvirt, gboolean static_method_with_first_arg_bound, MonoMethod *target_method)
{
MonoMethodSignature *sig, *invoke_sig;
MonoMethodSignature *sig, *invoke_sig, *target_method_sig = NULL;
MonoMethodBuilder *mb;
MonoMethod *res;
GHashTable *cache;
Expand Down Expand Up @@ -2129,6 +2130,11 @@ mono_marshal_get_delegate_invoke_internal (MonoMethod *method, gboolean callvirt
}

closed_over_null = sig->param_count == mono_method_signature_internal (target_method)->param_count;

/*
* We don't want to use target_method's signature because it can be freed early
*/
target_method_sig = mono_method_signature_internal (target_method);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The caching code needs to be modified to cache based on either the class or the signature. I.e. this code:

	} else if (callvirt) {
		GHashTable **cache_ptr;

		cache_ptr = &mono_method_get_wrapper_cache (method)->delegate_abstract_invoke_cache;

and:

	} else if (callvirt) {
		new_key = g_new0 (SignaturePointerPair, 1);
		*new_key = key;


if (static_method_with_first_arg_bound) {
Expand Down Expand Up @@ -2239,7 +2245,7 @@ mono_marshal_get_delegate_invoke_internal (MonoMethod *method, gboolean callvirt
/* FIXME: Other subtypes */
mb->mem_manager = m_method_get_mem_manager (method);

get_marshal_cb ()->emit_delegate_invoke_internal (mb, sig, invoke_sig, static_method_with_first_arg_bound, callvirt, closed_over_null, method, target_method, target_class, ctx, container);
get_marshal_cb ()->emit_delegate_invoke_internal (mb, sig, invoke_sig, target_method_sig, static_method_with_first_arg_bound, callvirt, closed_over_null, method, target_method, target_class, ctx, container);

get_marshal_cb ()->mb_skip_visibility (mb);

Expand Down Expand Up @@ -5481,6 +5487,31 @@ mono_struct_delete_old (MonoClass *klass, char *ptr)
}
}

void*
mono_get_addr_compiled_method (MonoObject *object, MonoMethod *method)
{
ERROR_DECL (error);
MonoMethod *res;
gpointer addr;

if (object == NULL) {
mono_error_set_null_reference (error);
mono_error_set_pending_exception (error);
return NULL;
}

res = mono_object_get_virtual_method_internal (object, method);

addr = mono_compile_method_checked (res, error);

if (!is_ok (error)) {
mono_error_set_pending_exception (error);
return NULL;
}

return addr;
}

void
ves_icall_System_Runtime_InteropServices_Marshal_DestroyStructure (gpointer src, MonoReflectionTypeHandle type, MonoError *error)
{
Expand Down
6 changes: 5 additions & 1 deletion src/mono/mono/metadata/marshal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ typedef struct {
void (*emit_runtime_invoke_dynamic) (MonoMethodBuilder *mb);
void (*emit_delegate_begin_invoke) (MonoMethodBuilder *mb, MonoMethodSignature *sig);
void (*emit_delegate_end_invoke) (MonoMethodBuilder *mb, MonoMethodSignature *sig);
void (*emit_delegate_invoke_internal) (MonoMethodBuilder *mb, MonoMethodSignature *sig, MonoMethodSignature *invoke_sig, gboolean static_method_with_first_arg_bound, gboolean callvirt, gboolean closed_over_null, MonoMethod *method, MonoMethod *target_method, MonoClass *target_class, MonoGenericContext *ctx, MonoGenericContainer *container);
void (*emit_delegate_invoke_internal) (MonoMethodBuilder *mb, MonoMethodSignature *sig, MonoMethodSignature *invoke_sig, MonoMethodSignature *target_method_sig, gboolean static_method_with_first_arg_bound, gboolean callvirt, gboolean closed_over_null, MonoMethod *method, MonoMethod *target_method, MonoClass *target_class, MonoGenericContext *ctx, MonoGenericContainer *container);
void (*emit_synchronized_wrapper) (MonoMethodBuilder *mb, MonoMethod *method, MonoGenericContext *ctx, MonoGenericContainer *container, MonoMethod *enter_method, MonoMethod *exit_method, MonoMethod *gettypefromhandle_method);
void (*emit_unbox_wrapper) (MonoMethodBuilder *mb, MonoMethod *method);
void (*emit_array_accessor_wrapper) (MonoMethodBuilder *mb, MonoMethod *method, MonoMethodSignature *sig, MonoGenericContext *ctx);
Expand Down Expand Up @@ -624,6 +624,10 @@ ICALL_EXPORT
void
mono_struct_delete_old (MonoClass *klass, char *ptr);

ICALL_EXPORT
void*
mono_get_addr_compiled_method (MonoObject *object, MonoMethod *method);

int
mono_emit_marshal (EmitMarshalContext *m, int argnum, MonoType *t,
MonoMarshalSpec *spec, int conv_arg,
Expand Down
18 changes: 18 additions & 0 deletions src/tests/JIT/Delegate/Tests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
using System;
using System.Runtime.InteropServices;

public class Tests
{
public static int Main () {
try {
var del = (Func<string, string>)Delegate.CreateDelegate (typeof (Func<string, string>), null, typeof (object).GetMethod ("ToString"));
Console.WriteLine (del ("FOO"));
} catch(Exception e) {
vargaz marked this conversation as resolved.
Show resolved Hide resolved
Console.WriteLine(e);
return 0;
}

return 100;

}
}
12 changes: 12 additions & 0 deletions src/tests/JIT/Delegate/Tests.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<Project Sdk="Microsoft.NET.Sdk">
lambdageek marked this conversation as resolved.
Show resolved Hide resolved
<PropertyGroup>
<OutputType>Exe</OutputType>
</PropertyGroup>
<PropertyGroup>
<DebugType>PdbOnly</DebugType>
<Optimize>True</Optimize>
</PropertyGroup>
<ItemGroup>
<Compile Include="Tests.cs" />
</ItemGroup>
</Project>