Migrate interpreter stack maps to InterpCompiler and use them in place of CORINFO_FLG_CONTAINS_GC_PTR (#118860)

* Use GetInterpreterStackMap to determine whether a class actually contains any refs
* Keep a unique stackmap hashtable per InterpCompiler instance that is owned by the compiler
* Add dtor to InterpreterStackMap that frees slots if present
* Add optional ValueDestroyCallback ctor arg to dn_simdhash_ptr_ptr_holder

Author: kg

src/coreclr/interpreter/compiler.cpp

@@ -1078,7 +1078,7 @@ void InterpCompiler::BuildGCInfo(InterpMethod *pInterpMethod)
                     break;
                 case InterpTypeVT:
                 {
-                    InterpreterStackMap *stackMap = GetInterpreterStackMap(m_compHnd, pVar->clsHnd);
+                    InterpreterStackMap *stackMap = GetInterpreterStackMap(pVar->clsHnd);
                     for (unsigned j = 0; j < stackMap->m_slotCount; j++)
                     {
                         InterpreterStackMapSlot slotInfo = stackMap->m_slots[j];
@@ -1232,9 +1232,26 @@ int32_t* InterpCompiler::GetCode(int32_t *pCodeSize)
     return m_pMethodCode;
 }
 
+InterpreterStackMap* InterpCompiler::GetInterpreterStackMap(CORINFO_CLASS_HANDLE classHandle)
+{
+    InterpreterStackMap* result = nullptr;
+    if (!dn_simdhash_ptr_ptr_try_get_value(m_stackmapsByClass.GetValue(), classHandle, (void **)&result))
+    {
+        result = new InterpreterStackMap(m_compHnd, classHandle);
+        checkAddedNew(dn_simdhash_ptr_ptr_try_add(m_stackmapsByClass.GetValue(), classHandle, result));
+    }
+    return result;
+}
+
+static void FreeInterpreterStackMap(void *key, void *value, void *userdata)
+{
+    delete (InterpreterStackMap *)value;
+}
+
 InterpCompiler::InterpCompiler(COMP_HANDLE compHnd,
                                 CORINFO_METHOD_INFO* methodInfo)
-    : m_pInitLocalsIns(nullptr)
+    : m_stackmapsByClass(FreeInterpreterStackMap)
+    , m_pInitLocalsIns(nullptr)
     , m_globalVarsWithRefsStackTop(0)
 {
     m_genericLookupToDataItemIndex.Init(&m_dataItems, this);
@@ -3220,7 +3237,7 @@ void InterpCompiler::EmitStind(InterpType interpType, CORINFO_CLASS_HANDLE clsHn
     // or in the reverse order if the flag is set
     if (interpType == InterpTypeVT)
     {
-        if (m_compHnd->getClassAttribs(clsHnd) & CORINFO_FLG_CONTAINS_GC_PTR)
+        if (GetInterpreterStackMap(clsHnd)->m_slotCount)
         {
             AddIns(INTOP_STIND_VT);
             m_pLastNewIns->data[1] = GetDataItemIndex(clsHnd);
@@ -6048,7 +6065,7 @@ void InterpCompiler::GenerateCode(CORINFO_METHOD_INFO* methodInfo)
                     case InterpTypeVT:
                     {
                         int size = m_compHnd->getClassSize(elemClsHnd);
-                        bool hasRefs = (m_compHnd->getClassAttribs(elemClsHnd) & CORINFO_FLG_CONTAINS_GC_PTR) != 0;
+                        bool hasRefs = GetInterpreterStackMap(elemClsHnd)->m_slotCount > 0;
                         m_pStackPointer -= 3;
                         if (hasRefs)
                         {

src/coreclr/interpreter/compiler.h

@@ -24,6 +24,7 @@ struct InterpException
     const CorJitResult m_result;
 };
 
+class InterpreterStackMap;
 class InterpCompiler;
 
 class InterpDataItemIndexMap
@@ -501,6 +502,9 @@ class InterpCompiler
     void PrintNameInPointerMap(void* ptr);
 #endif // DEBUG
 
+    dn_simdhash_ptr_ptr_holder m_stackmapsByClass;
+    InterpreterStackMap* GetInterpreterStackMap(CORINFO_CLASS_HANDLE classHandle);
+
     static int32_t InterpGetMovForType(InterpType interpType, bool signExtend);
 
     uint8_t* m_ip;

src/coreclr/interpreter/compileropt.cpp

@@ -1,6 +1,7 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 #include "interpreter.h"
+#include "stackmap.h"
 
 // Allocates the offset for var at the stack position identified by
 // *pPos while bumping the pointer to point to the next stack location
@@ -269,7 +270,7 @@ void InterpCompiler::AllocOffsets()
 
                             int32_t opcode = InterpGetMovForType(m_pVars[newVar].interpType, false);
                             InterpInst *newInst = InsertInsBB(pBB, pIns->pPrev, opcode);
-                            // The InsertInsBB assigns m_currentILOffset to ins->ilOffset, which is incorrect for 
+                            // The InsertInsBB assigns m_currentILOffset to ins->ilOffset, which is incorrect for
                             // instructions injected here. Copy the ilOffset from the call instruction instead.
                             newInst->ilOffset = pIns->ilOffset;
 
@@ -421,7 +422,7 @@ void InterpCompiler::AllocOffsets()
                 (pVar->interpType == InterpTypeByRef) ||
                 (
                     (pVar->interpType == InterpTypeVT) &&
-                    (m_compHnd->getClassAttribs(pVar->clsHnd) & CORINFO_FLG_CONTAINS_GC_PTR)
+                    (GetInterpreterStackMap(pVar->clsHnd)->m_slotCount > 0)
                 )
             )
         )

src/coreclr/interpreter/simdhash.h

@@ -11,10 +11,26 @@
 
 class dn_simdhash_ptr_ptr_holder
 {
+public:
+    dn_simdhash_ptr_ptr_foreach_func ValueDestroyCallback;
+
+private:
     dn_simdhash_ptr_ptr_t *Value;
+
+    void free_hash_and_values()
+    {
+        if (Value == nullptr)
+            return;
+        if (ValueDestroyCallback)
+            dn_simdhash_ptr_ptr_foreach(Value, ValueDestroyCallback, nullptr);
+        dn_simdhash_free(Value);
+        Value = nullptr;
+    }
+
 public:
-    dn_simdhash_ptr_ptr_holder() :
-        Value(nullptr)
+    dn_simdhash_ptr_ptr_holder(dn_simdhash_ptr_ptr_foreach_func valueDestroyCallback = nullptr)
+        : ValueDestroyCallback(valueDestroyCallback)
+        , Value(nullptr)
     {
     }
 
@@ -36,8 +52,7 @@ class dn_simdhash_ptr_ptr_holder
     {
         if (this != &other)
         {
-            if (Value != nullptr)
-                dn_simdhash_free(Value);
+            free_hash_and_values();
             Value = other.Value;
             other.Value = nullptr;
         }
@@ -46,8 +61,7 @@ class dn_simdhash_ptr_ptr_holder
 
     ~dn_simdhash_ptr_ptr_holder()
     {
-        if (Value != nullptr)
-            dn_simdhash_free(Value);
+        free_hash_and_values();
     }
 };
 

src/coreclr/interpreter/stackmap.cpp

@@ -18,24 +18,6 @@ dn_simdhash_assert_fail (const char* file, int line, const char* condition) {
 #endif
 }
 
-thread_local dn_simdhash_ptr_ptr_t *t_sharedStackMapLookup = nullptr;
-
-InterpreterStackMap* GetInterpreterStackMap(ICorJitInfo* jitInfo, CORINFO_CLASS_HANDLE classHandle)
-{
-    InterpreterStackMap* result = nullptr;
-    if (!t_sharedStackMapLookup)
-        t_sharedStackMapLookup = dn_simdhash_ptr_ptr_new(0, nullptr);
-    if (!t_sharedStackMapLookup)
-        NOMEM();
-
-    if (!dn_simdhash_ptr_ptr_try_get_value(t_sharedStackMapLookup, classHandle, (void **)&result))
-    {
-        result = new InterpreterStackMap(jitInfo, classHandle);
-        checkAddedNew(dn_simdhash_ptr_ptr_try_add(t_sharedStackMapLookup, classHandle, result));
-    }
-    return result;
-}
-
 void InterpreterStackMap::PopulateStackMap(ICorJitInfo* jitInfo, CORINFO_CLASS_HANDLE classHandle)
 {
     unsigned size = jitInfo->getClassSize(classHandle);

src/coreclr/interpreter/stackmap.h

@@ -21,6 +21,11 @@ class InterpreterStackMap
     {
         PopulateStackMap(jitInfo, classHandle);
     }
-};
 
-InterpreterStackMap* GetInterpreterStackMap(ICorJitInfo* jitInfo, CORINFO_CLASS_HANDLE classHandle);
+    ~InterpreterStackMap ()
+    {
+        if (m_slots)
+            free(m_slots);
+        m_slots = nullptr;
+    }
+};