Change callvirt into calli for virtual delegates (#83461)

JIT delegates do not depend on the target method and an call is used

---------

Signed-off-by: Vlad - Alexandru Ionescu <[email protected]>
Co-authored-by: Vlad - Alexandru Ionescu <[email protected]>

src/mono/mono/metadata/icall-signatures.h

@@ -206,6 +206,7 @@ ICALL_SIG (3, (ptr, object, int))		\
 ICALL_SIG (3, (ptr, ptr, int))			\
 ICALL_SIG (3, (ptr, ptr, int32))		\
 ICALL_SIG (3, (ptr, ptr, ptr))			\
+ICALL_SIG (3, (ptr, ptr, object))       \
 ICALL_SIG (3, (ptr, ptr, ptrref))		\
 ICALL_SIG (3, (ptr, uint32, ptrref))		\
 ICALL_SIG (3, (uint32, double, double))		\

src/mono/mono/metadata/icall.c

@@ -6176,7 +6176,7 @@ mono_method_get_unmanaged_wrapper_ftnptr_internal (MonoMethod *method, gboolean
 	} else {
 		g_assert (!only_unmanaged_callers_only);
 	}
-	return mono_get_runtime_callbacks ()->get_ftnptr (method, error);
+	return mono_get_runtime_callbacks ()->get_ftnptr (method, FALSE, error);
 }
 
 MonoBoolean

src/mono/mono/metadata/jit-icall-reg.h

@@ -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) \

src/mono/mono/metadata/loader-internals.h

@@ -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;
 
 	/*

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

@@ -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;
@@ -2088,19 +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 {
-				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);
-			}
+			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)

src/mono/mono/metadata/marshal.c

@@ -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_ptr_object, 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);
@@ -2085,13 +2086,11 @@ 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;
 	gpointer cache_key = NULL;
-	SignaturePointerPair key = { NULL, NULL };
-	SignaturePointerPair *new_key;
 	char *name;
 	MonoClass *target_class = NULL;
 	gboolean closed_over_null = FALSE;
@@ -2101,7 +2100,6 @@ mono_marshal_get_delegate_invoke_internal (MonoMethod *method, gboolean callvirt
 	WrapperInfo *info;
 	WrapperSubtype subtype = WRAPPER_SUBTYPE_NONE;
 	MonoMemoryManager *mem_manager = NULL;
-	gboolean found;
 
 	g_assert (method && m_class_get_parent (method->klass) == mono_defaults.multicastdelegate_class &&
 		  !strcmp (method->name, "Invoke"));
@@ -2129,6 +2127,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);
 	}
 
 	if (static_method_with_first_arg_bound) {
@@ -2188,17 +2191,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);
@@ -2239,7 +2241,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);
 
@@ -2251,13 +2253,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);
 	}
@@ -5481,6 +5476,49 @@ mono_struct_delete_old (MonoClass *klass, char *ptr)
 	}
 }
 
+void*
+mono_get_addr_compiled_method (gpointer arg, MonoDelegate *del)
+{
+	ERROR_DECL (error);
+	MonoMethod *res, *method = del->method;
+	gpointer addr;
+	gboolean need_unbox = FALSE;
+
+	if (arg == NULL) {
+		mono_error_set_null_reference (error);
+		mono_error_set_pending_exception (error);
+		return NULL;
+	}
+
+	MonoClass *klass = del->object.vtable->klass;
+	MonoMethod *invoke = mono_get_delegate_invoke_internal (klass);
+	MonoMethodSignature *invoke_sig = mono_method_signature_internal (invoke);
+
+	MonoClass *arg_class = NULL;
+	if (m_type_is_byref (invoke_sig->params [0])) {
+		arg_class = mono_class_from_mono_type_internal (invoke_sig->params [0]);
+	} else {
+		MonoObject *object = (MonoObject*)arg;
+		arg_class = object->vtable->klass;
+	}
+
+	res = mono_class_get_virtual_method (arg_class, method, error);
+
+	if (!is_ok (error)) {
+		mono_error_set_pending_exception (error);
+		return NULL;
+	}
+
+	need_unbox = m_class_is_valuetype (res->klass) && !m_class_is_valuetype (method->klass);
+	addr = mono_get_runtime_callbacks ()->get_ftnptr (res, need_unbox, 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)
 {
@@ -6255,8 +6293,6 @@ 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);
 	// 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));

src/mono/mono/metadata/marshal.h

@@ -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);
@@ -624,6 +624,10 @@ ICALL_EXPORT
 void
 mono_struct_delete_old (MonoClass *klass, char *ptr);
 
+ICALL_EXPORT
+void*
+mono_get_addr_compiled_method (gpointer arg, MonoDelegate *del);
+
 int
 mono_emit_marshal (EmitMarshalContext *m, int argnum, MonoType *t,
 	      MonoMarshalSpec *spec, int conv_arg,

src/mono/mono/metadata/object-internals.h

@@ -695,7 +695,7 @@ typedef struct {
 	void (*get_jit_stats)(gint64 *methods_compiled, gint64 *cil_code_size_bytes, gint64 *native_code_size_bytes, gint64 *jit_time);
 	void (*get_exception_stats)(guint32 *exception_count);
 	// Same as compile_method, but returns a MonoFtnDesc in llvmonly mode
-	gpointer (*get_ftnptr)(MonoMethod *method, MonoError *error);
+	gpointer (*get_ftnptr)(MonoMethod *method, gboolean need_unbox, MonoError *error);
 	void (*interp_jit_info_foreach)(InterpJitInfoFunc func, gpointer user_data);
 	gboolean (*interp_sufficient_stack)(gsize size);
 	void (*init_class) (MonoClass *klass);

src/mono/mono/mini/mini-runtime.c

@@ -2817,11 +2817,11 @@ mono_jit_compile_method_jit_only (MonoMethod *method, MonoError *error)
  * On llvmonly, this returns a MonoFtnDesc, otherwise it returns a normal function pointer.
  */
 static gpointer
-get_ftnptr_for_method (MonoMethod *method, MonoError *error)
+get_ftnptr_for_method (MonoMethod *method, gboolean need_unbox, MonoError *error)
 {
 	if (!mono_llvm_only) {
 		gpointer res = mono_jit_compile_method (method, error);
-		res = mini_add_method_trampoline (method, res, mono_method_needs_static_rgctx_invoke (method, TRUE), FALSE);
+		res = mini_add_method_trampoline (method, res, mono_method_needs_static_rgctx_invoke (method, TRUE), need_unbox);
 		return res;
 	} else {
 		return mini_llvmonly_load_method_ftndesc (method, FALSE, FALSE, error);

src/tests/JIT/Methodical/delegate/GSDelegate.cs

@@ -0,0 +1,53 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+using System.Reflection;
+
+
+public interface IGetContents<T> {
+    (string, int, T) GetContents();
+}
+public struct MyStruct<T> : IGetContents<T> {
+    public string s;
+    public int a;
+    public T t;
+
+    public (string, int, T) GetContents()
+    {
+        return (s, a, t);
+    }
+}
+
+public class Program {
+
+    public delegate (string, int, T) MyDelegate<T>(IGetContents<T> arg);
+
+    public static int Main(string[] args)
+    {
+        int retVal = 100;
+
+        try {
+            MyStruct<string> myStruct = new MyStruct<string>();
+            myStruct.s = "test1";
+            myStruct.a = 42;
+            myStruct.t = "test2";
+
+            MethodInfo mi = typeof(IGetContents<string>).GetMethod("GetContents");
+            MyDelegate<string> func = (MyDelegate<string>)mi.CreateDelegate(typeof(MyDelegate<string>));
+
+            (string c1, int c2, string c3) = func(myStruct);
+            if (c1 != "test1")
+                retVal = 1;
+            if (c2 != 42)
+                retVal = 2;
+            if (c3 != "test2")
+                retVal = 3;
+        } catch (Exception e) {
+            Console.WriteLine(e);
+            retVal = 1;
+        }
+
+        return retVal;
+    }
+}

src/tests/JIT/Methodical/delegate/GSDelegate.csproj

@@ -0,0 +1,13 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+    <RequiresProcessIsolation>true</RequiresProcessIsolation>
+  </PropertyGroup>
+  <PropertyGroup>
+    <DebugType>PdbOnly</DebugType>
+    <Optimize>True</Optimize>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="GSDelegate.cs" />
+  </ItemGroup>
+</Project>

src/tests/JIT/Methodical/delegate/VirtualDelegate.cs

@@ -0,0 +1,24 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+
+using System;
+using System.Runtime.InteropServices;
+
+public class VirtualDelegate
+{
+    public static int Main () {
+        int retVal = 100;
+        try {
+            var del = (Func<string, string>)Delegate.CreateDelegate (typeof (Func<string, string>), null, typeof (object).GetMethod ("ToString"));
+            if (del ("FOO") != "FOO")
+                retVal = 1;
+        } catch(Exception e) {
+            Console.WriteLine(e);
+            retVal = 1;
+        }
+
+        return retVal;
+
+    }
+}

src/tests/JIT/Methodical/delegate/VirtualDelegate.csproj

@@ -0,0 +1,13 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+    <RequiresProcessIsolation>true</RequiresProcessIsolation>
+  </PropertyGroup>
+  <PropertyGroup>
+    <DebugType>PdbOnly</DebugType>
+    <Optimize>True</Optimize>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="VirtualDelegate.cs" />
+  </ItemGroup>
+</Project>