Respond to PR feedback.
authorEric Erhardt <eric.erhardt@microsoft.com>
Thu, 1 Feb 2018 00:19:50 +0000 (18:19 -0600)
committerEric Erhardt <eric.erhardt@microsoft.com>
Thu, 1 Feb 2018 00:19:50 +0000 (18:19 -0600)
- Move IsIntrinicType check earlier, and use it during CheckIfSIMDAndUpdateSize.
- Check all types for [Intrinsic] attribute.

src/mscorlib/shared/System/Runtime/CompilerServices/IntrinsicAttribute.cs
src/vm/methodtablebuilder.cpp
src/vm/methodtablebuilder.h

index bb73946..6bdd91d 100644 (file)
@@ -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
     {
index dce3e3c..dfb0ec4 100644 (file)
@@ -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())
index 1d10904..e64b72b 100644 (file)
@@ -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;