Address PR feedback

Author: kg

src/coreclr/interpreter/compiler.cpp

@@ -1046,7 +1046,7 @@ int32_t* InterpCompiler::GetCode(int32_t *pCodeSize)
 InterpCompiler::InterpCompiler(COMP_HANDLE compHnd,
                                 CORINFO_METHOD_INFO* methodInfo)
     : m_pInitLocalsIns(nullptr)
-    , m_globalVarsStackTop(0)
+    , m_globalVarsWithRefsStackTop(0)
 {
     // Fill in the thread-local used for assertions
     t_InterpJitInfoTls = compHnd;
@@ -1108,12 +1108,11 @@ InterpMethod* InterpCompiler::CompileMethod()
 void InterpCompiler::PatchInitLocals(CORINFO_METHOD_INFO* methodInfo)
 {
     // We may have global vars containing managed pointers or interior pointers, so we need
-    //  to zero the region of the stack containing global vars, not just IL locals.
-    // data[0] is either the start of IL locals or the end of IL locals depending on whether local
-    //  zeroing was enabled for this method. We want to preserve that so we don't unnecessarily
-    //  zero the IL locals if the method's author didn't want them zeroed
+    //  to zero the region of the stack containing global vars, not just IL locals. Now that
+    //  offset allocation has occurred we know where the global vars end, so we can expand
+    //  the initlocals opcode that was originally generated to also zero them.
     int32_t startOffset = m_pInitLocalsIns->data[0];
-    int32_t totalSize = m_globalVarsStackTop - startOffset;
+    int32_t totalSize = m_globalVarsWithRefsStackTop - startOffset;
     if (totalSize > m_pInitLocalsIns->data[1])
     {
         INTERP_DUMP(
@@ -1128,7 +1127,7 @@ void InterpCompiler::PatchInitLocals(CORINFO_METHOD_INFO* methodInfo)
         INTERP_DUMP(
             "Not expanding initlocals from [%d-%d] for global vars stack top of %d\n",
             startOffset, startOffset + m_pInitLocalsIns->data[1],
-            m_globalVarsStackTop
+            m_globalVarsWithRefsStackTop
         );
     }
 }

src/coreclr/interpreter/compiler.h

@@ -359,7 +359,7 @@ class InterpCompiler
 
     int32_t CreateVarExplicit(InterpType interpType, CORINFO_CLASS_HANDLE clsHnd, int size);
 
-    int32_t m_totalVarsStackSize, m_globalVarsStackTop;
+    int32_t m_totalVarsStackSize, m_globalVarsWithRefsStackTop;
     int32_t m_paramAreaOffset = 0;
     int32_t m_ILLocalsOffset, m_ILLocalsSize;
     void    AllocVarOffsetCB(int *pVar, void *pData);

src/coreclr/interpreter/compileropt.cpp

@@ -227,7 +227,7 @@ void InterpCompiler::AllocOffsets()
     INTERP_DUMP("\nAllocating var offsets\n");
 
     int finalVarsStackSize = m_totalVarsStackSize,
-        globalVarsStackTop = m_totalVarsStackSize;
+        globalVarsWithRefsStackTop = m_totalVarsStackSize;
 
     // We now have the top of stack offset. All local regs are allocated after this offset, with each basic block
     for (pBB = m_pEntryBB; pBB != NULL; pBB = pBB->pNextBB)
@@ -415,16 +415,19 @@ void InterpCompiler::AllocOffsets()
             pVar->global && (
                 (pVar->interpType == InterpTypeO) ||
                 (pVar->interpType == InterpTypeByRef) ||
-                (pVar->interpType == InterpTypeVT)
+                (
+                    (pVar->interpType == InterpTypeVT) &&
+                    (m_compHnd->getClassAttribs(pVar->clsHnd) & CORINFO_FLG_CONTAINS_GC_PTR)
+                )
             )
         )
         {
             int32_t endOfVar = pVar->offset + pVar->size;
-            if (endOfVar > globalVarsStackTop)
-                globalVarsStackTop = endOfVar;
+            if (endOfVar > globalVarsWithRefsStackTop)
+                globalVarsWithRefsStackTop = endOfVar;
         }
     }
 
-    m_globalVarsStackTop = globalVarsStackTop;
+    m_globalVarsWithRefsStackTop = globalVarsWithRefsStackTop;
     m_totalVarsStackSize = ALIGN_UP_TO(finalVarsStackSize, INTERP_STACK_ALIGNMENT);
 }