Allocate boxed static structs on Frozen Object Heap and remove getFie…

…ldAddress (dotnet#77737) Co-authored-by: Jan Kotas <jkotas@microsoft.com>
EgorBo · Nov 13, 2022 · cabb8b0 · cabb8b0
1 parent cc712fd
commit cabb8b0
Show file tree

Hide file tree

Showing 39 changed files with 293 additions and 515 deletions.
diff --git a/src/coreclr/inc/CrstTypes.def b/src/coreclr/inc/CrstTypes.def
@@ -577,3 +577,7 @@ End
 Crst PgoData
     AcquiredBefore LoaderHeap
 End
+
+Crst StaticBoxInit
+    AcquiredBefore LoaderHeap FrozenObjectHeap
+End
diff --git a/src/coreclr/inc/corinfo.h b/src/coreclr/inc/corinfo.h
@@ -141,7 +141,7 @@ The first 4 options are mutually exclusive
         have managed thread local statics, which work through the HELPER. Support for this is considered
         legacy, and going forward, the EE should
 
-    * <NONE> This is a normal static field. Its address in memory is determined by getFieldAddress. (see
+    * <NONE> This is a normal static field. Its address in memory is determined by getFieldInfo. (see
         also CORINFO_FLG_STATIC_IN_HEAP).
 
 
@@ -1719,7 +1719,7 @@ struct CORINFO_FIELD_INFO
     CorInfoIsAccessAllowedResult accessAllowed;
     CORINFO_HELPER_DESC     accessCalloutHelper;
 
-    CORINFO_CONST_LOOKUP    fieldLookup;        // Used by Ready-to-Run
+    CORINFO_CONST_LOOKUP    fieldLookup;
 };
 
 //----------------------------------------------------------------------------
@@ -3200,13 +3200,6 @@ class ICorDynamicInfo : public ICorStaticInfo
                     void                  **ppIndirection = NULL
                     ) = 0;
 
-
-    // return the data's address (for static fields only)
-    virtual void* getFieldAddress(
-                    CORINFO_FIELD_HANDLE    field,
-                    void                  **ppIndirection = NULL
-                    ) = 0;
-
     //------------------------------------------------------------------------------
     // getReadonlyStaticFieldValue: returns true and the actual field's value if the given
     //    field represents a statically initialized readonly field of any type.

diff --git a/src/coreclr/inc/crsttypes_generated.h b/src/coreclr/inc/crsttypes_generated.h
@@ -112,28 +112,29 @@ enum CrstType
     CrstSingleUseLock = 94,
     CrstSpecialStatics = 95,
     CrstStackSampler = 96,
-    CrstStressLog = 97,
-    CrstStubCache = 98,
-    CrstStubDispatchCache = 99,
-    CrstStubUnwindInfoHeapSegments = 100,
-    CrstSyncBlockCache = 101,
-    CrstSyncHashLock = 102,
-    CrstSystemBaseDomain = 103,
-    CrstSystemDomain = 104,
-    CrstSystemDomainDelayedUnloadList = 105,
-    CrstThreadIdDispenser = 106,
-    CrstThreadStore = 107,
-    CrstTieredCompilation = 108,
-    CrstTypeEquivalenceMap = 109,
-    CrstTypeIDMap = 110,
-    CrstUMEntryThunkCache = 111,
-    CrstUMEntryThunkFreeListLock = 112,
-    CrstUniqueStack = 113,
-    CrstUnresolvedClassLock = 114,
-    CrstUnwindInfoTableLock = 115,
-    CrstVSDIndirectionCellLock = 116,
-    CrstWrapperTemplate = 117,
-    kNumberOfCrstTypes = 118
+    CrstStaticBoxInit = 97,
+    CrstStressLog = 98,
+    CrstStubCache = 99,
+    CrstStubDispatchCache = 100,
+    CrstStubUnwindInfoHeapSegments = 101,
+    CrstSyncBlockCache = 102,
+    CrstSyncHashLock = 103,
+    CrstSystemBaseDomain = 104,
+    CrstSystemDomain = 105,
+    CrstSystemDomainDelayedUnloadList = 106,
+    CrstThreadIdDispenser = 107,
+    CrstThreadStore = 108,
+    CrstTieredCompilation = 109,
+    CrstTypeEquivalenceMap = 110,
+    CrstTypeIDMap = 111,
+    CrstUMEntryThunkCache = 112,
+    CrstUMEntryThunkFreeListLock = 113,
+    CrstUniqueStack = 114,
+    CrstUnresolvedClassLock = 115,
+    CrstUnwindInfoTableLock = 116,
+    CrstVSDIndirectionCellLock = 117,
+    CrstWrapperTemplate = 118,
+    kNumberOfCrstTypes = 119
 };
 
 #endif // __CRST_TYPES_INCLUDED
@@ -241,6 +242,7 @@ int g_rgCrstLevelMap[] =
     5,          // CrstSingleUseLock
     0,          // CrstSpecialStatics
     0,          // CrstStackSampler
+    -1,         // CrstStaticBoxInit
     -1,         // CrstStressLog
     5,          // CrstStubCache
     0,          // CrstStubDispatchCache
@@ -364,6 +366,7 @@ LPCSTR g_rgCrstNameMap[] =
     "CrstSingleUseLock",
     "CrstSpecialStatics",
     "CrstStackSampler",
+    "CrstStaticBoxInit",
     "CrstStressLog",
     "CrstStubCache",
     "CrstStubDispatchCache",

diff --git a/src/coreclr/inc/icorjitinfoimpl_generated.h b/src/coreclr/inc/icorjitinfoimpl_generated.h
@@ -616,10 +616,6 @@ unsigned getClassDomainID(
           CORINFO_CLASS_HANDLE cls,
           void** ppIndirection) override;
 
-void* getFieldAddress(
-          CORINFO_FIELD_HANDLE field,
-          void** ppIndirection) override;
-
 bool getReadonlyStaticFieldValue(
           CORINFO_FIELD_HANDLE field,
           uint8_t* buffer,

diff --git a/src/coreclr/inc/jiteeversionguid.h b/src/coreclr/inc/jiteeversionguid.h
@@ -43,11 +43,11 @@ typedef const GUID *LPCGUID;
 #define GUID_DEFINED
 #endif // !GUID_DEFINED
 
-constexpr GUID JITEEVersionIdentifier = { /* e452af1d-0a1a-44a8-a5b3-ef6074b8ab4a */
-    0xe452af1d,
-    0x0a1a,
-    0x44a8,
-    {0xa5, 0xb3, 0xef, 0x60, 0x74, 0xb8, 0xab, 0x4a}
+constexpr GUID JITEEVersionIdentifier = { /* c0c94f6c-5358-4a3d-be83-b2a9d1b2545e */
+    0xc0c94f6c,
+    0x5358,
+    0x4a3d,
+    {0xbe, 0x83, 0xb2, 0xa9, 0xd1, 0xb2, 0x54, 0x5e}
   };
 
 //////////////////////////////////////////////////////////////////////////////////////////////////////////

diff --git a/src/coreclr/jit/ICorJitInfo_names_generated.h b/src/coreclr/jit/ICorJitInfo_names_generated.h
@@ -155,7 +155,6 @@ DEF_CLR_API(getCallInfo)
 DEF_CLR_API(canAccessFamily)
 DEF_CLR_API(isRIDClassDomainID)
 DEF_CLR_API(getClassDomainID)
-DEF_CLR_API(getFieldAddress)
 DEF_CLR_API(getReadonlyStaticFieldValue)
 DEF_CLR_API(getStaticFieldCurrentClass)
 DEF_CLR_API(getVarArgsHandle)

diff --git a/src/coreclr/jit/ICorJitInfo_wrapper_generated.hpp b/src/coreclr/jit/ICorJitInfo_wrapper_generated.hpp
@@ -1479,16 +1479,6 @@ unsigned WrapICorJitInfo::getClassDomainID(
     return temp;
 }
 
-void* WrapICorJitInfo::getFieldAddress(
-          CORINFO_FIELD_HANDLE field,
-          void** ppIndirection)
-{
-    API_ENTER(getFieldAddress);
-    void* temp = wrapHnd->getFieldAddress(field, ppIndirection);
-    API_LEAVE(getFieldAddress);
-    return temp;
-}
-
 bool WrapICorJitInfo::getReadonlyStaticFieldValue(
           CORINFO_FIELD_HANDLE field,
           uint8_t* buffer,

diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp
@@ -3197,8 +3197,7 @@ GenTree* Compiler::optVNConstantPropOnTree(BasicBlock* block, GenTree* tree)
 
         case TYP_SIMD32:
         {
-            simd32_t value = vnStore->ConstantValue<simd32_t>(vnCns);
-
+            simd32_t       value  = vnStore->ConstantValue<simd32_t>(vnCns);
             GenTreeVecCon* vecCon = gtNewVconNode(tree->TypeGet());
             vecCon->gtSimd32Val   = value;
 

diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h
@@ -5733,7 +5733,6 @@ class Compiler
     GenTree* fgMorphField(GenTree* tree, MorphAddrContext* mac);
     GenTree* fgMorphExpandInstanceField(GenTree* tree, MorphAddrContext* mac);
     GenTree* fgMorphExpandTlsFieldAddr(GenTree* tree);
-    GenTree* fgMorphExpandStaticField(GenTree* tree);
     bool fgCanFastTailCall(GenTreeCall* call, const char** failReason);
 #if FEATURE_FASTTAILCALL
     bool fgCallHasMustCopyByrefParameter(GenTreeCall* callee);

diff --git a/src/coreclr/jit/emitarm.cpp b/src/coreclr/jit/emitarm.cpp
@@ -4012,10 +4012,7 @@ void emitter::emitIns_R_C(instruction ins, emitAttr attr, regNumber reg, CORINFO
     }
     else
     {
-        assert(!jitStaticFldIsGlobAddr(fldHnd));
-        addr = (ssize_t)emitComp->info.compCompHnd->getFieldAddress(fldHnd, NULL);
-        if (addr == NULL)
-            NO_WAY("could not obtain address of static field");
+        assert(!"Normal statics are expected to be handled in the importer");
     }
 
     // We can use reg to load the constant address,

diff --git a/src/coreclr/jit/emitxarch.cpp b/src/coreclr/jit/emitxarch.cpp
@@ -12977,18 +12977,8 @@ BYTE* emitter::emitOutputCV(BYTE* dst, instrDesc* id, code_t code, CnsVal* addc)
     else
     {
         // Special case: mov reg, fs:[ddd] or mov reg, [ddd]
-        if (jitStaticFldIsGlobAddr(fldh))
-        {
-            addr = nullptr;
-        }
-        else
-        {
-            addr = (BYTE*)emitComp->info.compCompHnd->getFieldAddress(fldh, nullptr);
-            if (addr == nullptr)
-            {
-                NO_WAY("could not obtain address of static field");
-            }
-        }
+        assert(jitStaticFldIsGlobAddr(fldh));
+        addr = nullptr;
     }
 
     BYTE* target = (addr + offs);

diff --git a/src/coreclr/jit/fginline.cpp b/src/coreclr/jit/fginline.cpp
@@ -1608,7 +1608,8 @@ Statement* Compiler::fgInlinePrependStatements(InlineInfo* inlineInfo)
                             GenTree* op2 = argNode->AsOp()->gtOp2;
                             if (op1->IsCall() &&
                                 ((op1->AsCall()->gtCallMoreFlags & GTF_CALL_M_HELPER_SPECIAL_DCE) != 0) &&
-                                (op2->gtOper == GT_FIELD) && ((op2->gtFlags & GTF_EXCEPT) == 0))
+                                op2->OperIs(GT_IND) && op2->gtGetOp1()->IsIconHandle() &&
+                                ((op2->gtFlags & GTF_EXCEPT) == 0))
                             {
                                 JITDUMP("\nPerforming special dce on unused arg [%06u]:"
                                         " actual arg [%06u] helper call [%06u]\n",

diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp
@@ -18079,6 +18079,16 @@ CORINFO_CLASS_HANDLE Compiler::gtGetClassHandle(GenTree* tree, bool* pIsExact, b
                         }
                     }
                 }
+                else if (base->IsIconHandle(GTF_ICON_CONST_PTR, GTF_ICON_STATIC_HDL))
+                {
+                    // Check if we have IND(ICON_HANDLE) that represents a static field
+                    FieldSeq* fldSeq = base->AsIntCon()->gtFieldSeq;
+                    if ((fldSeq != nullptr) && (fldSeq->GetOffset() == base->AsIntCon()->IconValue()))
+                    {
+                        CORINFO_FIELD_HANDLE fldHandle = base->AsIntCon()->gtFieldSeq->GetFieldHandle();
+                        objClass                       = gtGetFieldClassHandle(fldHandle, pIsExact, pIsNonNull);
+                    }
+                }
             }
 
             break;

diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h
@@ -2209,6 +2209,12 @@ struct GenTree
         return (gtOper == GT_CNS_INT) && ((gtFlags & GTF_ICON_HDL_MASK) == handleType);
     }
 
+    template <typename... T>
+    bool IsIconHandle(GenTreeFlags handleType, T... rest) const
+    {
+        return IsIconHandle(handleType) || IsIconHandle(rest...);
+    }
+
     // Return just the part of the flags corresponding to the GTF_ICON_*_HDL flag.
     // For non-icon handle trees, returns GTF_EMPTY.
     GenTreeFlags GetIconHandleFlag() const

diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp
@@ -1615,9 +1615,9 @@ GenTree* Compiler::impNormStructVal(GenTree* structVal, CORINFO_CLASS_HANDLE str
                 // In case of a chained GT_COMMA case, we sink the last
                 // GT_COMMA below the blockNode addr.
                 GenTree* blockNodeAddr = blockNode->AsOp()->gtOp1;
-                assert(blockNodeAddr->gtType == TYP_BYREF);
+                assert(blockNodeAddr->TypeIs(TYP_BYREF, TYP_I_IMPL));
                 GenTree* commaNode       = parent;
-                commaNode->gtType        = TYP_BYREF;
+                commaNode->gtType        = blockNodeAddr->gtType;
                 commaNode->AsOp()->gtOp2 = blockNodeAddr;
                 blockNode->AsOp()->gtOp1 = commaNode;
                 if (parent == structVal)
@@ -4374,6 +4374,9 @@ GenTree* Compiler::impImportStaticFieldAccess(CORINFO_RESOLVED_TOKEN* pResolvedT
     FieldSeq::FieldKind fieldKind =
         isSharedStatic ? FieldSeq::FieldKind::SharedStatic : FieldSeq::FieldKind::SimpleStatic;
 
+    bool hasConstAddr = (pFieldInfo->fieldAccessor == CORINFO_FIELD_STATIC_ADDRESS) ||
+                        (pFieldInfo->fieldAccessor == CORINFO_FIELD_STATIC_RVA_ADDRESS);
+
     FieldSeq* innerFldSeq;
     FieldSeq* outerFldSeq;
     if (isBoxedStatic)
@@ -4383,14 +4386,11 @@ GenTree* Compiler::impImportStaticFieldAccess(CORINFO_RESOLVED_TOKEN* pResolvedT
     }
     else
     {
-        bool hasConstAddr = (pFieldInfo->fieldAccessor == CORINFO_FIELD_STATIC_ADDRESS) ||
-                            (pFieldInfo->fieldAccessor == CORINFO_FIELD_STATIC_RVA_ADDRESS);
-
         ssize_t offset;
         if (hasConstAddr)
         {
-            offset = reinterpret_cast<ssize_t>(info.compCompHnd->getFieldAddress(pResolvedToken->hField));
-            assert(offset != 0);
+            assert(pFieldInfo->fieldLookup.accessType == IAT_VALUE);
+            offset = reinterpret_cast<ssize_t>(pFieldInfo->fieldLookup.addr);
         }
         else
         {
@@ -4401,6 +4401,7 @@ GenTree* Compiler::impImportStaticFieldAccess(CORINFO_RESOLVED_TOKEN* pResolvedT
         outerFldSeq = nullptr;
     }
 
+    bool     isStaticReadOnlyInitedRef = false;
     GenTree* op1;
     switch (pFieldInfo->fieldAccessor)
     {
@@ -4493,53 +4494,41 @@ GenTree* Compiler::impImportStaticFieldAccess(CORINFO_RESOLVED_TOKEN* pResolvedT
 
         default:
         {
-            // Do we need the address of a static field?
-            //
-            if (access & CORINFO_ACCESS_ADDRESS)
+// TODO-CQ: enable this optimization for 32 bit targets.
+#ifdef TARGET_64BIT
+            if (!isBoxedStatic && (lclTyp == TYP_REF) && ((access & CORINFO_ACCESS_ADDRESS) == 0))
             {
-                void** pFldAddr = nullptr;
-                void*  fldAddr  = info.compCompHnd->getFieldAddress(pResolvedToken->hField, (void**)&pFldAddr);
-
-                // We should always be able to access this static's address directly.
-                assert(pFldAddr == nullptr);
-
-                // Create the address node.
-                GenTreeFlags handleKind = isBoxedStatic ? GTF_ICON_STATIC_BOX_PTR : GTF_ICON_STATIC_HDL;
-                op1                     = gtNewIconHandleNode((size_t)fldAddr, handleKind, innerFldSeq);
-                INDEBUG(op1->AsIntCon()->gtTargetHandle = reinterpret_cast<size_t>(pResolvedToken->hField));
-
-                if (pFieldInfo->fieldFlags & CORINFO_FLG_FIELD_INITCLASS)
+                bool isSpeculative = true;
+                if ((info.compCompHnd->getStaticFieldCurrentClass(pResolvedToken->hField, &isSpeculative) !=
+                     NO_CLASS_HANDLE))
                 {
-                    op1->gtFlags |= GTF_ICON_INITCLASS;
+                    isStaticReadOnlyInitedRef = !isSpeculative;
                 }
             }
-            else // We need the value of a static field
-            {
-                op1 = gtNewFieldRef(lclTyp, pResolvedToken->hField);
-
-                if (pFieldInfo->fieldFlags & CORINFO_FLG_FIELD_INITCLASS)
-                {
-                    op1->gtFlags |= GTF_FLD_INITCLASS;
-                }
-
-                if (isBoxedStatic)
-                {
-                    op1->ChangeType(TYP_REF); // points at boxed object
-                    op1 = gtNewOperNode(GT_ADD, TYP_BYREF, op1, gtNewIconNode(TARGET_POINTER_SIZE, outerFldSeq));
-
-                    if (varTypeIsStruct(lclTyp))
-                    {
-                        // Constructor adds GTF_GLOB_REF.  Note that this is *not* GTF_EXCEPT.
-                        op1 = gtNewObjNode(pFieldInfo->structType, op1);
-                    }
-                    else
-                    {
-                        op1 = gtNewOperNode(GT_IND, lclTyp, op1);
-                        op1->gtFlags |= (GTF_GLOB_REF | GTF_IND_NONFAULTING);
-                    }
-                }
+#endif // TARGET_64BIT
 
-                return op1;
+            assert(hasConstAddr);
+            assert(pFieldInfo->fieldLookup.addr != nullptr);
+            assert(pFieldInfo->fieldLookup.accessType == IAT_VALUE);
+            size_t       fldAddr = reinterpret_cast<size_t>(pFieldInfo->fieldLookup.addr);
+            GenTreeFlags handleKind;
+            if (isBoxedStatic)
+            {
+                handleKind = GTF_ICON_STATIC_BOX_PTR;
+            }
+            else if (isStaticReadOnlyInitedRef)
+            {
+                handleKind = GTF_ICON_CONST_PTR;
+            }
+            else
+            {
+                handleKind = GTF_ICON_STATIC_HDL;
+            }
+            op1 = gtNewIconHandleNode(fldAddr, handleKind, innerFldSeq);
+            INDEBUG(op1->AsIntCon()->gtTargetHandle = reinterpret_cast<size_t>(pResolvedToken->hField));
+            if (pFieldInfo->fieldFlags & CORINFO_FLG_FIELD_INITCLASS)
+            {
+                op1->gtFlags |= GTF_ICON_INITCLASS;
             }
             break;
         }
@@ -4563,6 +4552,10 @@ GenTree* Compiler::impImportStaticFieldAccess(CORINFO_RESOLVED_TOKEN* pResolvedT
             op1 = gtNewOperNode(GT_IND, lclTyp, op1);
             op1->gtFlags |= GTF_GLOB_REF;
         }
+        if (isStaticReadOnlyInitedRef)
+        {
+            op1->gtFlags |= (GTF_IND_INVARIANT | GTF_IND_NONFAULTING | GTF_IND_NONNULL);
+        }
     }
 
     return op1;