Construct TypedReference on top of ByReference (#2216)
authorMichal Strehovský <MichalStrehovsky@users.noreply.github.com>
Wed, 29 Jan 2020 13:06:11 +0000 (14:06 +0100)
committerGitHub <noreply@github.com>
Wed, 29 Jan 2020 13:06:11 +0000 (14:06 +0100)
CoreCLR and Crossgen2 need to special case TypedReference because it's not a regular byref-like type (constructed on top of `ByReference<T>` fields). This gets rid of the special casing.

src/coreclr/src/System.Private.CoreLib/src/System/TypedReference.cs
src/coreclr/src/inc/corinfo.h
src/coreclr/src/tools/Common/JitInterface/CorInfoImpl.cs
src/coreclr/src/tools/Common/JitInterface/CorInfoTypes.cs
src/coreclr/src/tools/Common/JitInterface/SystemVStructClassificator.cs
src/coreclr/src/vm/appdomain.cpp
src/coreclr/src/vm/jitinterface.cpp
src/coreclr/src/vm/methodtable.cpp
src/coreclr/src/vm/methodtable.h
src/coreclr/src/vm/object.inl

index 206665b..f02a4c2 100644 (file)
@@ -7,6 +7,7 @@
 
 using System.Reflection;
 using System.Runtime.CompilerServices;
+using Internal.Runtime.CompilerServices;
 
 namespace System
 {
@@ -14,8 +15,8 @@ namespace System
     [System.Runtime.Versioning.NonVersionable] // This only applies to field layout
     public ref struct TypedReference
     {
-        private IntPtr Value;
-        private IntPtr Type;
+        private readonly ByReference<byte> _value;
+        private readonly IntPtr _type;
 
         [CLSCompliant(false)]
         public static TypedReference MakeTypedReference(object target, FieldInfo[] flds)
@@ -70,7 +71,7 @@ namespace System
 
         public override int GetHashCode()
         {
-            if (Type == IntPtr.Zero)
+            if (_type == IntPtr.Zero)
                 return 0;
             else
                 return __reftype(this).GetHashCode();
@@ -89,7 +90,7 @@ namespace System
         [MethodImpl(MethodImplOptions.InternalCall)]
         internal static extern unsafe object InternalToObject(void* value);
 
-        internal bool IsNull => Value == IntPtr.Zero && Type == IntPtr.Zero;
+        internal bool IsNull => Unsafe.IsNullRef(ref _value.Value) && _type == IntPtr.Zero;
 
         public static Type GetTargetType(TypedReference value)
         {
index 97e09a0..43891ef 100644 (file)
@@ -252,20 +252,6 @@ enum SystemVClassificationType : unsigned __int8
     // SystemVClassificationTypeX87             = Unused, // Not supported by the CLR.
     // SystemVClassificationTypeX87Up           = Unused, // Not supported by the CLR.
     // SystemVClassificationTypeComplexX87      = Unused, // Not supported by the CLR.
-
-    // Internal flags - never returned outside of the classification implementation.
-
-    // This value represents a very special type with two eightbytes.
-    // First ByRef, second Integer (platform int).
-    // The VM has a special Elem type for this type - ELEMENT_TYPE_TYPEDBYREF.
-    // This is the classification counterpart for that element type. It is used to detect
-    // the special TypedReference type and specialize its classification.
-    // This type is represented as a struct with two fields. The classification needs to do
-    // special handling of it since the source/methadata type of the fields is IntPtr.
-    // The VM changes the first to ByRef. The second is left as IntPtr (TYP_I_IMPL really). The classification needs to match this and
-    // special handling is warranted (similar thing is done in the getGCLayout function for this type).
-    SystemVClassificationTypeTypedReference     = 8,
-    SystemVClassificationTypeMAX                = 9,
 };
 
 // Represents classification information for a struct.
index c8ff1a8..47b229a 100644 (file)
@@ -1400,7 +1400,7 @@ namespace Internal.JitInterface
         {
             int result = 0;
 
-            if (type.IsByReferenceOfT || type.IsWellKnownType(WellKnownType.TypedReference))
+            if (type.IsByReferenceOfT)
             {
                 return MarkGcField(gcPtrs, CorInfoGCType.TYPE_GC_BYREF);
             }
index 49627ca..cd2427f 100644 (file)
@@ -1150,20 +1150,6 @@ namespace Internal.JitInterface
         // SystemVClassificationTypeX87             = Unused, // Not supported by the CLR.
         // SystemVClassificationTypeX87Up           = Unused, // Not supported by the CLR.
         // SystemVClassificationTypeComplexX87      = Unused, // Not supported by the CLR.
-
-        // Internal flags - never returned outside of the classification implementation.
-
-        // This value represents a very special type with two eightbytes.
-        // First ByRef, second Integer (platform int).
-        // The VM has a special Elem type for this type - ELEMENT_TYPE_TYPEDBYREF.
-        // This is the classification counterpart for that element type. It is used to detect
-        // the special TypedReference type and specialize its classification.
-        // This type is represented as a struct with two fields. The classification needs to do
-        // special handling of it since the source/methadata type of the fieds is IntPtr.
-        // The VM changes the first to ByRef. The second is left as IntPtr (TYP_I_IMPL really). The classification needs to match this and
-        // special handling is warranted (similar thing is done in the getGCLayout function for this type).
-        SystemVClassificationTypeTypedReference     = 8,
-        SystemVClassificationTypeMAX                = 9
     };
 
     public struct SYSTEMV_AMD64_CORINFO_STRUCT_REG_PASSING_DESCRIPTOR
index 1ab5064..2c54f3a 100644 (file)
@@ -99,8 +99,7 @@ namespace Internal.JitInterface
             int typeSize = typeDesc.GetElementSize().AsInt;
             if (typeDesc.IsValueType && (typeSize <= CLR_SYSTEMV_MAX_STRUCT_BYTES_TO_PASS_IN_REGISTERS))
             {
-                if ((TypeDef2SystemVClassification(typeDesc) != SystemVClassificationTypeStruct) &&
-                    (TypeDef2SystemVClassification(typeDesc) != SystemVClassificationTypeTypedReference))
+                if (TypeDef2SystemVClassification(typeDesc) != SystemVClassificationTypeStruct)
                 {
                     return;
                 }
@@ -126,12 +125,6 @@ namespace Internal.JitInterface
 
         private static SystemVClassificationType TypeDef2SystemVClassification(TypeDesc typeDesc)
         {
-            if (typeDesc.IsWellKnownType(WellKnownType.TypedReference))
-            {
-                // There is no category representing typed reference
-                return SystemVClassificationTypeTypedReference;
-            }
-
             switch (typeDesc.Category)
             {
                 case TypeFlags.Boolean:
@@ -336,47 +329,6 @@ namespace Internal.JitInterface
                     continue;
                 }
 
-                if (fieldClassificationType == SystemVClassificationTypeTypedReference || 
-                    TypeDef2SystemVClassification(typeDesc) == SystemVClassificationTypeTypedReference)
-                {
-                    // The TypedReference is a very special type.
-                    // In source/metadata it has two fields - Type and Value and both are defined of type IntPtr.
-                    // When the VM creates a layout of the type it changes the type of the Value to ByRef type and the
-                    // type of the Type field is left to IntPtr (TYPE_I internally - native int type.)
-                    // This requires a special treatment of this type. The code below handles the both fields (and this entire type).
-
-                    for (int i = 0; i < 2; i++)
-                    {
-                        fieldSize = 8;
-                        fieldOffset = (i == 0 ? 0 : 8);
-                        normalizedFieldOffset = fieldOffset + startOffsetOfStruct;
-                        fieldClassificationType = (i == 0 ? SystemVClassificationTypeIntegerByRef : SystemVClassificationTypeInteger);
-                        if ((normalizedFieldOffset % fieldSize) != 0)
-                        {
-                            // The spec requires that struct values on the stack from register passed fields expects
-                            // those fields to be at their natural alignment.
-                            return false;
-                        }
-
-                        helper.LargestFieldOffset = (int)normalizedFieldOffset;
-
-                        // Set the data for a new field.
-
-                        // The new field classification must not have been initialized yet.
-                        Debug.Assert(helper.FieldClassifications[helper.CurrentUniqueOffsetField] == SystemVClassificationTypeNoClass);
-
-                        helper.FieldClassifications[helper.CurrentUniqueOffsetField] = fieldClassificationType;
-                        helper.FieldSizes[helper.CurrentUniqueOffsetField] = fieldSize;
-                        helper.FieldOffsets[helper.CurrentUniqueOffsetField] = normalizedFieldOffset;
-
-                        helper.CurrentUniqueOffsetField++;
-                    }
-
-                    // Both fields of the special TypedReference struct are handled.
-                    // Done classifying the System.TypedReference struct fields.
-                    break;
-                }
-
                 if ((normalizedFieldOffset % fieldSize) != 0)
                 {
                     // The spec requires that struct values on the stack from register passed fields expects
index f3e7505..4fb2593 100644 (file)
@@ -1975,8 +1975,6 @@ void SystemDomain::LoadBaseSystemClasses()
     // We have delayed allocation of mscorlib's static handles until we load the object class
     MscorlibBinder::GetModule()->AllocateRegularStaticHandles(DefaultDomain());
 
-    g_TypedReferenceMT = MscorlibBinder::GetClass(CLASS__TYPED_REFERENCE);
-
     // Make sure all primitive types are loaded
     for (int et = ELEMENT_TYPE_VOID; et <= ELEMENT_TYPE_R8; et++)
         MscorlibBinder::LoadPrimitiveType((CorElementType)et);
@@ -1984,6 +1982,8 @@ void SystemDomain::LoadBaseSystemClasses()
     MscorlibBinder::LoadPrimitiveType(ELEMENT_TYPE_I);
     MscorlibBinder::LoadPrimitiveType(ELEMENT_TYPE_U);
 
+    g_TypedReferenceMT = MscorlibBinder::GetClass(CLASS__TYPED_REFERENCE);
+
     // further loading of nonprimitive types may need casting support.
     // initialize cast cache here.
 #ifndef CROSSGEN_COMPILE
index 725ac62..6febb1f 100644 (file)
@@ -2153,9 +2153,7 @@ static unsigned ComputeGCLayout(MethodTable * pMT, BYTE* gcPtrs)
 
     _ASSERTE(pMT->IsValueType());
 
-    // TODO: TypedReference should ideally be implemented as a by-ref-like struct containing a ByReference<T> field, in which
-    // case the check for g_TypedReferenceMT below would not be necessary
-    if (pMT == g_TypedReferenceMT || pMT->HasSameTypeDefAs(g_pByReferenceClass))
+    if (pMT->HasSameTypeDefAs(g_pByReferenceClass))
     {
         if (gcPtrs[0] == TYPE_GC_NONE)
         {
@@ -2223,23 +2221,10 @@ unsigned CEEInfo::getClassGClayout (CORINFO_CLASS_HANDLE clsHnd, BYTE* gcPtrs)
     }
     else if (pMT->IsByRefLike())
     {
-        // TODO: TypedReference should ideally be implemented as a by-ref-like struct containing a ByReference<T> field, in
-        // which case the check for g_TypedReferenceMT below would not be necessary
-        if (pMT == g_TypedReferenceMT)
-        {
-            gcPtrs[0] = TYPE_GC_BYREF;
-            gcPtrs[1] = TYPE_GC_NONE;
-            result = 1;
-        }
-        else
-        {
-            memset(gcPtrs, TYPE_GC_NONE,
-                (VMClsHnd.GetSize() + TARGET_POINTER_SIZE - 1) / TARGET_POINTER_SIZE);
-            // Note: This case is more complicated than the TypedReference case
-            // due to ByRefLike structs being included as fields in other value
-            // types (TypedReference can not be.)
-            result = ComputeGCLayout(VMClsHnd.AsMethodTable(), gcPtrs);
-        }
+        memset(gcPtrs, TYPE_GC_NONE,
+            (VMClsHnd.GetSize() + TARGET_POINTER_SIZE - 1) / TARGET_POINTER_SIZE);
+        // ByRefLike structs can be included as fields in other value types.
+        result = ComputeGCLayout(VMClsHnd.AsMethodTable(), gcPtrs);
     }
     else
     {
@@ -2309,8 +2294,7 @@ bool CEEInfo::getSystemVAmd64PassStructInRegisterDescriptor(
     // Make sure this is a value type.
     if (th.IsValueType())
     {
-        _ASSERTE((CorInfoType2UnixAmd64Classification(th.GetInternalCorElementType()) == SystemVClassificationTypeStruct) ||
-                 (CorInfoType2UnixAmd64Classification(th.GetInternalCorElementType()) == SystemVClassificationTypeTypedReference));
+        _ASSERTE(CorInfoType2UnixAmd64Classification(th.GetInternalCorElementType()) == SystemVClassificationTypeStruct);
 
         // The useNativeLayout in this case tracks whether the classification
         // is for a native layout of the struct or not.
index 6cad413..57c6938 100644 (file)
@@ -2206,7 +2206,6 @@ const char* GetSystemVClassificationTypeName(SystemVClassificationType t)
     case SystemVClassificationTypeIntegerReference:     return "IntegerReference";
     case SystemVClassificationTypeIntegerByRef:         return "IntegerByReference";
     case SystemVClassificationTypeSSE:                  return "SSE";
-    case SystemVClassificationTypeTypedReference:       return "TypedReference";
     default:                                            return "ERROR";
     }
 };
@@ -2438,64 +2437,6 @@ bool MethodTable::ClassifyEightBytesWithManagedLayout(SystemVStructRegisterPassi
             continue;
         }
 
-        if (fieldClassificationType == SystemVClassificationTypeTypedReference ||
-            CorInfoType2UnixAmd64Classification(GetClass_NoLogging()->GetInternalCorElementType()) == SystemVClassificationTypeTypedReference)
-        {
-            // The TypedReference is a very special type.
-            // In source/metadata it has two fields - Type and Value and both are defined of type IntPtr.
-            // When the VM creates a layout of the type it changes the type of the Value to ByRef type and the
-            // type of the Type field is left to IntPtr (TYPE_I internally - native int type.)
-            // This requires a special treatment of this type. The code below handles the both fields (and this entire type).
-
-            for (unsigned i = 0; i < 2; i++)
-            {
-                fieldSize = 8;
-                fieldOffset = (i == 0 ? 0 : 8);
-                normalizedFieldOffset = fieldOffset + startOffsetOfStruct;
-                fieldClassificationType = (i == 0 ? SystemVClassificationTypeIntegerByRef : SystemVClassificationTypeInteger);
-                if ((normalizedFieldOffset % fieldSize) != 0)
-                {
-                    // The spec requires that struct values on the stack from register passed fields expects
-                    // those fields to be at their natural alignment.
-
-                    LOG((LF_JIT, LL_EVERYTHING, "     %*sxxxx Field %d %s: offset %d (normalized %d), size %d not at natural alignment; not enregistering struct\n",
-                        nestingLevel * 5, "", fieldIndex, (i == 0 ? "Value" : "Type"), fieldOffset, normalizedFieldOffset, fieldSize));
-                    return false;
-                }
-
-                helperPtr->largestFieldOffset = (int)normalizedFieldOffset;
-
-                // Set the data for a new field.
-
-                // The new field classification must not have been initialized yet.
-                _ASSERTE(helperPtr->fieldClassifications[helperPtr->currentUniqueOffsetField] == SystemVClassificationTypeNoClass);
-
-                // There are only a few field classifications that are allowed.
-                _ASSERTE((fieldClassificationType == SystemVClassificationTypeInteger) ||
-                    (fieldClassificationType == SystemVClassificationTypeIntegerReference) ||
-                    (fieldClassificationType == SystemVClassificationTypeIntegerByRef) ||
-                    (fieldClassificationType == SystemVClassificationTypeSSE));
-
-                helperPtr->fieldClassifications[helperPtr->currentUniqueOffsetField] = fieldClassificationType;
-                helperPtr->fieldSizes[helperPtr->currentUniqueOffsetField] = fieldSize;
-                helperPtr->fieldOffsets[helperPtr->currentUniqueOffsetField] = normalizedFieldOffset;
-
-                LOG((LF_JIT, LL_EVERYTHING, "     %*s**** Field %d %s: offset %d (normalized %d), size %d, currentUniqueOffsetField %d, field type classification %s, chosen field classification %s\n",
-                    nestingLevel * 5, "", fieldIndex, (i == 0 ? "Value" : "Type"), fieldOffset, normalizedFieldOffset, fieldSize, helperPtr->currentUniqueOffsetField,
-                    GetSystemVClassificationTypeName(fieldClassificationType),
-                    GetSystemVClassificationTypeName(helperPtr->fieldClassifications[helperPtr->currentUniqueOffsetField])));
-
-                helperPtr->currentUniqueOffsetField++;
-#ifdef _DEBUG
-                ++fieldIndex;
-#endif // _DEBUG
-            }
-
-            // Both fields of the special TypedReference struct are handled.
-            // Done classifying the System.TypedReference struct fields.
-            break;
-        }
-
         if ((normalizedFieldOffset % fieldSize) != 0)
         {
             // The spec requires that struct values on the stack from register passed fields expects
index 434d4db..045abe0 100644 (file)
@@ -520,7 +520,7 @@ SystemVClassificationType CorInfoType2UnixAmd64Classification(CorElementType eeT
         SystemVClassificationTypeIntegerReference,      // ELEMENT_TYPE_VAR (type variable)
         SystemVClassificationTypeIntegerReference,      // ELEMENT_TYPE_ARRAY
         SystemVClassificationTypeIntegerReference,      // ELEMENT_TYPE_GENERICINST
-        SystemVClassificationTypeTypedReference,        // ELEMENT_TYPE_TYPEDBYREF
+        SystemVClassificationTypeStruct,                // ELEMENT_TYPE_TYPEDBYREF
         SystemVClassificationTypeUnknown,               // ELEMENT_TYPE_VALUEARRAY_UNSUPPORTED
         SystemVClassificationTypeInteger,               // ELEMENT_TYPE_I
         SystemVClassificationTypeInteger,               // ELEMENT_TYPE_U
@@ -543,7 +543,7 @@ SystemVClassificationType CorInfoType2UnixAmd64Classification(CorElementType eeT
     _ASSERTE((SystemVClassificationType)toSystemVAmd64ClassificationTypeMap[ELEMENT_TYPE_I4] == SystemVClassificationTypeInteger);
     _ASSERTE((SystemVClassificationType)toSystemVAmd64ClassificationTypeMap[ELEMENT_TYPE_PTR] == SystemVClassificationTypeInteger);
     _ASSERTE((SystemVClassificationType)toSystemVAmd64ClassificationTypeMap[ELEMENT_TYPE_VALUETYPE] == SystemVClassificationTypeStruct);
-    _ASSERTE((SystemVClassificationType)toSystemVAmd64ClassificationTypeMap[ELEMENT_TYPE_TYPEDBYREF] == SystemVClassificationTypeTypedReference);
+    _ASSERTE((SystemVClassificationType)toSystemVAmd64ClassificationTypeMap[ELEMENT_TYPE_TYPEDBYREF] == SystemVClassificationTypeStruct);
     _ASSERTE((SystemVClassificationType)toSystemVAmd64ClassificationTypeMap[ELEMENT_TYPE_BYREF] == SystemVClassificationTypeIntegerByRef);
 
     return (((unsigned)eeType) < ELEMENT_TYPE_MAX) ? (toSystemVAmd64ClassificationTypeMap[(unsigned)eeType]) : SystemVClassificationTypeUnknown;
index 42d1682..8ed202d 100644 (file)
@@ -314,9 +314,7 @@ inline void FindByRefPointerOffsetsInByRefLikeObject(PTR_MethodTable pMT, SIZE_T
     _ASSERTE(pMT != nullptr);
     _ASSERTE(pMT->IsByRefLike());
 
-    // TODO: TypedReference should ideally be implemented as a by-ref-like struct containing a ByReference<T> field,
-    // in which case the check for g_TypedReferenceMT below would not be necessary
-    if (pMT == g_TypedReferenceMT || pMT->HasSameTypeDefAs(g_pByReferenceClass))
+    if (pMT->HasSameTypeDefAs(g_pByReferenceClass))
     {
         processPointerOffset(baseOffset);
         return;