From 957e6fa4ddb9a4cdae818b49850a7b6949a5e6a2 Mon Sep 17 00:00:00 2001 From: Jay Krell Date: Mon, 7 Oct 2019 07:40:22 -0700 Subject: [PATCH] [Coop] Convert Get/SetGenericValueImpl. (#17034) This is an alternative to https://github.com/mono/mono/pull/16994 and does *not* depend on https://github.com/mono/mono/pull/16987 or https://github.com/mono/mono/pull/17009 or similar. --- configure.ac | 2 +- mcs/class/corlib/System/Array.cs | 23 ++++++++++++++++++++-- mono/metadata/class.c | 1 + mono/metadata/icall-decl.h | 4 ++-- mono/metadata/icall-def-netcore.h | 8 ++------ mono/metadata/icall-def.h | 8 ++------ mono/metadata/icall.c | 23 ++++++++-------------- mono/metadata/marshal-ilgen.c | 2 +- mono/mini/aot-compiler.c | 9 ++++----- mono/mini/aot-runtime.c | 6 +++--- mono/mini/intrinsics.c | 4 ++-- netcore/System.Private.CoreLib/src/System/Array.cs | 21 ++++++++++++++++++-- 12 files changed, 66 insertions(+), 45 deletions(-) diff --git a/configure.ac b/configure.ac index 12717c4a0f4..b979751740a 100644 --- a/configure.ac +++ b/configure.ac @@ -63,7 +63,7 @@ MONO_VERSION_BUILD=`echo $VERSION | cut -d . -f 3` # This line is parsed by tools besides autoconf, such as msvc/mono.winconfig.targets. # It should remain in the format they expect. # -MONO_CORLIB_VERSION=de62d46f-9502-49c7-9842-841f367a6a3a +MONO_CORLIB_VERSION=7b616e7f-0b14-48da-9d67-9446aad4fc5e # # Put a quoted #define in config.h. diff --git a/mcs/class/corlib/System/Array.cs b/mcs/class/corlib/System/Array.cs index 70998486558..1ab627110a5 100644 --- a/mcs/class/corlib/System/Array.cs +++ b/mcs/class/corlib/System/Array.cs @@ -110,6 +110,7 @@ namespace System int length = this.Length; for (int i = 0; i < length; i++) { T value; + // Do not change this to call GetGenericValue_icall directly, due to special casing in the runtime. GetGenericValueImpl (i, out value); if (item == null){ if (value == null) { @@ -138,6 +139,7 @@ namespace System throw new ArgumentOutOfRangeException ("index"); T value; + // Do not change this to call GetGenericValue_icall directly, due to special casing in the runtime. GetGenericValueImpl (index, out value); return value; } @@ -165,6 +167,7 @@ namespace System int length = this.Length; for (int i = 0; i < length; i++) { T value; + // Do not change this to call GetGenericValue_icall directly, due to special casing in the runtime. GetGenericValueImpl (i, out value); if (item == null){ if (value == null) @@ -190,6 +193,7 @@ namespace System throw new ArgumentOutOfRangeException ("index"); T value; + // Do not change this to call GetGenericValue_icall directly, due to special casing in the runtime. GetGenericValueImpl (index, out value); return value; } @@ -204,16 +208,31 @@ namespace System oarray [index] = (object)item; return; } + // Do not change this to call SetGenericValue_icall directly, due to special casing in the runtime. SetGenericValueImpl (index, ref item); } // CAUTION! No bounds checking! [MethodImplAttribute (MethodImplOptions.InternalCall)] - internal extern void GetGenericValueImpl (int pos, out T value); + extern static void GetGenericValue_icall (ref Array self, int pos, out T value); // CAUTION! No bounds checking! [MethodImplAttribute (MethodImplOptions.InternalCall)] - internal extern void SetGenericValueImpl (int pos, ref T value); + extern static void SetGenericValue_icall (ref Array self, int pos, ref T value); + + // This is a special case in the runtime. + internal void GetGenericValueImpl (int pos, out T value) + { + var self = this; + GetGenericValue_icall (ref self, pos, out value); + } + + // This is a special case in the runtime. + internal void SetGenericValueImpl (int pos, ref T value) + { + var self = this; + SetGenericValue_icall (ref self, pos, ref value); + } internal struct InternalEnumerator : IEnumerator { diff --git a/mono/metadata/class.c b/mono/metadata/class.c index 419e88089ea..f76724d2f95 100644 --- a/mono/metadata/class.c +++ b/mono/metadata/class.c @@ -2002,6 +2002,7 @@ mono_class_from_mono_type (MonoType *type) MonoClass * mono_class_from_mono_type_internal (MonoType *type) { + g_assert (type); switch (type->type) { case MONO_TYPE_OBJECT: return type->data.klass? type->data.klass: mono_defaults.object_class; diff --git a/mono/metadata/icall-decl.h b/mono/metadata/icall-decl.h index e3a38a1f1f5..b7b1da9adbc 100644 --- a/mono/metadata/icall-decl.h +++ b/mono/metadata/icall-decl.h @@ -166,8 +166,8 @@ ICALL_EXPORT int ves_icall_System_Threading_Thread_SystemMaxStackSize (void); ICALL_EXPORT int ves_icall_get_method_attributes (MonoMethod* method); ICALL_EXPORT void ves_icall_Mono_Runtime_RegisterReportingForNativeLib (const char*, const char*); ICALL_EXPORT void ves_icall_Mono_Runtime_RegisterReportingForAllNativeLibs (void); -ICALL_EXPORT void ves_icall_System_Array_GetGenericValueImpl (MonoArray*, guint32, gpointer); -ICALL_EXPORT void ves_icall_System_Array_SetGenericValueImpl (MonoArray*, guint32, gpointer); +ICALL_EXPORT void ves_icall_System_Array_GetGenericValue_icall (MonoArray**, guint32, gpointer); +ICALL_EXPORT void ves_icall_System_Array_SetGenericValue_icall (MonoArray**, guint32, gpointer); ICALL_EXPORT void ves_icall_System_Buffer_MemcpyInternal (gpointer dest, gconstpointer src, gint32 count); ICALL_EXPORT void ves_icall_System_Environment_Exit (int); ICALL_EXPORT void ves_icall_System_GCHandle_FreeHandle (guint32 handle); diff --git a/mono/metadata/icall-def-netcore.h b/mono/metadata/icall-def-netcore.h index bfd0c17188a..42ff627c04d 100644 --- a/mono/metadata/icall-def-netcore.h +++ b/mono/metadata/icall-def-netcore.h @@ -41,18 +41,14 @@ HANDLES(ARRAY_0, "CanChangePrimitive", ves_icall_System_Array_CanChangePrimitiv HANDLES(ARRAY_1, "ClearInternal", ves_icall_System_Array_ClearInternal, void, 3, (MonoArray, int, int)) HANDLES(ARRAY_3, "CreateInstanceImpl", ves_icall_System_Array_CreateInstanceImpl, MonoArray, 3, (MonoReflectionType, MonoArray, MonoArray)) HANDLES(ARRAY_4, "FastCopy", ves_icall_System_Array_FastCopy, MonoBoolean, 5, (MonoArray, int, MonoArray, int, int)) -// Generic ref/out parameters are not supported by HANDLES(), so NOHANDLES(). -// FIXME HANDLES should handle this trivially. Do it separately. -NOHANDLES(ICALL(ARRAY_5, "GetGenericValueImpl", ves_icall_System_Array_GetGenericValueImpl)) +NOHANDLES(ICALL(ARRAY_5, "GetGenericValue_icall", ves_icall_System_Array_GetGenericValue_icall)) HANDLES(ARRAY_6, "GetLength", ves_icall_System_Array_GetLength, gint32, 2, (MonoArray, gint32)) HANDLES(ARRAY_15, "GetLongLength", ves_icall_System_Array_GetLongLength, gint64, 2, (MonoArray, gint32)) HANDLES(ARRAY_7, "GetLowerBound", ves_icall_System_Array_GetLowerBound, gint32, 2, (MonoArray, gint32)) HANDLES(ARRAY_8, "GetRank", ves_icall_System_Array_GetRank, gint32, 1, (MonoObject)) HANDLES(ARRAY_9, "GetValue", ves_icall_System_Array_GetValue, MonoObject, 2, (MonoArray, MonoArray)) HANDLES(ARRAY_10, "GetValueImpl", ves_icall_System_Array_GetValueImpl, MonoObject, 2, (MonoArray, guint32)) -// Generic ref/out parameters are not supported by HANDLES(), so NOHANDLES(). -// FIXME HANDLES should handle this trivially. Do it separately. -NOHANDLES(ICALL(ARRAY_11, "SetGenericValueImpl", ves_icall_System_Array_SetGenericValueImpl)) +NOHANDLES(ICALL(ARRAY_11, "SetGenericValue_icall", ves_icall_System_Array_SetGenericValue_icall)) HANDLES(ARRAY_12, "SetValue", ves_icall_System_Array_SetValue, void, 3, (MonoArray, MonoObject, MonoArray)) HANDLES(ARRAY_13, "SetValueImpl", ves_icall_System_Array_SetValueImpl, void, 3, (MonoArray, MonoObject, guint32)) HANDLES(ARRAY_14, "SetValueRelaxedImpl", ves_icall_System_Array_SetValueRelaxedImpl, void, 3, (MonoArray, MonoObject, guint32)) diff --git a/mono/metadata/icall-def.h b/mono/metadata/icall-def.h index 0da387ebceb..74a3a004ad3 100644 --- a/mono/metadata/icall-def.h +++ b/mono/metadata/icall-def.h @@ -210,18 +210,14 @@ ICALL_TYPE(ARRAY, "System.Array", ARRAY_1) HANDLES(ARRAY_1, "ClearInternal", ves_icall_System_Array_ClearInternal, void, 3, (MonoArray, int, int)) HANDLES(ARRAY_3, "CreateInstanceImpl", ves_icall_System_Array_CreateInstanceImpl, MonoArray, 3, (MonoReflectionType, MonoArray, MonoArray)) HANDLES(ARRAY_4, "FastCopy", ves_icall_System_Array_FastCopy, MonoBoolean, 5, (MonoArray, int, MonoArray, int, int)) -// Generic ref/out parameters are not supported by HANDLES(), so NOHANDLES(). -// FIXME HANDLES should handle this trivially. Do it separately. -NOHANDLES(ICALL(ARRAY_5, "GetGenericValueImpl", ves_icall_System_Array_GetGenericValueImpl)) +NOHANDLES(ICALL(ARRAY_5, "GetGenericValue_icall", ves_icall_System_Array_GetGenericValue_icall)) HANDLES(ARRAY_6, "GetLength", ves_icall_System_Array_GetLength, gint32, 2, (MonoArray, gint32)) HANDLES(ARRAY_15, "GetLongLength", ves_icall_System_Array_GetLongLength, gint64, 2, (MonoArray, gint32)) HANDLES(ARRAY_7, "GetLowerBound", ves_icall_System_Array_GetLowerBound, gint32, 2, (MonoArray, gint32)) HANDLES(ARRAY_8, "GetRank", ves_icall_System_Array_GetRank, gint32, 1, (MonoObject)) HANDLES(ARRAY_9, "GetValue", ves_icall_System_Array_GetValue, MonoObject, 2, (MonoArray, MonoArray)) HANDLES(ARRAY_10, "GetValueImpl", ves_icall_System_Array_GetValueImpl, MonoObject, 2, (MonoArray, guint32)) -// Generic ref/out parameters are not supported by HANDLES(), so NOHANDLES(). -// FIXME HANDLES should handle this trivially. Do it separately. -NOHANDLES(ICALL(ARRAY_11, "SetGenericValueImpl", ves_icall_System_Array_SetGenericValueImpl)) +NOHANDLES(ICALL(ARRAY_11, "SetGenericValue_icall", ves_icall_System_Array_SetGenericValue_icall)) HANDLES(ARRAY_12, "SetValue", ves_icall_System_Array_SetValue, void, 3, (MonoArray, MonoObject, MonoArray)) HANDLES(ARRAY_13, "SetValueImpl", ves_icall_System_Array_SetValueImpl, void, 3, (MonoArray, MonoObject, guint32)) diff --git a/mono/metadata/icall.c b/mono/metadata/icall.c index 74ecfe2a750..6850e49405d 100644 --- a/mono/metadata/icall.c +++ b/mono/metadata/icall.c @@ -1015,38 +1015,31 @@ ves_icall_System_Array_FastCopy (MonoArrayHandle source, int source_idx, MonoArr } void -ves_icall_System_Array_GetGenericValueImpl (MonoArray *arr, guint32 pos, gpointer value) +ves_icall_System_Array_GetGenericValue_icall (MonoArray **arr, guint32 pos, gpointer value) { - // FIXME? - // Generic ref/out parameters are not supported by HANDLES(), so NOHANDLES(). - - icallarray_print ("%s arr:%p pos:%u value:%p\n", __func__, arr, pos, value); + icallarray_print ("%s arr:%p pos:%u value:%p\n", __func__, *arr, pos, value); MONO_REQ_GC_UNSAFE_MODE; // because of gpointer value - MonoClass * const ac = mono_object_class (arr); + MonoClass * const ac = mono_object_class (*arr); gsize const esize = mono_array_element_size (ac); - gconstpointer * const ea = (gconstpointer*)((char*)arr->vector + (pos * esize)); + gconstpointer * const ea = (gconstpointer*)((char*)(*arr)->vector + (pos * esize)); mono_gc_memmove_atomic (value, ea, esize); } void -ves_icall_System_Array_SetGenericValueImpl (MonoArray *arr, guint32 pos, gpointer value) +ves_icall_System_Array_SetGenericValue_icall (MonoArray **arr, guint32 pos, gpointer value) { - // FIXME? - // Generic ref/out parameters are not supported by HANDLES(), so NOHANDLES(). - - icallarray_print ("%s arr:%p pos:%u value:%p\n", __func__, arr, pos, value); + icallarray_print ("%s arr:%p pos:%u value:%p\n", __func__, *arr, pos, value); MONO_REQ_GC_UNSAFE_MODE; // because of gpointer value - - MonoClass * const ac = mono_object_class (arr); + MonoClass * const ac = mono_object_class (*arr); MonoClass * const ec = m_class_get_element_class (ac); gsize const esize = mono_array_element_size (ac); - gpointer * const ea = (gpointer*)((char*)arr->vector + (pos * esize)); + gpointer * const ea = (gpointer*)((char*)(*arr)->vector + (pos * esize)); if (MONO_TYPE_IS_REFERENCE (m_class_get_byval_arg (ec))) { g_assert (esize == sizeof (gpointer)); diff --git a/mono/metadata/marshal-ilgen.c b/mono/metadata/marshal-ilgen.c index f1aaa2a7234..fdc385cbec6 100644 --- a/mono/metadata/marshal-ilgen.c +++ b/mono/metadata/marshal-ilgen.c @@ -6283,7 +6283,7 @@ typedef enum { /* Wrap the argument (a valuetype reference) in a handle to pin its enclosing object, but pass the raw reference to the icall. This is also how we pass byref generic parameter arguments to generic method - icalls (eg, System.Array:GetGenericValueImpl(int idx, T out value)) */ + icalls (e.g. System.Array:GetGenericValue_icall(int idx, T out value)) */ ICALL_HANDLES_WRAP_VALUETYPE_REF, } IcallHandlesWrap; diff --git a/mono/mini/aot-compiler.c b/mono/mini/aot-compiler.c index d38f16bb2fc..54ccdce60db 100644 --- a/mono/mini/aot-compiler.c +++ b/mono/mini/aot-compiler.c @@ -5679,22 +5679,21 @@ add_generic_instances (MonoAotCompile *acfg) add_instances_of (acfg, klass, insts, ninsts, TRUE); /* - * Add a managed-to-native wrapper of Array.GetGenericValueImpl, which is - * used for all instances of GetGenericValueImpl by the AOT runtime. + * Add a managed-to-native wrapper of Array.GetGenericValue_icall, which is + * used for all instances of GetGenericValue_icall by the AOT runtime. */ { ERROR_DECL (error); MonoGenericContext ctx; - MonoType *args [16]; MonoMethod *get_method; MonoClass *array_klass = m_class_get_parent (mono_class_create_array (mono_defaults.object_class, 1)); - get_method = mono_class_get_method_from_name_checked (array_klass, "GetGenericValueImpl", 2, 0, error); + get_method = mono_class_get_method_from_name_checked (array_klass, "GetGenericValue_icall", 3, 0, error); mono_error_assert_ok (error); if (get_method) { memset (&ctx, 0, sizeof (ctx)); - args [0] = object_type; + MonoType *args [ ] = { object_type }; ctx.method_inst = mono_metadata_get_generic_inst (1, args); add_extra_method (acfg, mono_marshal_get_native_wrapper (mono_class_inflate_generic_method_checked (get_method, &ctx, error), TRUE, TRUE)); mono_error_assert_ok (error); /* FIXME don't swallow the error */ diff --git a/mono/mini/aot-runtime.c b/mono/mini/aot-runtime.c index d71d3400a8e..170ff28dc7c 100644 --- a/mono/mini/aot-runtime.c +++ b/mono/mini/aot-runtime.c @@ -4769,11 +4769,11 @@ mono_aot_get_method (MonoDomain *domain, MonoMethod *method, MonoError *error) } /* - * Special case Array.GetGenericValueImpl which is a generic icall. + * Special case Array.GetGenericValue_icall which is a generic icall. * Generic sharing currently can't handle it, but the icall returns data using * an out parameter, so the managed-to-native wrappers can share the same code. */ - if (method_index == 0xffffff && method->wrapper_type == MONO_WRAPPER_MANAGED_TO_NATIVE && method->klass == mono_defaults.array_class && !strcmp (method->name, "GetGenericValueImpl")) { + if (method_index == 0xffffff && method->wrapper_type == MONO_WRAPPER_MANAGED_TO_NATIVE && method->klass == mono_defaults.array_class && !strcmp (method->name, "GetGenericValue_icall")) { MonoMethod *m; MonoGenericContext ctx; @@ -4781,7 +4781,7 @@ mono_aot_get_method (MonoDomain *domain, MonoMethod *method, MonoError *error) /* Avoid recursion */ return NULL; - m = mono_class_get_method_from_name_checked (mono_defaults.array_class, "GetGenericValueImpl", 2, 0, error); + m = mono_class_get_method_from_name_checked (mono_defaults.array_class, "GetGenericValue_icall", 3, 0, error); mono_error_assert_ok (error); g_assert (m); diff --git a/mono/mini/intrinsics.c b/mono/mini/intrinsics.c index 87117bfc868..a10fa519365 100644 --- a/mono/mini/intrinsics.c +++ b/mono/mini/intrinsics.c @@ -22,12 +22,12 @@ static GENERATE_GET_CLASS_WITH_CACHE (runtime_helpers, "System.Runtime.CompilerServices", "RuntimeHelpers") static GENERATE_TRY_GET_CLASS_WITH_CACHE (math, "System", "Math") -/* optimize the simple GetGenericValueImpl/SetGenericValueImpl generic icalls */ +/* optimize the simple GetGenericValueImpl/SetGenericValueImpl generic calls */ static MonoInst* emit_array_generic_access (MonoCompile *cfg, MonoMethodSignature *fsig, MonoInst **args, int is_set) { MonoInst *addr, *store, *load; - MonoClass *eklass = mono_class_from_mono_type_internal (fsig->params [2]); + MonoClass *eklass = mono_class_from_mono_type_internal (fsig->params [1]); /* the bounds check is already done by the callers */ addr = mini_emit_ldelema_1_ins (cfg, eklass, args [0], args [1], FALSE); diff --git a/netcore/System.Private.CoreLib/src/System/Array.cs b/netcore/System.Private.CoreLib/src/System/Array.cs index e5761a9aa42..05207607e6c 100644 --- a/netcore/System.Private.CoreLib/src/System/Array.cs +++ b/netcore/System.Private.CoreLib/src/System/Array.cs @@ -498,7 +498,7 @@ namespace System // CAUTION! No bounds checking! [MethodImplAttribute (MethodImplOptions.InternalCall)] - extern void GetGenericValueImpl (int pos, out T value); + extern static void GetGenericValue_icall (ref Array self, int pos, out T value); // CAUTION! No bounds checking! [MethodImplAttribute (MethodImplOptions.InternalCall)] @@ -506,7 +506,21 @@ namespace System // CAUTION! No bounds checking! [MethodImplAttribute (MethodImplOptions.InternalCall)] - extern void SetGenericValueImpl (int pos, ref T value); + extern static void SetGenericValue_icall (ref Array self, int pos, ref T value); + + // This is a special case in the runtime. + void GetGenericValueImpl (int pos, out T value) + { + var self = this; + GetGenericValue_icall (ref self, pos, out value); + } + + // This is a special case in the runtime. + void SetGenericValueImpl (int pos, ref T value) + { + var self = this; + SetGenericValue_icall (ref self, pos, ref value); + } // CAUTION! No bounds checking! [MethodImplAttribute (MethodImplOptions.InternalCall)] @@ -569,6 +583,7 @@ namespace System ThrowHelper.ThrowArgumentOutOfRange_IndexException (); T value; + // Do not change this to call GetGenericValue_icall directly, due to special casing in the runtime. GetGenericValueImpl (index, out value); return value; } @@ -599,6 +614,7 @@ namespace System ThrowHelper.ThrowArgumentOutOfRange_IndexException (); T value; + // Do not change this to call GetGenericValue_icall directly, due to special casing in the runtime. GetGenericValueImpl (index, out value); return value; } @@ -613,6 +629,7 @@ namespace System return; } + // Do not change this to call SetGenericValue_icall directly, due to special casing in the runtime. SetGenericValueImpl (index, ref item); } } -- 2.11.4.GIT