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 1 commit
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
2 changes: 1 addition & 1 deletion src/mono/mono/metadata/loader-internals.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,11 @@ typedef struct {
GHashTable *delegate_end_invoke_cache;
GHashTable *runtime_invoke_signature_cache;
GHashTable *runtime_invoke_sig_cache;
GHashTable *delegate_abstract_invoke_cache;

/*
* indexed by SignaturePointerPair
*/
GHashTable *delegate_abstract_invoke_cache;
GHashTable *delegate_bound_static_invoke_cache;

/*
Expand Down
20 changes: 6 additions & 14 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 @@ -2088,20 +2088,12 @@ emit_delegate_invoke_internal_ilgen (MonoMethodBuilder *mb, MonoMethodSignature

if (callvirt) {
if (!closed_over_null) {
/* if target_method is not really virtual, turn it into a direct call */
if (!(target_method->flags & METHOD_ATTRIBUTE_VIRTUAL) || m_class_is_valuetype (target_class)) {
mono_mb_emit_ldarg (mb, 1);
for (i = 1; i < sig->param_count; ++i)
mono_mb_emit_ldarg (mb, i + 1);
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_ldarg (mb, 0);
mono_mb_emit_icall (mb, mono_get_addr_compiled_method);
mono_mb_emit_op (mb, CEE_CALLI, target_method_sig);
}
for (i = 1; i <= sig->param_count; ++i)
mono_mb_emit_ldarg (mb, i);
mono_mb_emit_ldarg (mb, 1);
mono_mb_emit_ldarg (mb, 0);
mono_mb_emit_icall (mb, mono_get_addr_compiled_method);
mono_mb_emit_op (mb, CEE_CALLI, target_method_sig);
} else {
mono_mb_emit_byte (mb, CEE_LDNULL);
for (i = 0; i < sig->param_count; ++i)
Expand Down
26 changes: 11 additions & 15 deletions src/mono/mono/metadata/marshal.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2194,17 +2194,16 @@ mono_marshal_get_delegate_invoke_internal (MonoMethod *method, gboolean callvirt

cache_ptr = &mono_method_get_wrapper_cache (method)->delegate_abstract_invoke_cache;

/* We need to cache the signature+method pair */
/* We need to cache the signature */
mono_marshal_lock ();
if (!*cache_ptr)
*cache_ptr = g_hash_table_new_full (signature_pointer_pair_hash, (GEqualFunc)signature_pointer_pair_equal, (GDestroyNotify)free_signature_pointer_pair, NULL);
cache = *cache_ptr;
key.sig = invoke_sig;
key.pointer = target_method;
res = (MonoMethod *)g_hash_table_lookup (cache, &key);
cache = get_cache (cache_ptr,
(GHashFunc)mono_signature_hash,
(GCompareFunc)mono_metadata_signature_equal);
res = (MonoMethod *)g_hash_table_lookup (cache, invoke_sig);
mono_marshal_unlock ();
if (res)
return res;
cache_key = invoke_sig;
} else {
// Inflated methods should not be in this cache because it's not stored on the imageset.
g_assert (!method->is_inflated);
Expand Down Expand Up @@ -2257,13 +2256,6 @@ mono_marshal_get_delegate_invoke_internal (MonoMethod *method, gboolean callvirt

def = mono_mb_create_and_cache_full (cache, cache_key, mb, sig, sig->param_count + 16, info, NULL);
res = cache_generic_delegate_wrapper (cache, orig_method, def, ctx);
} else if (callvirt) {
new_key = g_new0 (SignaturePointerPair, 1);
*new_key = key;

res = mono_mb_create_and_cache_full (cache, new_key, mb, sig, sig->param_count + 16, info, &found);
if (found)
g_free (new_key);
} else {
res = mono_mb_create_and_cache_full (cache, cache_key, mb, sig, sig->param_count + 16, info, NULL);
}
Expand Down Expand Up @@ -5493,6 +5485,7 @@ mono_get_addr_compiled_method (MonoObject *object, MonoDelegate *del)
ERROR_DECL (error);
MonoMethod *res, *method = del->method;
gpointer addr;
gboolean need_unbox = FALSE;

if (object == NULL) {
mono_error_set_null_reference (error);
Expand All @@ -5509,6 +5502,9 @@ mono_get_addr_compiled_method (MonoObject *object, MonoDelegate *del)
return NULL;
}

need_unbox = m_class_is_valuetype (res->klass) && !m_class_is_valuetype (method->klass);
addr = mono_get_runtime_callbacks ()->add_delegate_trampolines (res, addr, need_unbox);

return addr;
}

Expand Down Expand Up @@ -6287,7 +6283,7 @@ mono_marshal_free_dynamic_wrappers (MonoMethod *method)
if (image->wrapper_caches.runtime_invoke_method_cache)
clear_runtime_invoke_method_cache (image->wrapper_caches.runtime_invoke_method_cache, method);
if (image->wrapper_caches.delegate_abstract_invoke_cache)
g_hash_table_foreach_remove (image->wrapper_caches.delegate_abstract_invoke_cache, signature_pointer_pair_matches_pointer, method);
g_hash_table_remove (image->wrapper_caches.delegate_abstract_invoke_cache, mono_method_signature_internal (method));
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not be needed anymore since the wrappers are no longer associated with the method.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And how is that cache going to be freed if we remove it from here?

Copy link
Contributor

Choose a reason for hiding this comment

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

Its freed already in marshal.c, we just don't need to handle it here, since these wrappers will no longer depend on dynamic methods.

// FIXME: Need to clear the caches in other images as well
if (image->wrapper_caches.delegate_bound_static_invoke_cache)
g_hash_table_remove (image->wrapper_caches.delegate_bound_static_invoke_cache, mono_method_signature_internal (method));
Expand Down
1 change: 1 addition & 0 deletions src/mono/mono/metadata/object-internals.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -700,6 +700,7 @@ typedef struct {
void (*interp_jit_info_foreach)(InterpJitInfoFunc func, gpointer user_data);
gboolean (*interp_sufficient_stack)(gsize size);
void (*init_class) (MonoClass *klass);
gpointer (*add_delegate_trampolines) (MonoMethod *method, gpointer compiled_method, gboolean need_unbox);
MonoArray *(*get_trace) (MonoException *exc, gint32 skip, MonoBoolean need_file_info);
MonoBoolean (*get_frame_info) (gint32 skip, MonoMethod **out_method,
MonoDebugSourceLocation **out_location,
Expand Down
1 change: 1 addition & 0 deletions src/mono/mono/mini/mini-runtime.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4582,6 +4582,7 @@ mini_init (const char *filename)
callbacks.init_class = init_class;
callbacks.get_trace = mono_get_trace;
callbacks.get_frame_info = mono_get_frame_info;
callbacks.add_delegate_trampolines = mono_add_delegate_trampolines;

mono_install_callbacks (&callbacks);

Expand Down
9 changes: 9 additions & 0 deletions src/mono/mono/mini/mini-trampolines.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1458,6 +1458,15 @@ mono_create_delegate_trampoline (MonoClass *klass)
return mono_create_delegate_trampoline_info (klass, NULL, FALSE)->invoke_impl;
}

gpointer mono_add_delegate_trampolines (MonoMethod *method, gpointer compiled_method, gboolean need_unbox)
{
gpointer addr;

addr = mini_add_method_trampoline (method, compiled_method, mono_method_needs_static_rgctx_invoke (method, TRUE), need_unbox);
vargaz marked this conversation as resolved.
Show resolved Hide resolved

return addr;
}

gpointer
mono_create_rgctx_lazy_fetch_trampoline (guint32 offset)
{
Expand Down
1 change: 1 addition & 0 deletions src/mono/mono/mini/mini.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2271,6 +2271,7 @@ gpointer mono_create_jump_trampoline (MonoMethod *method,
gpointer mono_create_jit_trampoline (MonoMethod *method, MonoError *error);
gpointer mono_create_jit_trampoline_from_token (MonoImage *image, guint32 token);
gpointer mono_create_delegate_trampoline (MonoClass *klass);
gpointer mono_add_delegate_trampolines (MonoMethod *method, gpointer compiled_method, gboolean need_unbox);
MonoDelegateTrampInfo* mono_create_delegate_trampoline_info (MonoClass *klass, MonoMethod *method, gboolean is_virtual);
gpointer mono_create_rgctx_lazy_fetch_trampoline (guint32 offset);
gpointer mono_create_static_rgctx_trampoline (MonoMethod *m, gpointer addr);
Expand Down