Unify Default Comparer logic

src/coreclr/System.Private.CoreLib/src/System/Collections/Generic/ComparerHelpers.cs

@@ -46,44 +46,12 @@ internal static object CreateDefaultComparer(Type type)
             // The comparer for enums is specialized to avoid boxing.
             else if (type.IsEnum)
             {
-                result = TryCreateEnumComparer(runtimeType);
+                result = CreateInstanceForAnotherGenericParameter((RuntimeType)typeof(EnumComparer<>), runtimeType);
             }
 
             return result ?? CreateInstanceForAnotherGenericParameter((RuntimeType)typeof(ObjectComparer<object>), runtimeType);
         }
 
-        /// <summary>
-        /// Creates the default <see cref="Comparer{T}"/> for an enum type.
-        /// </summary>
-        /// <param name="enumType">The enum type to create the default comparer for.</param>
-        private static object? TryCreateEnumComparer(RuntimeType enumType)
-        {
-            Debug.Assert(enumType != null);
-            Debug.Assert(enumType.IsEnum);
-
-            // Explicitly call Enum.GetUnderlyingType here. Although GetTypeCode
-            // ends up doing this anyway, we end up avoiding an unnecessary P/Invoke
-            // and virtual method call.
-            TypeCode underlyingTypeCode = Type.GetTypeCode(Enum.GetUnderlyingType(enumType));
-
-            // Depending on the enum type, we need to special case the comparers so that we avoid boxing.
-            // Specialize differently for signed/unsigned types so we avoid problems with large numbers.
-            switch (underlyingTypeCode)
-            {
-                case TypeCode.SByte:
-                case TypeCode.Int16:
-                case TypeCode.Int32:
-                case TypeCode.Byte:
-                case TypeCode.UInt16:
-                case TypeCode.UInt32:
-                case TypeCode.Int64:
-                case TypeCode.UInt64:
-                    return CreateInstanceForAnotherGenericParameter((RuntimeType)typeof(EnumComparer<>), enumType);
-            }
-
-            return null;
-        }
-
         /// <summary>
         /// Creates the default <see cref="EqualityComparer{T}"/>.
         /// </summary>
@@ -98,15 +66,9 @@ internal static object CreateDefaultEqualityComparer(Type type)
             object? result = null;
             var runtimeType = (RuntimeType)type;
 
-            if (type == typeof(byte))
-            {
-                // Specialize for byte so Array.IndexOf is faster.
-                result = new ByteEqualityComparer();
-            }
-            else if (type == typeof(string))
+            if (type == typeof(string))
             {
-                // Specialize for string, as EqualityComparer<string>.Default is on the startup path
-                result = new GenericEqualityComparer<string>();
+                return new GenericEqualityComparer<string>();
             }
             else if (type.IsAssignableTo(typeof(IEquatable<>).MakeGenericType(type)))
             {
@@ -122,41 +84,10 @@ internal static object CreateDefaultEqualityComparer(Type type)
             else if (type.IsEnum)
             {
                 // The equality comparer for enums is specialized to avoid boxing.
-                result = TryCreateEnumEqualityComparer(runtimeType);
+                result = CreateInstanceForAnotherGenericParameter((RuntimeType)typeof(EnumEqualityComparer<>), runtimeType);
             }
 
             return result ?? CreateInstanceForAnotherGenericParameter((RuntimeType)typeof(ObjectEqualityComparer<object>), runtimeType);
         }
-
-        /// <summary>
-        /// Creates the default <see cref="EqualityComparer{T}"/> for an enum type.
-        /// </summary>
-        /// <param name="enumType">The enum type to create the default equality comparer for.</param>
-        private static object? TryCreateEnumEqualityComparer(RuntimeType enumType)
-        {
-            Debug.Assert(enumType != null);
-            Debug.Assert(enumType.IsEnum);
-
-            // See the METHOD__JIT_HELPERS__UNSAFE_ENUM_CAST and METHOD__JIT_HELPERS__UNSAFE_ENUM_CAST_LONG cases in getILIntrinsicImplementation
-            // for how we cast the enum types to integral values in the comparer without boxing.
-
-            TypeCode underlyingTypeCode = Type.GetTypeCode(Enum.GetUnderlyingType(enumType));
-
-            // Depending on the enum type, we need to special case the comparers so that we avoid boxing.
-            switch (underlyingTypeCode)
-            {
-                case TypeCode.Int32:
-                case TypeCode.UInt32:
-                case TypeCode.SByte:
-                case TypeCode.Byte:
-                case TypeCode.Int16:
-                case TypeCode.Int64:
-                case TypeCode.UInt64:
-                case TypeCode.UInt16:
-                    return CreateInstanceForAnotherGenericParameter((RuntimeType)typeof(EnumEqualityComparer<>), enumType);
-            }
-
-            return null;
-        }
     }
 }

src/coreclr/jit/importercalls.cpp

@@ -7553,57 +7553,40 @@ CORINFO_CLASS_HANDLE Compiler::impGetSpecialIntrinsicExactReturnType(GenTreeCall
             CORINFO_SIG_INFO sig;
             info.compCompHnd->getMethodSig(methodHnd, &sig);
             assert(sig.sigInst.classInstCount == 1);
+
             CORINFO_CLASS_HANDLE typeHnd = sig.sigInst.classInst[0];
             assert(typeHnd != nullptr);
 
-            // Lookup can incorrect when we have __Canon as it won't appear
-            // to implement any interface types.
-            //
-            // And if we do not have a final type, devirt & inlining is
-            // unlikely to result in much simplification.
-            //
-            // We can use CORINFO_FLG_FINAL to screen out both of these cases.
-            const DWORD typeAttribs = info.compCompHnd->getClassAttribs(typeHnd);
-            bool        isFinalType = ((typeAttribs & CORINFO_FLG_FINAL) != 0);
-
-            if (!isFinalType)
+            CallArg* instParam = call->gtArgs.FindWellKnownArg(WellKnownArg::InstParam);
+            if (instParam != nullptr)
             {
-                CallArg* instParam = call->gtArgs.FindWellKnownArg(WellKnownArg::InstParam);
-                if (instParam != nullptr)
+                assert(instParam->GetNext() == nullptr);
+                CORINFO_CLASS_HANDLE hClass = gtGetHelperArgClassHandle(instParam->GetNode());
+                if (hClass != NO_CLASS_HANDLE)
                 {
-                    assert(instParam->GetNext() == nullptr);
-                    CORINFO_CLASS_HANDLE hClass = gtGetHelperArgClassHandle(instParam->GetNode());
-                    if (hClass != NO_CLASS_HANDLE)
-                    {
-                        hClass = getTypeInstantiationArgument(hClass, 0);
-                        if ((info.compCompHnd->getClassAttribs(hClass) & CORINFO_FLG_FINAL) != 0)
-                        {
-                            typeHnd     = hClass;
-                            isFinalType = true;
-                        }
-                    }
+                    typeHnd = getTypeInstantiationArgument(hClass, 0);
                 }
             }
 
-            if (isFinalType)
+            if (ni == NI_System_Collections_Generic_EqualityComparer_get_Default)
+            {
+                result = info.compCompHnd->getDefaultEqualityComparerClass(typeHnd);
+            }
+            else
+            {
+                assert(ni == NI_System_Collections_Generic_Comparer_get_Default);
+                result = info.compCompHnd->getDefaultComparerClass(typeHnd);
+            }
+
+            if (result != NO_CLASS_HANDLE)
             {
-                if (ni == NI_System_Collections_Generic_EqualityComparer_get_Default)
-                {
-                    result = info.compCompHnd->getDefaultEqualityComparerClass(typeHnd);
-                }
-                else
-                {
-                    assert(ni == NI_System_Collections_Generic_Comparer_get_Default);
-                    result = info.compCompHnd->getDefaultComparerClass(typeHnd);
-                }
                 JITDUMP("Special intrinsic for type %s: return type is %s\n", eeGetClassName(typeHnd),
                         result != nullptr ? eeGetClassName(result) : "unknown");
             }
             else
             {
-                JITDUMP("Special intrinsic for type %s: type not final, so deferring opt\n", eeGetClassName(typeHnd));
+                JITDUMP("Special intrinsic for type %s: type shared, so deferring opt\n", eeGetClassName(typeHnd));
             }
-
             break;
         }
 

src/coreclr/vm/corelib.h

@@ -1155,7 +1155,6 @@ DEFINE_METHOD(UTF8BUFFERMARSHALER, CONVERT_TO_MANAGED, ConvertToManaged, NoSig)
 
 // Classes referenced in EqualityComparer<T>.Default optimization
 
-DEFINE_CLASS(BYTE_EQUALITYCOMPARER, CollectionsGeneric, ByteEqualityComparer)
 DEFINE_CLASS(ENUM_EQUALITYCOMPARER, CollectionsGeneric, EnumEqualityComparer`1)
 DEFINE_CLASS(NULLABLE_EQUALITYCOMPARER, CollectionsGeneric, NullableEqualityComparer`1)
 DEFINE_CLASS(GENERIC_EQUALITYCOMPARER, CollectionsGeneric, GenericEqualityComparer`1)

src/coreclr/vm/jitinterface.cpp

@@ -8786,6 +8786,11 @@ CORINFO_CLASS_HANDLE CEEInfo::getDefaultComparerClassHelper(CORINFO_CLASS_HANDLE
 
     TypeHandle elemTypeHnd(elemType);
 
+    if (elemTypeHnd == TypeHandle(g_pCanonMethodTableClass))
+    {
+        return NULL;
+    }
+
     // Mirrors the logic in BCL's CompareHelpers.CreateDefaultComparer
     // And in compile.cpp's SpecializeComparer
     //
@@ -8803,44 +8808,15 @@ CORINFO_CLASS_HANDLE CEEInfo::getDefaultComparerClassHelper(CORINFO_CLASS_HANDLE
     if (Nullable::IsNullableType(elemTypeHnd))
     {
         Instantiation nullableInst = elemTypeHnd.AsMethodTable()->GetInstantiation();
-        TypeHandle iequatable = TypeHandle(CoreLibBinder::GetClass(CLASS__IEQUATABLEGENERIC)).Instantiate(nullableInst);
-        if (nullableInst[0].CanCastTo(iequatable))
-        {
-            TypeHandle resultTh = ((TypeHandle)CoreLibBinder::GetClass(CLASS__NULLABLE_COMPARER)).Instantiate(nullableInst);
-            return CORINFO_CLASS_HANDLE(resultTh.GetMethodTable());
-        }
+        TypeHandle resultTh = ((TypeHandle)CoreLibBinder::GetClass(CLASS__NULLABLE_COMPARER)).Instantiate(nullableInst);
+        return CORINFO_CLASS_HANDLE(resultTh.GetMethodTable());
     }
 
     // We need to special case the Enum comparers based on their underlying type to avoid boxing
     if (elemTypeHnd.IsEnum())
     {
-        MethodTable* targetClass = NULL;
-        CorElementType normType = elemTypeHnd.GetVerifierCorElementType();
-
-        switch(normType)
-        {
-            case ELEMENT_TYPE_I1:
-            case ELEMENT_TYPE_I2:
-            case ELEMENT_TYPE_U1:
-            case ELEMENT_TYPE_U2:
-            case ELEMENT_TYPE_I4:
-            case ELEMENT_TYPE_U4:
-            case ELEMENT_TYPE_I8:
-            case ELEMENT_TYPE_U8:
-            {
-                targetClass = CoreLibBinder::GetClass(CLASS__ENUM_COMPARER);
-                break;
-            }
-
-            default:
-                break;
-        }
-
-        if (targetClass != NULL)
-        {
-            TypeHandle resultTh = ((TypeHandle)targetClass->GetCanonicalMethodTable()).Instantiate(inst);
-            return CORINFO_CLASS_HANDLE(resultTh.GetMethodTable());
-        }
+        TypeHandle resultTh = ((TypeHandle)CoreLibBinder::GetClass(CLASS__ENUM_COMPARER)).Instantiate(inst);
+        return CORINFO_CLASS_HANDLE(resultTh.GetMethodTable());
     }
 
     // Default case
@@ -8881,10 +8857,9 @@ CORINFO_CLASS_HANDLE CEEInfo::getDefaultEqualityComparerClassHelper(CORINFO_CLAS
     // And in compile.cpp's SpecializeEqualityComparer
     TypeHandle elemTypeHnd(elemType);
 
-    // Special case for byte
-    if (elemTypeHnd.AsMethodTable()->HasSameTypeDefAs(CoreLibBinder::GetClass(CLASS__ELEMENT_TYPE_U1)))
+    if (elemTypeHnd == TypeHandle(g_pCanonMethodTableClass))
     {
-        return CORINFO_CLASS_HANDLE(CoreLibBinder::GetClass(CLASS__BYTE_EQUALITYCOMPARER));
+        return NULL;
     }
 
     // Mirrors the logic in BCL's CompareHelpers.CreateDefaultComparer
@@ -8914,33 +8889,8 @@ CORINFO_CLASS_HANDLE CEEInfo::getDefaultEqualityComparerClassHelper(CORINFO_CLAS
     // to avoid boxing and call the correct versions of GetHashCode.
     if (elemTypeHnd.IsEnum())
     {
-        MethodTable* targetClass = NULL;
-        CorElementType normType = elemTypeHnd.GetVerifierCorElementType();
-
-        switch(normType)
-        {
-            case ELEMENT_TYPE_I1:
-            case ELEMENT_TYPE_I2:
-            case ELEMENT_TYPE_U1:
-            case ELEMENT_TYPE_U2:
-            case ELEMENT_TYPE_I4:
-            case ELEMENT_TYPE_U4:
-            case ELEMENT_TYPE_I8:
-            case ELEMENT_TYPE_U8:
-            {
-                targetClass = CoreLibBinder::GetClass(CLASS__ENUM_EQUALITYCOMPARER);
-                break;
-            }
-
-            default:
-                break;
-        }
-
-        if (targetClass != NULL)
-        {
-            TypeHandle resultTh = ((TypeHandle)targetClass->GetCanonicalMethodTable()).Instantiate(inst);
-            return CORINFO_CLASS_HANDLE(resultTh.GetMethodTable());
-        }
+        TypeHandle resultTh = ((TypeHandle)CoreLibBinder::GetClass(CLASS__ENUM_EQUALITYCOMPARER)).Instantiate(inst);
+        return CORINFO_CLASS_HANDLE(resultTh.GetMethodTable());
     }
 
     // Default case

src/mono/System.Private.CoreLib/src/System/Collections/Generic/EqualityComparer.Mono.cs

@@ -35,11 +35,7 @@ private static EqualityComparer<T> CreateComparer()
             // IN mini_handle_call_res_devirt
             /////////////////////////////////////////////////
 
-            if (t == typeof(byte))
-            {
-                return (EqualityComparer<T>)(object)(new ByteEqualityComparer());
-            }
-            else if (t == typeof(string))
+            if (t == typeof(string))
             {
                 // Specialize for string, as EqualityComparer<string>.Default is on the startup path
                 return (EqualityComparer<T>)(object)(new GenericEqualityComparer<string>());

src/mono/System.Private.CoreLib/src/System/Runtime/CompilerServices/JitHelpers.cs

@@ -5,13 +5,11 @@ namespace System.Runtime.CompilerServices
 {
     internal static class JitHelpers
     {
-#pragma warning disable IDE0060
         [Intrinsic]
-        public static bool EnumEquals<T>(T x, T y) where T : struct, Enum => throw new NotImplementedException();
+        public static bool EnumEquals<T>(T x, T y) where T : struct, Enum => x.Equals(y);
 
         [Intrinsic]
-        public static int EnumCompareTo<T>(T x, T y) where T : struct, Enum => throw new NotImplementedException();
-#pragma warning restore IDE0060
+        public static int EnumCompareTo<T>(T x, T y) where T : struct, Enum => x.CompareTo(y);
 
         [Intrinsic]
         internal static void DisableInline () => throw new NotImplementedException();

src/mono/mono/mini/interp/transform.c

@@ -2536,7 +2536,10 @@ interp_handle_intrinsics (TransformData *td, MonoMethod *target_method, MonoClas
 		MonoType *t = ctx->method_inst->type_argv [0];
 		t = mini_get_underlying_type (t);
 
-		gboolean is_i8 = (t->type == MONO_TYPE_I8 || t->type == MONO_TYPE_U8);
+		if (t->type == MONO_TYPE_R4 || t->type == MONO_TYPE_R8)
+			return FALSE;
+
+		gboolean is_i8 = (t->type == MONO_TYPE_I8 || t->type == MONO_TYPE_U8 || (TARGET_SIZEOF_VOID_P == 8 && (t->type == MONO_TYPE_I || t->type == MONO_TYPE_U)));
 		gboolean is_unsigned = (t->type == MONO_TYPE_U1 || t->type == MONO_TYPE_U2 || t->type == MONO_TYPE_U4 || t->type == MONO_TYPE_U8 || t->type == MONO_TYPE_U);
 
 		gboolean is_compareto = strcmp (tm, "EnumCompareTo") == 0;

src/mono/mono/mini/intrinsics.c

@@ -672,10 +672,10 @@ emit_jit_helpers_intrinsics (MonoCompile *cfg, MonoMethod *cmethod, MonoMethodSi
 
 		t = ctx->method_inst->type_argv [0];
 		t = mini_get_underlying_type (t);
-		if (mini_is_gsharedvt_variable_type (t))
+		if (mini_is_gsharedvt_variable_type (t) || t->type == MONO_TYPE_R4 || t->type == MONO_TYPE_R8)
 			return NULL;
 
-		gboolean is_i8 = (t->type == MONO_TYPE_I8 || t->type == MONO_TYPE_U8);
+		gboolean is_i8 = (t->type == MONO_TYPE_I8 || t->type == MONO_TYPE_U8 || (TARGET_SIZEOF_VOID_P == 8 && (t->type == MONO_TYPE_I || t->type == MONO_TYPE_U)));
 		gboolean is_unsigned = (t->type == MONO_TYPE_U1 || t->type == MONO_TYPE_U2 || t->type == MONO_TYPE_U4 || t->type == MONO_TYPE_U8 || t->type == MONO_TYPE_U);
 		int cmp_op, ceq_op, cgt_op, clt_op;
 

src/mono/mono/mini/mini.c

@@ -4301,7 +4301,7 @@ mini_handle_call_res_devirt (MonoMethod *cmethod)
 
 		// EqualityComparer<T>.Default returns specific types depending on T
 		// FIXME: Special case more types: byte, string, nullable, enum ?
-		if (mono_class_is_assignable_from_internal (inst, mono_class_from_mono_type_internal (param_type)) && param_type->type != MONO_TYPE_U1 && param_type->type != MONO_TYPE_STRING) {
+		if (mono_class_is_assignable_from_internal (inst, mono_class_from_mono_type_internal (param_type)) && param_type->type != MONO_TYPE_STRING) {
 			MonoClass *gcomparer_inst;
 
 			memset (&ctx, 0, sizeof (ctx));