From 0a7461a1172e646e12b211aee11d215e6a86f5fd Mon Sep 17 00:00:00 2001 From: Eric Erhardt Date: Wed, 31 Jan 2018 18:19:50 -0600 Subject: [PATCH] Respond to PR feedback. - Move IsIntrinicType check earlier, and use it during CheckIfSIMDAndUpdateSize. - Check all types for [Intrinsic] attribute. --- .../Runtime/CompilerServices/IntrinsicAttribute.cs | 2 +- src/vm/methodtablebuilder.cpp | 36 ++++++++++++---------- src/vm/methodtablebuilder.h | 4 +-- 3 files changed, 23 insertions(+), 19 deletions(-) diff --git a/src/mscorlib/shared/System/Runtime/CompilerServices/IntrinsicAttribute.cs b/src/mscorlib/shared/System/Runtime/CompilerServices/IntrinsicAttribute.cs index bb73946..6bdd91d 100644 --- a/src/mscorlib/shared/System/Runtime/CompilerServices/IntrinsicAttribute.cs +++ b/src/mscorlib/shared/System/Runtime/CompilerServices/IntrinsicAttribute.cs @@ -6,7 +6,7 @@ namespace System.Runtime.CompilerServices { // Calls to methods or references to fields marked with this attribute may be replaced at // some call sites with jit intrinsic expansions. - // Types marked with this attribute may be specially treated by the rumtime/compiler. + // Types marked with this attribute may be specially treated by the runtime/compiler. [AttributeUsage(AttributeTargets.Class | AttributeTargets.Struct | AttributeTargets.Method | AttributeTargets.Constructor | AttributeTargets.Field, Inherited = false)] internal sealed class IntrinsicAttribute : Attribute { diff --git a/src/vm/methodtablebuilder.cpp b/src/vm/methodtablebuilder.cpp index dce3e3c..dfb0ec4 100644 --- a/src/vm/methodtablebuilder.cpp +++ b/src/vm/methodtablebuilder.cpp @@ -1170,7 +1170,7 @@ BOOL MethodTableBuilder::CheckIfSIMDAndUpdateSize() STANDARD_VM_CONTRACT; #if defined(_TARGET_X86_) || defined(_TARGET_AMD64_) - if (!(GetAssembly()->IsSIMDVectorAssembly() || GetModule()->IsSystem())) + if (!(GetAssembly()->IsSIMDVectorAssembly() || bmtProp->fIsIntrinsicType)) return false; if (bmtFP->NumInstanceFieldBytes != 16) @@ -1533,6 +1533,23 @@ MethodTableBuilder::BuildMethodTableThrowing( } #endif + // 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 fairly early to ensure other downstream checks on these types can be slightly more efficient. + if (GetModule()->IsSystem() || GetAssembly()->IsSIMDVectorAssembly()) + { + HRESULT hr = GetMDImport()->GetCustomAttributeByName(bmtInternal->pType->GetTypeDefToken(), + g_CompilerServicesIntrinsicAttribute, + NULL, + NULL); + + if (hr == S_OK) + { + bmtProp->fIsIntrinsicType = true; + } + } + #ifdef FEATURE_COMINTEROP // Com Import classes are special. These types must derive from System.Object, @@ -10428,22 +10445,9 @@ MethodTableBuilder::SetupMethodTable2( } pMT->SetInternalCorElementType(normalizedType); - // 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 fairly early to ensure other downstream checks on these types can be slightly more efficient. - if ((GetModule()->IsSystem() || GetAssembly()->IsSIMDVectorAssembly()) && - ((IsValueClass() && bmtGenerics->HasInstantiation()) || (IsAbstract() && IsSealed()))) + if (bmtProp->fIsIntrinsicType) { - HRESULT hr = GetMDImport()->GetCustomAttributeByName(bmtInternal->pType->GetTypeDefToken(), - g_CompilerServicesIntrinsicAttribute, - NULL, - NULL); - - if (hr == S_OK) - { - pMT->SetIsIntrinsicType(); - } + pMT->SetIsIntrinsicType(); } if (GetModule()->IsSystem()) diff --git a/src/vm/methodtablebuilder.h b/src/vm/methodtablebuilder.h index 1d10904..e64b72b 100644 --- a/src/vm/methodtablebuilder.h +++ b/src/vm/methodtablebuilder.h @@ -218,8 +218,7 @@ private: BOOL HasNonPublicFields() { WRAPPER_NO_CONTRACT; return GetHalfBakedClass()->HasNonPublicFields(); } BOOL IsValueClass() { WRAPPER_NO_CONTRACT; return bmtProp->fIsValueClass; } BOOL IsUnsafeValueClass() { WRAPPER_NO_CONTRACT; return GetHalfBakedClass()->IsUnsafeValueClass(); } - BOOL IsAbstract() { WRAPPER_NO_CONTRACT; return GetHalfBakedClass()->IsAbstract(); } - BOOL IsSealed() { WRAPPER_NO_CONTRACT; return GetHalfBakedClass()->IsSealed(); } + BOOL IsAbstract() { WRAPPER_NO_CONTRACT; return GetHalfBakedClass()->IsAbstract(); } BOOL HasLayout() { WRAPPER_NO_CONTRACT; return GetHalfBakedClass()->HasLayout(); } BOOL IsDelegate() { WRAPPER_NO_CONTRACT; return GetHalfBakedClass()->IsDelegate(); } BOOL IsNested() { WRAPPER_NO_CONTRACT; return GetHalfBakedClass()->IsNested(); } @@ -1334,6 +1333,7 @@ private: bool fDynamicStatics; // Set to true if the statics will be allocated in the dynamic bool fGenericsStatics; // Set to true if the there are per-instantiation statics + bool fIsIntrinsicType; // Set to true if the type has an [Intrinsic] attribute on it bool fIsHardwareIntrinsic; // Set to true if the class is a hardware intrinsic DWORD dwNonGCRegularStaticFieldBytes; -- 2.7.4