Fix struct ReturnKind on SysV AMD64.

src/coreclr/jit/gcencode.cpp

@@ -21,6 +21,8 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
 #include "gcinfotypes.h"
 #include "patchpointinfo.h"
 
+#ifdef JIT32_GCENCODER
+
 ReturnKind VarTypeToReturnKind(var_types type)
 {
     switch (type)
@@ -29,40 +31,38 @@ ReturnKind VarTypeToReturnKind(var_types type)
             return RT_Object;
         case TYP_BYREF:
             return RT_ByRef;
-#ifdef TARGET_X86
         case TYP_FLOAT:
         case TYP_DOUBLE:
             return RT_Float;
-#endif // TARGET_X86
         default:
             return RT_Scalar;
     }
 }
 
 ReturnKind GCInfo::getReturnKind()
 {
-    // Note the GCInfo representation only supports structs with up to 2 GC pointers.
+    // Note the JIT32 GCInfo representation only supports structs with 1 GC pointer.
     ReturnTypeDesc retTypeDesc = compiler->compRetTypeDesc;
     const unsigned regCount    = retTypeDesc.GetReturnRegCount();
 
-    switch (regCount)
+    if (regCount == 1)
+    {
+        return VarTypeToReturnKind(retTypeDesc.GetReturnRegType(0));
+    }
+    else
     {
-        case 1:
-            return VarTypeToReturnKind(retTypeDesc.GetReturnRegType(0));
-        case 2:
-            return GetStructReturnKind(VarTypeToReturnKind(retTypeDesc.GetReturnRegType(0)),
-                                       VarTypeToReturnKind(retTypeDesc.GetReturnRegType(1)));
-        default:
 #ifdef DEBUG
-            for (unsigned i = 0; i < regCount; i++)
-            {
-                assert(!varTypeIsGC(retTypeDesc.GetReturnRegType(i)));
-            }
+        for (unsigned i = 0; i < regCount; i++)
+        {
+            assert(!varTypeIsGC(retTypeDesc.GetReturnRegType(i)));
+        }
 #endif // DEBUG
-            return RT_Scalar;
+        return RT_Scalar;
     }
 }
 
+#endif // JIT32_GCENCODER
+
 // gcMarkFilterVarsPinned - Walk all lifetimes and make it so that anything
 //     live in a filter is marked as pinned (often by splitting the lifetime
 //     so that *only* the filter region is pinned).  This should only be

src/coreclr/jit/jitgcinfo.h

@@ -365,6 +365,8 @@ class GCInfo
 
 private:
     static size_t gcRecordEpilog(void* pCallBackData, unsigned offset);
+
+    ReturnKind getReturnKind();
 #else // JIT32_GCENCODER
     void gcInfoBlockHdrSave(GcInfoEncoder* gcInfoEncoder, unsigned methodSize, unsigned prologSize);
 
@@ -398,9 +400,6 @@ class GCInfo
 public:
     // This method updates the appropriate reg masks when a variable is moved.
     void gcUpdateForRegVarMove(regMaskTP srcMask, regMaskTP dstMask, LclVarDsc* varDsc);
-
-private:
-    ReturnKind getReturnKind();
 };
 
 inline unsigned char encodeUnsigned(BYTE* dest, unsigned value)

src/coreclr/vm/CMakeLists.txt

@@ -307,7 +307,6 @@ set(VM_SOURCES_WKS
     cachelinealloc.cpp
     callconvbuilder.cpp
     callhelpers.cpp
-    callsiteinspect.cpp
     callstubgenerator.cpp
     clrconfignative.cpp
     clrex.cpp
@@ -589,6 +588,7 @@ if(CLR_CMAKE_TARGET_WIN32)
 
     # COM interop scenarios
     list(APPEND VM_SOURCES_WKS
+        callsiteinspect.cpp
         classcompat.cpp
         comcache.cpp
         comcallablewrapper.cpp

src/coreclr/vm/arm/asmconstants.h

@@ -103,26 +103,6 @@ ASMCONSTANTS_C_ASSERT(SIZEOF__FloatArgumentRegisters == sizeof(FloatArgumentRegi
 #define ASM_ENREGISTERED_RETURNTYPE_MAXSIZE 0x20
 ASMCONSTANTS_C_ASSERT(ASM_ENREGISTERED_RETURNTYPE_MAXSIZE == ENREGISTERED_RETURNTYPE_MAXSIZE)
 
-#ifdef FEATURE_COMINTEROP
-
-#define Stub__m_pCode DBG_FRE(0x10, 0x0c)
-ASMCONSTANTS_C_ASSERT(Stub__m_pCode == sizeof(Stub))
-
-#define SIZEOF__ComMethodFrame 0x24
-ASMCONSTANTS_C_ASSERT(SIZEOF__ComMethodFrame == sizeof(ComMethodFrame))
-
-#define UnmanagedToManagedFrame__m_pvDatum 0x08
-ASMCONSTANTS_C_ASSERT(UnmanagedToManagedFrame__m_pvDatum == offsetof(UnmanagedToManagedFrame, m_pvDatum))
-
-// In ComCallPreStub and GenericCLRToCOMCallStub, we setup R12 to contain address of ComCallMethodDesc after doing the following:
-//
-// mov r12, pc
-//
-// This constant defines where ComCallMethodDesc is post execution of the above instruction.
-#define ComCallMethodDesc_Offset_FromR12 0x8
-
-#endif // FEATURE_COMINTEROP
-
 #define               Thread__m_fPreemptiveGCDisabled   0x04
 ASMCONSTANTS_C_ASSERT(Thread__m_fPreemptiveGCDisabled == offsetof(Thread, m_fPreemptiveGCDisabled));
 #define Thread_m_fPreemptiveGCDisabled Thread__m_fPreemptiveGCDisabled

src/coreclr/vm/callsiteinspect.cpp

@@ -194,11 +194,9 @@ namespace
 
                 _ASSERTE((*src) != NULL || Nullable::IsNullableType(ty));
 #ifdef TARGET_UNIX
-                // Unboxing on non-Windows ABIs must be special cased
-                COMPlusThrowHR(COR_E_NOTSUPPORTED);
-#else
-                ty.GetMethodTable()->UnBoxIntoUnchecked(pvDest, (*src));
+#error Non-Windows ABIs must be special cased
 #endif
+                ty.GetMethodTable()->UnBoxIntoUnchecked(pvDest, (*src));
 
                 // return the object so it can be stored in the frame and
                 // propagated to the root set

src/coreclr/vm/frames.cpp

@@ -1713,7 +1713,6 @@ CLRToCOMMethodFrame::CLRToCOMMethodFrame(TransitionBlock * pTransitionBlock, Met
 }
 #endif // #ifndef DACCESS_COMPILE
 
-//virtual
 void CLRToCOMMethodFrame::GcScanRoots_Impl(promote_func* fn, ScanContext* sc)
 {
     WRAPPER_NO_CONTRACT;
@@ -1724,22 +1723,32 @@ void CLRToCOMMethodFrame::GcScanRoots_Impl(promote_func* fn, ScanContext* sc)
     FramedMethodFrame::GcScanRoots_Impl(fn, sc);
     PromoteCallerStack(fn, sc);
 
-
+    //
     // Promote the returned object
-    MethodDesc* methodDesc = GetFunction();
-    ReturnKind returnKind = methodDesc->GetReturnKind();
-    if (returnKind == RT_Object)
+    //
+
+    MetaSig sig(GetFunction());
+
+    TypeHandle thValueType;
+    CorElementType et = sig.GetReturnTypeNormalized(&thValueType);
+    if (CorTypeInfo::IsObjRef_NoThrow(et))
     {
         (*fn)(GetReturnObjectPtr(), sc, CHECK_APP_DOMAIN);
     }
-    else if (returnKind == RT_ByRef)
+    else if (CorTypeInfo::IsByRef_NoThrow(et))
     {
         PromoteCarefully(fn, GetReturnObjectPtr(), sc, GC_CALL_INTERIOR | CHECK_APP_DOMAIN);
     }
-    else
+    else if (et == ELEMENT_TYPE_VALUETYPE)
     {
-        _ASSERTE_MSG(!IsStructReturnKind(returnKind), "NYI: We can't promote multiregs struct returns");
-        _ASSERTE_MSG(IsScalarReturnKind(returnKind), "Non-scalar types must be promoted.");
+        ArgIterator argit(&sig);
+        if (!argit.HasRetBuffArg())
+        {
+#ifdef TARGET_UNIX
+#error Non-Windows ABIs must be special cased
+#endif
+            ReportPointersFromValueType(fn, sc, thValueType.AsMethodTable(), GetReturnObjectPtr());
+        }
     }
 }
 #endif // FEATURE_COMINTEROP

src/coreclr/vm/gccover.cpp

@@ -438,44 +438,16 @@ static MethodDesc* getTargetMethodDesc(PCODE target)
 
 void ReplaceInstrAfterCall(PBYTE instrToReplace, MethodDesc* callMD)
 {
-    ReturnKind returnKind = callMD->GetReturnKind(true);
+    ReturnKind returnKind = callMD->GetReturnKind();
     if (!IsValidReturnKind(returnKind))
     {
         // SKip GC coverage after the call.
         return;
     }
-    _ASSERTE(IsValidReturnKind(returnKind));
 
-    bool ispointerKind = IsPointerReturnKind(returnKind);
-    if (ispointerKind)
+    if (IsPointerReturnKind(returnKind))
     {
-        bool protectRegister[2] = { false, false };
-
-        bool moreRegisters = false;
-
-        ReturnKind fieldKind1 = ExtractRegReturnKind(returnKind, 0, moreRegisters);
-        if (IsPointerFieldReturnKind(fieldKind1))
-        {
-            protectRegister[0] = true;
-        }
-        if (moreRegisters)
-        {
-            ReturnKind fieldKind2 = ExtractRegReturnKind(returnKind, 1, moreRegisters);
-            if (IsPointerFieldReturnKind(fieldKind2))
-            {
-                protectRegister[1] = true;
-            }
-        }
-        _ASSERTE(!moreRegisters);
-
-        if (protectRegister[0] && !protectRegister[1])
-        {
-            *instrToReplace = INTERRUPT_INSTR_PROTECT_FIRST_RET;
-        }
-        else
-        {
-            _ASSERTE(!"Not expected multi reg return with pointers.");
-        }
+        *instrToReplace = INTERRUPT_INSTR_PROTECT_FIRST_RET;
     }
     else
     {

src/coreclr/vm/method.cpp

@@ -1234,8 +1234,9 @@ COR_ILMETHOD* MethodDesc::GetILHeader()
 #endif // !DACCESS_COMPILE
 }
 
+#if defined(TARGET_X86) && defined(HAVE_GCCOVER)
 //*******************************************************************************
-ReturnKind MethodDesc::ParseReturnKindFromSig(INDEBUG(bool supportStringConstructors))
+ReturnKind MethodDesc::GetReturnKind()
 {
     CONTRACTL
     {
@@ -1247,13 +1248,29 @@ ReturnKind MethodDesc::ParseReturnKindFromSig(INDEBUG(bool supportStringConstruc
 
     ENABLE_FORBID_GC_LOADER_USE_IN_THIS_SCOPE();
 
+    // For simplicity, we don't hijack in funclets, but if you ever change that,
+    // be sure to choose the OnHijack... callback type to match that of the FUNCLET
+    // not the main method (it would probably be Scalar).
+
+    // Mark that we are performing a stackwalker like operation on the current thread.
+    // This is necessary to allow the signature parsing functions to work without triggering any loads
+    StackWalkerWalkingThreadHolder threadStackWalking(GetThread());
+
     TypeHandle thValueType;
 
     MetaSig sig(this);
     CorElementType et = sig.GetReturnTypeNormalized(&thValueType);
 
     switch (et)
     {
+        case ELEMENT_TYPE_R4:
+        case ELEMENT_TYPE_R8:
+            // Figuring out whether the function returns FP or not is hard to do
+            // on-the-fly, so we use a different callback helper on x86 where this
+            // piece of information is needed in order to perform the right save &
+            // restore of the return value around the call to OnHijackScalarWorker.
+            return RT_Float;
+
         case ELEMENT_TYPE_STRING:
         case ELEMENT_TYPE_CLASS:
         case ELEMENT_TYPE_SZARRAY:
@@ -1262,7 +1279,6 @@ ReturnKind MethodDesc::ParseReturnKindFromSig(INDEBUG(bool supportStringConstruc
         case ELEMENT_TYPE_VAR:
             return RT_Object;
 
-#ifdef ENREGISTERED_RETURNTYPE_INTEGER_MAXSIZE
         case ELEMENT_TYPE_VALUETYPE:
             // We return value types in registers if they fit in ENREGISTERED_RETURNTYPE_MAXSIZE
             // These valuetypes could contain gc refs.
@@ -1275,68 +1291,29 @@ ReturnKind MethodDesc::ParseReturnKindFromSig(INDEBUG(bool supportStringConstruc
                     if (!thValueType.IsTypeDesc())
                     {
                         MethodTable * pReturnTypeMT = thValueType.AsMethodTable();
-#ifdef UNIX_AMD64_ABI
-                        if (pReturnTypeMT->IsRegPassedStruct())
+                        if (pReturnTypeMT->ContainsGCPointers())
                         {
-                            // The Multi-reg return case using the classhandle is only implemented for AMD64 SystemV ABI.
-                            // On other platforms, multi-reg return is not supported with GcInfo v1.
-                            // So, the relevant information must be obtained from the GcInfo tables (which requires version2).
-                            EEClass* eeClass = pReturnTypeMT->GetClass();
-                            ReturnKind regKinds[2] = { RT_Unset, RT_Unset };
-                            int orefCount = 0;
-                            for (int i = 0; i < 2; i++)
-                            {
-                                if (eeClass->GetEightByteClassification(i) == SystemVClassificationTypeIntegerReference)
-                                {
-                                    regKinds[i] = RT_Object;
-                                }
-                                else if (eeClass->GetEightByteClassification(i) == SystemVClassificationTypeIntegerByRef)
-                                {
-                                    regKinds[i] = RT_ByRef;
-                                }
-                                else
-                                {
-                                    regKinds[i] = RT_Scalar;
-                                }
-                            }
-                            ReturnKind structReturnKind = GetStructReturnKind(regKinds[0], regKinds[1]);
-                            return structReturnKind;
+                            _ASSERTE(pReturnTypeMT->GetNumInstanceFieldBytes() == sizeof(void*));
+                            return RT_Object;
                         }
-#endif // UNIX_AMD64_ABI
 
-                        if (pReturnTypeMT->ContainsGCPointers() || pReturnTypeMT->IsByRefLike())
+                        if (pReturnTypeMT->IsByRefLike())
                         {
-                            if (pReturnTypeMT->GetNumInstanceFields() == 1)
-                            {
-                                _ASSERTE(pReturnTypeMT->GetNumInstanceFieldBytes() == sizeof(void*));
-                                // Note: we can't distinguish RT_Object from RT_ByRef, the caller has to tolerate that.
-                                return RT_Object;
-                            }
-                            else
-                            {
-                                // Multi reg return case with pointers, can't restore the actual kind.
-                                return RT_Illegal;
-                            }
+                            // This would require going through all fields
+                            return RT_Illegal;
                         }
                     }
                 }
             }
             break;
-#endif // ENREGISTERED_RETURNTYPE_INTEGER_MAXSIZE
 
-#ifdef _DEBUG
         case ELEMENT_TYPE_VOID:
-            // String constructors return objects.  We should not have any ecall string
-            // constructors, except when called from gc coverage codes (which is only
-            // done under debug).  We will therefore optimize the retail version of this
-            // method to not support string constructors.
+            // String constructors return objects..
             if (IsCtor() && GetMethodTable()->HasComponentSize())
             {
-                _ASSERTE(supportStringConstructors);
                 return RT_Object;
             }
             break;
-#endif // _DEBUG
 
         case ELEMENT_TYPE_BYREF:
             return RT_ByRef;
@@ -1347,32 +1324,7 @@ ReturnKind MethodDesc::ParseReturnKindFromSig(INDEBUG(bool supportStringConstruc
 
     return RT_Scalar;
 }
-
-ReturnKind MethodDesc::GetReturnKind(INDEBUG(bool supportStringConstructors))
-{
-    // For simplicity, we don't hijack in funclets, but if you ever change that,
-    // be sure to choose the OnHijack... callback type to match that of the FUNCLET
-    // not the main method (it would probably be Scalar).
-
-    ENABLE_FORBID_GC_LOADER_USE_IN_THIS_SCOPE();
-    // Mark that we are performing a stackwalker like operation on the current thread.
-    // This is necessary to allow the signature parsing functions to work without triggering any loads
-    StackWalkerWalkingThreadHolder threadStackWalking(GetThread());
-
-#ifdef TARGET_X86
-    MetaSig msig(this);
-    if (msig.HasFPReturn())
-    {
-        // Figuring out whether the function returns FP or not is hard to do
-        // on-the-fly, so we use a different callback helper on x86 where this
-        // piece of information is needed in order to perform the right save &
-        // restore of the return value around the call to OnHijackScalarWorker.
-        return RT_Float;
-    }
-#endif // TARGET_X86
-
-    return ParseReturnKindFromSig(INDEBUG(supportStringConstructors));
-}
+#endif // TARGET_X86 && HAVE_GCCOVER
 
 #ifdef FEATURE_COMINTEROP