Jit: use class attributes check to reduce GC layout asks
authorAndy Ayers <andya@microsoft.com>
Thu, 20 Oct 2016 20:48:22 +0000 (13:48 -0700)
committerAndy Ayers <andya@microsoft.com>
Fri, 21 Oct 2016 00:38:28 +0000 (17:38 -0700)
In `impNormStructType`, check the class attributes for absence of gc
pointers as a quick way of identifying likely SIMD types. Use SIMD max
and min size as a secondary screen.

Only request GC layout if the caller has asked for it.

This should reduce the number of times the jit makes the somewhat
costly ask for struct GC layouts.

Note that typed byrefs require special handling; also the class
attributes may sometimes report GC pointers when in fact there are none.

No diffs in System.Private.CoreLib codegen; no diffs seen in desktop
SPMI testing.

Closes #7625.

src/jit/importer.cpp

index 53e856c..c9e03ad 100644 (file)
@@ -1455,6 +1455,8 @@ GenTreePtr Compiler::impGetStructAddr(GenTreePtr           structVal,
 //                      into which the gcLayout will be written.
 //    pNumGCVars      - (optional, default nullptr) - if non-null, a pointer to an unsigned,
 //                      which will be set to the number of GC fields in the struct.
+//    pSimdBaseType   - (optional, default nullptr) - if non-null, and the struct is a SIMD
+//                      type, set to the SIMD base type
 //
 // Return Value:
 //    The JIT type for the struct (e.g. TYP_STRUCT, or TYP_SIMD*).
@@ -1476,53 +1478,63 @@ var_types Compiler::impNormStructType(CORINFO_CLASS_HANDLE structHnd,
                                       var_types*           pSimdBaseType)
 {
     assert(structHnd != NO_CLASS_HANDLE);
-    unsigned  originalSize        = info.compCompHnd->getClassSize(structHnd);
-    unsigned  numGCVars           = 0;
-    var_types structType          = TYP_STRUCT;
-    var_types simdBaseType        = TYP_UNKNOWN;
-    bool      definitelyHasGCPtrs = false;
 
-#ifdef FEATURE_SIMD
-    // We don't want to consider this as a possible SIMD type if it has GC pointers.
-    // (Saves querying about the SIMD assembly.)
-    BYTE gcBytes[maxPossibleSIMDStructBytes / TARGET_POINTER_SIZE];
-    if ((gcLayout == nullptr) && (originalSize >= minSIMDStructBytes()) && (originalSize <= maxSIMDStructBytes()))
-    {
-        gcLayout = gcBytes;
-    }
-#endif // FEATURE_SIMD
+    const DWORD structFlags = info.compCompHnd->getClassAttribs(structHnd);
+    const bool  isRefAny    = (structHnd == impGetRefAnyClass());
+    const bool  hasGCPtrs   = isRefAny || ((structFlags & CORINFO_FLG_CONTAINS_GC_PTR) != 0);
+    var_types   structType  = TYP_STRUCT;
 
-    if (gcLayout != nullptr)
-    {
-        numGCVars           = info.compCompHnd->getClassGClayout(structHnd, gcLayout);
-        definitelyHasGCPtrs = (numGCVars != 0);
-    }
 #ifdef FEATURE_SIMD
     // Check to see if this is a SIMD type.
-    if (featureSIMD && (originalSize <= getSIMDVectorRegisterByteLength()) && (originalSize >= TARGET_POINTER_SIZE) &&
-        !definitelyHasGCPtrs)
+    if (featureSIMD && !hasGCPtrs)
     {
-        unsigned int sizeBytes;
-        simdBaseType = getBaseTypeAndSizeOfSIMDType(structHnd, &sizeBytes);
-        if (simdBaseType != TYP_UNKNOWN)
+        unsigned originalSize = info.compCompHnd->getClassSize(structHnd);
+
+        if ((originalSize >= minSIMDStructBytes()) && (originalSize <= maxSIMDStructBytes()))
         {
-            assert(sizeBytes == originalSize);
-            structType = getSIMDTypeForSize(sizeBytes);
-            if (pSimdBaseType != nullptr)
+            unsigned int sizeBytes;
+            var_types    simdBaseType = getBaseTypeAndSizeOfSIMDType(structHnd, &sizeBytes);
+            if (simdBaseType != TYP_UNKNOWN)
             {
-                *pSimdBaseType = simdBaseType;
-            }
+                assert(sizeBytes == originalSize);
+                structType = getSIMDTypeForSize(sizeBytes);
+                if (pSimdBaseType != nullptr)
+                {
+                    *pSimdBaseType = simdBaseType;
+                }
 #ifdef _TARGET_AMD64_
-            // Amd64: also indicate that we use floating point registers
-            compFloatingPointUsed = true;
+                // Amd64: also indicate that we use floating point registers
+                compFloatingPointUsed = true;
 #endif
+            }
         }
     }
 #endif // FEATURE_SIMD
-    if (pNumGCVars != nullptr)
+
+    // Fetch GC layout info if requested
+    if (gcLayout != nullptr)
+    {
+        unsigned numGCVars = info.compCompHnd->getClassGClayout(structHnd, gcLayout);
+
+        // Verify that the quick test up above via the class attributes gave a
+        // safe view of the type's GCness.
+        //
+        // Note there are cases where hasGCPtrs is true but getClassGClayout
+        // does not report any gc fields.
+        assert(hasGCPtrs || (numGCVars == 0));
+
+        if (pNumGCVars != nullptr)
+        {
+            *pNumGCVars = numGCVars;
+        }
+    }
+    else
     {
-        *pNumGCVars = numGCVars;
+        // Can't safely ask for number of GC pointers without also
+        // asking for layout.
+        assert(pNumGCVars == nullptr);
     }
+
     return structType;
 }