From 34776bf5f6bba461b9f616744cb6e17962ccce3b Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Fri, 19 Jan 2018 15:49:47 -0800 Subject: [PATCH 1/2] Updating the VM to no longer treat the SIMD HWIntrinsic types as HFA or MultiReg structs. --- src/vm/class.cpp | 14 +++++++++++++ src/vm/methodtable.cpp | 37 +++++++++++++++++++++++++++++++++++ src/vm/methodtablebuilder.cpp | 33 +++++++++++++++++-------------- 3 files changed, 69 insertions(+), 15 deletions(-) diff --git a/src/vm/class.cpp b/src/vm/class.cpp index 99156ffb5936..1bbf9b674bac 100644 --- a/src/vm/class.cpp +++ b/src/vm/class.cpp @@ -1722,6 +1722,20 @@ EEClass::CheckForHFA() if (HasExplicitFieldOffsetLayout()) return false; + // The SIMD Intrinsic types are meant to be handled specially and should not be treated as HFA + if (GetMethodTable()->IsIntrinsicType()) + { + LPCUTF8 namespaceName; + LPCUTF8 className = GetMethodTable()->GetFullyQualifiedNameInfo(&namespaceName); + + if ((strcmp(className, "Vector256`1") == 0) || (strcmp(className, "Vector128`1") == 0) || + (strcmp(className, "Vector64`1") == 0)) + { + assert(strcmp(namespaceName, "System.Runtime.Intrinsics") == 0); + return false; + } + } + CorElementType hfaType = ELEMENT_TYPE_END; FieldDesc *pFieldDescList = GetFieldDescList(); diff --git a/src/vm/methodtable.cpp b/src/vm/methodtable.cpp index 24c69cfb012e..76d490d251e9 100644 --- a/src/vm/methodtable.cpp +++ b/src/vm/methodtable.cpp @@ -2340,6 +2340,25 @@ bool MethodTable::ClassifyEightBytesWithManagedLayout(SystemVStructRegisterPassi nestingLevel * 5, "", this->GetDebugClassName())); return false; } + + // The SIMD Intrinsic types are meant to be handled specially and should not be passed as struct registers + if (IsIntrinsicType()) + { + LPCUTF8 namespaceName; + LPCUTF8 className = GetFullyQualifiedNameInfo(&namespaceName); + + if ((strcmp(className, "Vector256`1") == 0) || (strcmp(className, "Vector128`1") == 0) || + (strcmp(className, "Vector64`1") == 0)) + { + assert(strcmp(namespaceName, "System.Runtime.Intrinsics") == 0); + + LOG((LF_JIT, LL_EVERYTHING, "%*s**** ClassifyEightBytesWithManagedLayout: struct %s is a SIMD intrinsic type; will not be enregistered\n", + nestingLevel * 5, "", this->GetDebugClassName())); + + return false; + } + } + #ifdef _DEBUG LOG((LF_JIT, LL_EVERYTHING, "%*s**** Classify %s (%p), startOffset %d, total struct size %d\n", nestingLevel * 5, "", this->GetDebugClassName(), this, startOffsetOfStruct, helperPtr->structSize)); @@ -2619,6 +2638,24 @@ bool MethodTable::ClassifyEightBytesWithNativeLayout(SystemVStructRegisterPassin return false; } + // The SIMD Intrinsic types are meant to be handled specially and should not be passed as struct registers + if (IsIntrinsicType()) + { + LPCUTF8 namespaceName; + LPCUTF8 className = GetFullyQualifiedNameInfo(&namespaceName); + + if ((strcmp(className, "Vector256`1") == 0) || (strcmp(className, "Vector128`1") == 0) || + (strcmp(className, "Vector64`1") == 0)) + { + assert(strcmp(namespaceName, "System.Runtime.Intrinsics") == 0); + + LOG((LF_JIT, LL_EVERYTHING, "%*s**** ClassifyEightBytesWithNativeLayout: struct %s is a SIMD intrinsic type; will not be enregistered\n", + nestingLevel * 5, "", this->GetDebugClassName())); + + return false; + } + } + #ifdef _DEBUG LOG((LF_JIT, LL_EVERYTHING, "%*s**** Classify for native struct %s (%p), startOffset %d, total struct size %d\n", nestingLevel * 5, "", this->GetDebugClassName(), this, startOffsetOfStruct, helperPtr->structSize)); diff --git a/src/vm/methodtablebuilder.cpp b/src/vm/methodtablebuilder.cpp index 725fb2d8637f..d25fc95054ed 100644 --- a/src/vm/methodtablebuilder.cpp +++ b/src/vm/methodtablebuilder.cpp @@ -1844,6 +1844,24 @@ MethodTableBuilder::BuildMethodTableThrowing( pMT->SetIsByRefLike(); } + // If this type is marked by [Intrinsic] attribute, it may be specially treated by the runtime/compiler + // Currently, only SIMD types have [Intrinsic] attribute + // + // We check this here, before the SystemVAmd64CheckForPass[Native]StructInRegister calls to ensure the SIMD + // intrinsics are not enregistered incorrectly. + if ((GetModule()->IsSystem() || GetAssembly()->IsSIMDVectorAssembly()) && IsValueClass() && bmtGenerics->HasInstantiation()) + { + HRESULT hr = GetMDImport()->GetCustomAttributeByName(bmtInternal->pType->GetTypeDefToken(), + g_CompilerServicesIntrinsicAttribute, + NULL, + NULL); + + if (hr == S_OK) + { + pMT->SetIsIntrinsicType(); + } + } + if (IsValueClass()) { if (bmtFP->NumInstanceFieldBytes != totalDeclaredFieldSize || HasOverLayedField()) @@ -2025,21 +2043,6 @@ MethodTableBuilder::BuildMethodTableThrowing( pMT->SetICastable(); } #endif // FEATURE_ICASTABLE - - // If this type is marked by [Intrinsic] attribute, it may be specially treated by the runtime/compiler - // Currently, only SIMD types have [Intrinsic] attribute - if ((GetModule()->IsSystem() || GetAssembly()->IsSIMDVectorAssembly()) && IsValueClass() && bmtGenerics->HasInstantiation()) - { - HRESULT hr = GetMDImport()->GetCustomAttributeByName(bmtInternal->pType->GetTypeDefToken(), - g_CompilerServicesIntrinsicAttribute, - NULL, - NULL); - - if (hr == S_OK) - { - pMT->SetIsIntrinsicType(); - } - } // Grow the typedef ridmap in advance as we can't afford to // fail once we set the resolve bit From 441487910f9b183bb41deffe9476cd87c1c2ba6b Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Sat, 20 Jan 2018 10:23:52 -0800 Subject: [PATCH 2/2] Stop the SIMD hardware intrinsics types from undergoing crossgen. --- src/vm/methodtablebuilder.cpp | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/vm/methodtablebuilder.cpp b/src/vm/methodtablebuilder.cpp index d25fc95054ed..a47e464d72c1 100644 --- a/src/vm/methodtablebuilder.cpp +++ b/src/vm/methodtablebuilder.cpp @@ -1499,7 +1499,21 @@ MethodTableBuilder::BuildMethodTableThrowing( LPCUTF8 className; LPCUTF8 nameSpace; HRESULT hr = GetMDImport()->GetNameOfTypeDef(bmtInternal->pType->GetTypeDefToken(), &className, &nameSpace); - + + if (hr == S_OK && strcmp(nameSpace, "System.Runtime.Intrinsics") == 0) + { + if (IsCompilationProcess()) + { + // Disable AOT compiling for the SIMD hardware intrinsic types. These types require special + // ABI handling as they represent fundamental data types (__m64, __m128, and __m256) and not + // aggregate or union types. See https://github.com/dotnet/coreclr/issues/15943 + // + // Once they are properly handled according to the ABI requirements, we can remove this check + // and allow them to be used in crossgen/AOT scenarios. + COMPlusThrow(kTypeLoadException, IDS_EE_HWINTRINSIC_NGEN_DISALLOWED); + } + } + #if defined(_TARGET_ARM64_) // All the funtions in System.Runtime.Intrinsics.Arm.Arm64 are hardware intrinsics. if (hr == S_OK && strcmp(nameSpace, "System.Runtime.Intrinsics.Arm.Arm64") == 0)