Fixed a bad merge issue and some internal API cleanup.

Sometime ago when the SystemVStructs branch was PRed into master a bad
merge has occured.

Some struct classification cleanup for native vs. managed struct layout
representation.

When classifying a native layout of the struct use the following
algorithm.

1. If the struct has special marshaling it has a native layout.
   In such cases the classifier needs to use the native layout.
2. For structs with no native layout, the managed layout should be used
   even if classified for the purposes of marshaling/PInvoke passing.

diff --git a/src/vm/jitinterface.cpp b/src/vm/jitinterface.cpp
index 04f8244..c331a30 100644 (file)
--- a/src/vm/jitinterface.cpp
+++ b/src/vm/jitinterface.cpp
@@ -2584,7 +2584,14 @@ bool CEEInfo::getSystemVAmd64PassStructInRegisterDescriptor(
         _ASSERTE(CorInfoType2UnixAmd64Classification(th.GetInternalCorElementType()) == SystemVClassificationTypeStruct);
 
         MethodTable* methodTablePtr = nullptr;
-        bool isNativeStruct = false;
+
+        // The useNativeLayout in this case tracks whether the classification
+        // is for a native layout of the struct or not.
+        // If the struct has special marshaling it has a native layout. 
+        // In such cases the classifier needs to use the native layout.
+        // For structs with no native layout, the managed layout should be used
+        // even if classified for the purposes of marshaling/PInvoke passing.
+        bool useNativeLayout = false;
         if (!th.IsTypeDesc())
         {
             methodTablePtr = th.AsMethodTable();
@@ -2595,7 +2602,7 @@ bool CEEInfo::getSystemVAmd64PassStructInRegisterDescriptor(
             if (th.IsNativeValueType())
             {
                 methodTablePtr = th.AsNativeValueType();
-                isNativeStruct = true;
+                useNativeLayout = true;
                 _ASSERTE(methodTablePtr != nullptr);
             }
             else
@@ -2606,12 +2613,14 @@ bool CEEInfo::getSystemVAmd64PassStructInRegisterDescriptor(
 
         bool isPassableInRegs = false;
 
-        if (isNativeStruct)
+        if (useNativeLayout)
         {
+            _ASSERTE(th.IsNativeValueType());
             isPassableInRegs = methodTablePtr->GetLayoutInfo()->IsNativeStructPassedInRegisters();
         }
         else
         {
+            _ASSERTE(!th.IsNativeValueType());
             isPassableInRegs = methodTablePtr->IsRegPassedStruct();
         }
 
@@ -2621,10 +2630,12 @@ bool CEEInfo::getSystemVAmd64PassStructInRegisterDescriptor(
         }
         else
         {
-            structPassInRegDescPtr->passedInRegisters = true;
-
             SystemVStructRegisterPassingHelper helper((unsigned int)th.GetSize());
-            bool result = methodTablePtr->ClassifyEightBytes(&helper, 0, 0);
+            bool result = methodTablePtr->ClassifyEightBytes(&helper, 0, 0, useNativeLayout);
+
+            // The answer must be true at this point.
+            _ASSERTE(result);
+            structPassInRegDescPtr->passedInRegisters = true;
 
             structPassInRegDescPtr->eightByteCount = helper.eightByteCount;
             _ASSERTE(structPassInRegDescPtr->eightByteCount <= CLR_SYSTEMV_MAX_EIGHTBYTES_COUNT_TO_PASS_IN_REGISTERS);

diff --git a/src/vm/methodtable.cpp b/src/vm/methodtable.cpp
index e138f26..431095f 100644 (file)
--- a/src/vm/methodtable.cpp
+++ b/src/vm/methodtable.cpp
@@ -2300,6 +2300,19 @@ const char* GetSystemVClassificationTypeName(SystemVClassificationType t)
 };
 #endif // _DEBUG && LOGGING
 
+// Returns 'true' if the struct is passed in registers, 'false' otherwise.
+bool MethodTable::ClassifyEightBytes(SystemVStructRegisterPassingHelperPtr helperPtr, unsigned int nestingLevel, unsigned int startOffsetOfStruct, bool useNativeLayout)
+{
+    if (useNativeLayout)
+    {
+        return ClassifyEightBytesWithNativeLayout(helperPtr, nestingLevel, startOffsetOfStruct, useNativeLayout);
+    }
+    else
+    {
+        return ClassifyEightBytesWithManagedLayout(helperPtr, nestingLevel, startOffsetOfStruct, useNativeLayout);
+    }
+}
+
 // If we have a field classification already, but there is a union, we must merge the classification type of the field. Returns the
 // new, merged classification type.
 /* static */
@@ -2344,7 +2357,10 @@ SystemVClassificationType MethodTable::ReClassifyField(SystemVClassificationType
 }
 
 // Returns 'true' if the struct is passed in registers, 'false' otherwise.
-bool MethodTable::ClassifyEightBytes(SystemVStructRegisterPassingHelperPtr helperPtr, unsigned int nestingLevel, unsigned int startOffsetOfStruct)
+bool MethodTable::ClassifyEightBytesWithManagedLayout(SystemVStructRegisterPassingHelperPtr helperPtr,
+                                                     unsigned int nestingLevel, 
+                                                     unsigned int startOffsetOfStruct,
+                                                     bool useNativeLayout)
 {
     CONTRACTL
     {
@@ -2369,7 +2385,7 @@ bool MethodTable::ClassifyEightBytes(SystemVStructRegisterPassingHelperPtr helpe
     // unconditionally disable register struct passing for explicit layout.
     if (GetClass()->HasExplicitFieldOffsetLayout())
     {
-        LOG((LF_JIT, LL_EVERYTHING, "%*s**** ClassifyEightBytes: struct %s has explicit layout; will not be enregistered\n",
+        LOG((LF_JIT, LL_EVERYTHING, "%*s**** ClassifyEightBytesWithManagedLayout: struct %s has explicit layout; will not be enregistered\n",
                nestingLevel * 5, "", this->GetDebugClassName()));
         return false;
     }
@@ -2418,7 +2434,19 @@ bool MethodTable::ClassifyEightBytes(SystemVStructRegisterPassingHelperPtr helpe
 
             bool inEmbeddedStructPrev = helperPtr->inEmbeddedStruct;
             helperPtr->inEmbeddedStruct = true;
-            bool structRet = pFieldMT->ClassifyEightBytes(helperPtr, nestingLevel + 1, normalizedFieldOffset);
+
+            bool structRet = false;
+            // If classifying for marshaling/PInvoke and the aggregated struct has a native layout
+            // use the native classification. If not, continue using the managed layout.
+            if (useNativeLayout && pFieldMT->HasLayout())
+            {
+                structRet = pFieldMT->ClassifyEightBytesWithNativeLayout(helperPtr, nestingLevel + 1, normalizedFieldOffset, useNativeLayout);
+            }
+            else
+            {
+                structRet = pFieldMT->ClassifyEightBytesWithManagedLayout(helperPtr, nestingLevel + 1, normalizedFieldOffset, useNativeLayout);
+            }
+            
             helperPtr->inEmbeddedStruct = inEmbeddedStructPrev;
 
             if (!structRet)
@@ -2515,7 +2543,10 @@ bool MethodTable::ClassifyEightBytes(SystemVStructRegisterPassingHelperPtr helpe
 }
 
 // Returns 'true' if the struct is passed in registers, 'false' otherwise.
-bool MethodTable::ClassifyEightBytesForNativeStruct(SystemVStructRegisterPassingHelperPtr helperPtr, unsigned int nestingLevel, unsigned int startOffsetOfStruct)
+bool MethodTable::ClassifyEightBytesWithNativeLayout(SystemVStructRegisterPassingHelperPtr helperPtr,
+                                                    unsigned int nestingLevel, 
+                                                    unsigned int startOffsetOfStruct, 
+                                                    bool useNativeLayout)
 {
     CONTRACTL
     {
@@ -2526,6 +2557,9 @@ bool MethodTable::ClassifyEightBytesForNativeStruct(SystemVStructRegisterPassing
     }
     CONTRACTL_END;
 
+    // Should be in this method only doing a native layout classification.
+    _ASSERTE(useNativeLayout);
+
 #ifdef DACCESS_COMPILE
     // No register classification for this case.
     return false;
@@ -2533,7 +2567,8 @@ bool MethodTable::ClassifyEightBytesForNativeStruct(SystemVStructRegisterPassing
 
     if (!HasLayout())
     {
-        return false;
+        // If there is no native layout for this struct use the managed layout instead.
+        return ClassifyEightBytesWithManagedLayout(helperPtr, nestingLevel, startOffsetOfStruct, useNativeLayout);
     }
 
     const FieldMarshaler *pFieldMarshaler = GetLayoutInfo()->GetFieldMarshalers();
@@ -2550,7 +2585,7 @@ bool MethodTable::ClassifyEightBytesForNativeStruct(SystemVStructRegisterPassing
     // unconditionally disable register struct passing for explicit layout.
     if (GetClass()->HasExplicitFieldOffsetLayout())
     {
-        LOG((LF_JIT, LL_EVERYTHING, "%*s**** ClassifyEightBytesForNativeStruct: struct %s has explicit layout; will not be enregistered\n",
+        LOG((LF_JIT, LL_EVERYTHING, "%*s**** ClassifyEightBytesWithNativeLayout: struct %s has explicit layout; will not be enregistered\n",
             nestingLevel * 5, "", this->GetDebugClassName()));
         return false;
     }
@@ -2681,7 +2716,7 @@ bool MethodTable::ClassifyEightBytesForNativeStruct(SystemVStructRegisterPassing
 
             bool inEmbeddedStructPrev = helperPtr->inEmbeddedStruct;
             helperPtr->inEmbeddedStruct = true;
-            bool structRet = pFieldMT->ClassifyEightBytesForNativeStruct(helperPtr, nestingLevel + 1, normalizedFieldOffset);
+            bool structRet = pFieldMT->ClassifyEightBytesWithNativeLayout(helperPtr, nestingLevel + 1, normalizedFieldOffset, useNativeLayout);
             helperPtr->inEmbeddedStruct = inEmbeddedStructPrev;
 
             if (!structRet)
@@ -2698,7 +2733,7 @@ bool MethodTable::ClassifyEightBytesForNativeStruct(SystemVStructRegisterPassing
 
             bool inEmbeddedStructPrev = helperPtr->inEmbeddedStruct;
             helperPtr->inEmbeddedStruct = true;
-            bool structRet = pFieldMT->ClassifyEightBytesForNativeStruct(helperPtr, nestingLevel + 1, normalizedFieldOffset);
+            bool structRet = pFieldMT->ClassifyEightBytesWithNativeLayout(helperPtr, nestingLevel + 1, normalizedFieldOffset, useNativeLayout);
             helperPtr->inEmbeddedStruct = inEmbeddedStructPrev;
 
             if (!structRet)

diff --git a/src/vm/methodtable.h b/src/vm/methodtable.h
index 17f9b36..61933c0 100644 (file)
--- a/src/vm/methodtable.h
+++ b/src/vm/methodtable.h
@@ -1048,12 +1048,11 @@ public:
     void CheckRunClassInitAsIfConstructingThrowing();
 
 #if defined(FEATURE_UNIX_AMD64_STRUCT_PASSING_ITF)
-    // Helper function for ClassifyEightBytes
+    // Helper function for ClassifyEightBytesWithManagedLayout and ClassifyEightBytesWithNativeLayout
     static SystemVClassificationType ReClassifyField(SystemVClassificationType originalClassification, SystemVClassificationType newFieldClassification);
 
     // Builds the internal data structures and classifies struct eightbytes for Amd System V calling convention.
-    bool ClassifyEightBytes(SystemVStructRegisterPassingHelperPtr helperPtr, unsigned int nestingLevel, unsigned int startOffsetOfStruct);
-    bool ClassifyEightBytesForNativeStruct(SystemVStructRegisterPassingHelperPtr helperPtr, unsigned int nestingLevel, unsigned int startOffsetOfStruct);
+    bool ClassifyEightBytes(SystemVStructRegisterPassingHelperPtr helperPtr, unsigned int nestingLevel, unsigned int startOffsetOfStruct, bool isNativeStruct);
 #endif // defined(FEATURE_UNIX_AMD64_STRUCT_PASSING_ITF)
 
     // Copy m_dwFlags from another method table
@@ -1093,6 +1092,9 @@ private:
 
 #if defined(FEATURE_UNIX_AMD64_STRUCT_PASSING_ITF)
     void AssignClassifiedEightByteTypes(SystemVStructRegisterPassingHelperPtr helperPtr, unsigned int nestingLevel);
+    // Builds the internal data structures and classifies struct eightbytes for Amd System V calling convention.
+    bool ClassifyEightBytesWithManagedLayout(SystemVStructRegisterPassingHelperPtr helperPtr, unsigned int nestingLevel, unsigned int startOffsetOfStruct, bool isNativeStruct);
+    bool ClassifyEightBytesWithNativeLayout(SystemVStructRegisterPassingHelperPtr helperPtr, unsigned int nestingLevel, unsigned int startOffsetOfStruct, bool isNativeStruct);
 #endif // defined(FEATURE_UNIX_AMD64_STRUCT_PASSING_ITF)
 
     DWORD   GetClassIndexFromToken(mdTypeDef typeToken)

diff --git a/src/vm/methodtablebuilder.cpp b/src/vm/methodtablebuilder.cpp
index 0e3cb45..874cc13 100644 (file)
--- a/src/vm/methodtablebuilder.cpp
+++ b/src/vm/methodtablebuilder.cpp
@@ -8472,10 +8472,10 @@ void MethodTableBuilder::SystemVAmd64CheckForPassStructInRegister()
         return;
     }
 
+    const bool useNativeLayout = false;
     // Iterate through the fields and make sure they meet requirements to pass in registers
     SystemVStructRegisterPassingHelper helper((unsigned int)totalStructSize);
-
-    if (GetHalfBakedMethodTable()->ClassifyEightBytes(&helper, 0, 0))
+    if (GetHalfBakedMethodTable()->ClassifyEightBytes(&helper, 0, 0, useNativeLayout))
     {
         // All the above tests passed. It's registers passed struct!
         GetHalfBakedMethodTable()->SetRegPassedStruct();
@@ -8510,10 +8510,10 @@ void MethodTableBuilder::SystemVAmd64CheckForPassNativeStructInRegister()
     _ASSERTE(HasLayout());
 
     // Classify the native layout for this struct.
-   
+    const bool useNativeLayout = true;
     // Iterate through the fields and make sure they meet requirements to pass in registers
     SystemVStructRegisterPassingHelper helper((unsigned int)totalStructSize);
-    if (GetHalfBakedMethodTable()->ClassifyEightBytesForNativeStruct(&helper, 0, 0))
+    if (GetHalfBakedMethodTable()->ClassifyEightBytes(&helper, 0, 0, useNativeLayout))
     {
         GetLayoutInfo()->SetNativeStructPassedInRegisters();
     }