From 77a42b77df0b90d250588938f199a73710d9b8ac Mon Sep 17 00:00:00 2001 From: EgorBo Date: Tue, 1 Nov 2022 21:11:57 +0200 Subject: [PATCH 01/33] Smarter approach suggested by SingleAccretion --- src/coreclr/vm/jitinterface.cpp | 17 ++++++++++++++++- src/coreclr/vm/methodtable.cpp | 21 ++++++++++++++++++--- src/coreclr/vm/methodtable.h | 2 +- 3 files changed, 35 insertions(+), 5 deletions(-) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 3d0012cbd4119..1afd33529629d 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -1514,7 +1514,22 @@ void CEEInfo::getFieldInfo (CORINFO_RESOLVED_TOKEN * pResolvedToken, CORINFO_FIELD_ACCESSOR intrinsicAccessor; if (pField->GetFieldType() == ELEMENT_TYPE_VALUETYPE) - fieldFlags |= CORINFO_FLG_FIELD_STATIC_IN_HEAP; + { + bool frozenBoxedStatic = false; + if (pFieldMT->ContainsPointersOrCollectible()) + { + Object** handle = (Object**)pField->GetStaticAddressHandle(pField->GetBase()); + if (*handle == nullptr && GCHeapUtilities::GetGCHeap()->IsInFrozenSegment(*handle)) + { + frozenBoxedStatic = true; + } + } + + if (!frozenBoxedStatic) + { + fieldFlags |= CORINFO_FLG_FIELD_STATIC_IN_HEAP; + } + } if (pFieldMT->IsSharedByGenericInstantiations()) { diff --git a/src/coreclr/vm/methodtable.cpp b/src/coreclr/vm/methodtable.cpp index d42d4e3efc145..7c0a6385440c9 100644 --- a/src/coreclr/vm/methodtable.cpp +++ b/src/coreclr/vm/methodtable.cpp @@ -58,6 +58,7 @@ #include "array.h" #include "castcache.h" #include "dynamicinterfacecastable.h" +#include "frozenobjectheap.h" #ifdef FEATURE_INTERPRETER #include "interpreter.h" @@ -3499,7 +3500,7 @@ void MethodTable::AllocateRegularStaticBoxes() MethodTable* pFieldMT = th.GetMethodTable(); LOG((LF_CLASSLOADER, LL_INFO10000, "\tInstantiating static of type %s\n", pFieldMT->GetDebugClassName())); - OBJECTREF obj = AllocateStaticBox(pFieldMT, HasFixedAddressVTStatics()); + OBJECTREF obj = AllocateStaticBox(pFieldMT, HasFixedAddressVTStatics(), NULL, !pFieldMT->ContainsPointersOrCollectible()); SetObjectReference( (OBJECTREF*)(pStaticBase + pField->GetOffset()), obj); } @@ -3511,7 +3512,7 @@ void MethodTable::AllocateRegularStaticBoxes() } //========================================================================================== -OBJECTREF MethodTable::AllocateStaticBox(MethodTable* pFieldMT, BOOL fPinned, OBJECTHANDLE* pHandle) +OBJECTREF MethodTable::AllocateStaticBox(MethodTable* pFieldMT, BOOL fPinned, OBJECTHANDLE* pHandle, bool neverCollected) { CONTRACTL { @@ -3526,7 +3527,21 @@ OBJECTREF MethodTable::AllocateStaticBox(MethodTable* pFieldMT, BOOL fPinned, OB // Activate any dependent modules if necessary pFieldMT->EnsureInstanceActive(); - OBJECTREF obj = AllocateObject(pFieldMT); + OBJECTREF obj = NULL; + if (neverCollected) + { + // In case if we don't plan to collect this handle we may try to allocate it on FOH + _ASSERT(!pFieldMT->ContainsPointersOrCollectible()); + FrozenObjectHeapManager* foh = SystemDomain::GetFrozenObjectHeapManager(); + obj = ObjectToOBJECTREF(foh->TryAllocateObject(pFieldMT, pFieldMT->GetBaseSize())); + // obj can be null in case if struct is huge (>64kb) + if (obj != NULL) + { + return obj; + } + } + + obj = AllocateObject(pFieldMT); // Pin the object if necessary if (fPinned) diff --git a/src/coreclr/vm/methodtable.h b/src/coreclr/vm/methodtable.h index a378ec69ad5d0..2c7263143250b 100644 --- a/src/coreclr/vm/methodtable.h +++ b/src/coreclr/vm/methodtable.h @@ -831,7 +831,7 @@ class MethodTable // instantiations of the superclass or interfaces e.g. System.Int32 : IComparable void AllocateRegularStaticBoxes(); - static OBJECTREF AllocateStaticBox(MethodTable* pFieldMT, BOOL fPinned, OBJECTHANDLE* pHandle = 0); + static OBJECTREF AllocateStaticBox(MethodTable* pFieldMT, BOOL fPinned, OBJECTHANDLE* pHandle = 0, bool neverCollected = false); void CheckRestore(); From 2b2b2ec6bd96e2d139a9f22916f2efebe0ddc9f9 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Tue, 1 Nov 2022 21:29:56 +0200 Subject: [PATCH 02/33] Address Jan's feedback, Jakob noticed invalid contract in AllocateStaticBoxes --- src/coreclr/vm/methodtable.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/coreclr/vm/methodtable.cpp b/src/coreclr/vm/methodtable.cpp index 7c0a6385440c9..ccd3ad778a1d1 100644 --- a/src/coreclr/vm/methodtable.cpp +++ b/src/coreclr/vm/methodtable.cpp @@ -3499,8 +3499,11 @@ void MethodTable::AllocateRegularStaticBoxes() TypeHandle th = pField->GetFieldTypeHandleThrowing(); MethodTable* pFieldMT = th.GetMethodTable(); + bool neverCollected = !pFieldMT->ContainsPointersOrCollectible() && + !pField->GetApproxEnclosingMethodTable()->Collectible(); + LOG((LF_CLASSLOADER, LL_INFO10000, "\tInstantiating static of type %s\n", pFieldMT->GetDebugClassName())); - OBJECTREF obj = AllocateStaticBox(pFieldMT, HasFixedAddressVTStatics(), NULL, !pFieldMT->ContainsPointersOrCollectible()); + OBJECTREF obj = AllocateStaticBox(pFieldMT, HasFixedAddressVTStatics(), NULL, neverCollected); SetObjectReference( (OBJECTREF*)(pStaticBase + pField->GetOffset()), obj); } @@ -3518,7 +3521,7 @@ OBJECTREF MethodTable::AllocateStaticBox(MethodTable* pFieldMT, BOOL fPinned, OB { THROWS; GC_TRIGGERS; - MODE_ANY; + MODE_COOPERATIVE; CONTRACTL_END; } From b524f0d67c4a17808326b7a2a965143d98979d63 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Wed, 2 Nov 2022 02:49:40 +0200 Subject: [PATCH 03/33] Update src/coreclr/vm/methodtable.cpp Co-authored-by: Jan Kotas --- src/coreclr/vm/methodtable.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/coreclr/vm/methodtable.cpp b/src/coreclr/vm/methodtable.cpp index ccd3ad778a1d1..9ad9ab2c47d11 100644 --- a/src/coreclr/vm/methodtable.cpp +++ b/src/coreclr/vm/methodtable.cpp @@ -3499,9 +3499,8 @@ void MethodTable::AllocateRegularStaticBoxes() TypeHandle th = pField->GetFieldTypeHandleThrowing(); MethodTable* pFieldMT = th.GetMethodTable(); - bool neverCollected = !pFieldMT->ContainsPointersOrCollectible() && - !pField->GetApproxEnclosingMethodTable()->Collectible(); - + bool neverCollected = !pFieldMT->ContainsPointers() && + !Collectible(); LOG((LF_CLASSLOADER, LL_INFO10000, "\tInstantiating static of type %s\n", pFieldMT->GetDebugClassName())); OBJECTREF obj = AllocateStaticBox(pFieldMT, HasFixedAddressVTStatics(), NULL, neverCollected); From a96a66495d946005402c858d3227a465871e6a8d Mon Sep 17 00:00:00 2001 From: EgorBo Date: Wed, 2 Nov 2022 02:53:40 +0200 Subject: [PATCH 04/33] Address feedback --- src/coreclr/vm/jitinterface.cpp | 57 +++++++++++++++++++++++++-------- 1 file changed, 44 insertions(+), 13 deletions(-) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 1afd33529629d..ac14c17d69228 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -1453,6 +1453,33 @@ static CorInfoHelpFunc getInstanceFieldHelper(FieldDesc * pField, CORINFO_ACCESS return (CorInfoHelpFunc)helper; } +static OBJECTREF getFrozenBoxedStatic(FieldDesc* field) +{ + CONTRACTL { + THROWS; + GC_TRIGGERS; + MODE_COOPERATIVE; + } CONTRACTL_END; + + // These fields never hold frozen boxed statics + if (!field->IsStatic() || !field->IsByValue() || field->IsSpecialStatic()) + { + return NULL; + } + + TypeHandle typeHandle = field->GetFieldTypeHandleThrowing(); + MethodTable* pFieldMT = typeHandle.GetMethodTable(); + if (!typeHandle.IsCanonicalSubtype() && !pFieldMT->ContainsPointers()) + { + Object** handle = (Object**)field->GetStaticAddressHandle(field->GetBase()); + if (handle != nullptr && GCHeapUtilities::GetGCHeap()->IsInFrozenSegment(*handle)) + { + return ObjectToOBJECTREF(*handle); + } + } + return NULL; +} + /*********************************************************************/ void CEEInfo::getFieldInfo (CORINFO_RESOLVED_TOKEN * pResolvedToken, CORINFO_METHOD_HANDLE callerHandle, @@ -1515,17 +1542,8 @@ void CEEInfo::getFieldInfo (CORINFO_RESOLVED_TOKEN * pResolvedToken, if (pField->GetFieldType() == ELEMENT_TYPE_VALUETYPE) { - bool frozenBoxedStatic = false; - if (pFieldMT->ContainsPointersOrCollectible()) - { - Object** handle = (Object**)pField->GetStaticAddressHandle(pField->GetBase()); - if (*handle == nullptr && GCHeapUtilities::GetGCHeap()->IsInFrozenSegment(*handle)) - { - frozenBoxedStatic = true; - } - } - - if (!frozenBoxedStatic) + GCX_COOP(); + if (getFrozenBoxedStatic(pField) == NULL) { fieldFlags |= CORINFO_FLG_FIELD_STATIC_IN_HEAP; } @@ -11965,10 +11983,23 @@ void* CEEJitInfo::getFieldAddress(CORINFO_FIELD_HANDLE fieldHnd, GCX_COOP(); - base = (void *) field->GetBase(); + // Check if the field holds a frozen boxed static, return its content in that case + result = OBJECTREFToObject(getFrozenBoxedStatic(field)); + if (result != NULL) + { + // Skip pMT + result = (uint8_t*)result + sizeof(void*); + } + else + { + base = (void*)field->GetBase(); + } } - result = field->GetStaticAddressHandle(base); + if (result == NULL) + { + result = field->GetStaticAddressHandle(base); + } EE_TO_JIT_TRANSITION(); From 7ebf21471fbe519d004077555203c0dcf7baaa85 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Wed, 2 Nov 2022 02:56:21 +0200 Subject: [PATCH 05/33] Clean up --- src/coreclr/vm/methodtable.cpp | 12 ++++++------ src/coreclr/vm/methodtable.h | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/coreclr/vm/methodtable.cpp b/src/coreclr/vm/methodtable.cpp index 9ad9ab2c47d11..5a9bf92d81cb8 100644 --- a/src/coreclr/vm/methodtable.cpp +++ b/src/coreclr/vm/methodtable.cpp @@ -3499,10 +3499,10 @@ void MethodTable::AllocateRegularStaticBoxes() TypeHandle th = pField->GetFieldTypeHandleThrowing(); MethodTable* pFieldMT = th.GetMethodTable(); - bool neverCollected = !pFieldMT->ContainsPointers() && - !Collectible(); LOG((LF_CLASSLOADER, LL_INFO10000, "\tInstantiating static of type %s\n", pFieldMT->GetDebugClassName())); - OBJECTREF obj = AllocateStaticBox(pFieldMT, HasFixedAddressVTStatics(), NULL, neverCollected); + + bool canBeFrozen = !pFieldMT->ContainsPointers() && !Collectible(); + OBJECTREF obj = AllocateStaticBox(pFieldMT, HasFixedAddressVTStatics(), NULL, canBeFrozen); SetObjectReference( (OBJECTREF*)(pStaticBase + pField->GetOffset()), obj); } @@ -3514,7 +3514,7 @@ void MethodTable::AllocateRegularStaticBoxes() } //========================================================================================== -OBJECTREF MethodTable::AllocateStaticBox(MethodTable* pFieldMT, BOOL fPinned, OBJECTHANDLE* pHandle, bool neverCollected) +OBJECTREF MethodTable::AllocateStaticBox(MethodTable* pFieldMT, BOOL fPinned, OBJECTHANDLE* pHandle, bool canBeFrozen) { CONTRACTL { @@ -3530,10 +3530,10 @@ OBJECTREF MethodTable::AllocateStaticBox(MethodTable* pFieldMT, BOOL fPinned, OB pFieldMT->EnsureInstanceActive(); OBJECTREF obj = NULL; - if (neverCollected) + if (canBeFrozen) { // In case if we don't plan to collect this handle we may try to allocate it on FOH - _ASSERT(!pFieldMT->ContainsPointersOrCollectible()); + _ASSERT(!pFieldMT->ContainsPointers()); FrozenObjectHeapManager* foh = SystemDomain::GetFrozenObjectHeapManager(); obj = ObjectToOBJECTREF(foh->TryAllocateObject(pFieldMT, pFieldMT->GetBaseSize())); // obj can be null in case if struct is huge (>64kb) diff --git a/src/coreclr/vm/methodtable.h b/src/coreclr/vm/methodtable.h index 2c7263143250b..de4808ecc80eb 100644 --- a/src/coreclr/vm/methodtable.h +++ b/src/coreclr/vm/methodtable.h @@ -831,7 +831,7 @@ class MethodTable // instantiations of the superclass or interfaces e.g. System.Int32 : IComparable void AllocateRegularStaticBoxes(); - static OBJECTREF AllocateStaticBox(MethodTable* pFieldMT, BOOL fPinned, OBJECTHANDLE* pHandle = 0, bool neverCollected = false); + static OBJECTREF AllocateStaticBox(MethodTable* pFieldMT, BOOL fPinned, OBJECTHANDLE* pHandle = 0, bool canBeFrozen = false); void CheckRestore(); From ec21c92b1f6f778b16fa0eaa5b864071ce1eb7a6 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Wed, 2 Nov 2022 04:50:01 +0200 Subject: [PATCH 06/33] give up if GetBase() returns nullptr --- src/coreclr/vm/jitinterface.cpp | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index ac14c17d69228..780105d03d620 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -1469,13 +1469,21 @@ static OBJECTREF getFrozenBoxedStatic(FieldDesc* field) TypeHandle typeHandle = field->GetFieldTypeHandleThrowing(); MethodTable* pFieldMT = typeHandle.GetMethodTable(); - if (!typeHandle.IsCanonicalSubtype() && !pFieldMT->ContainsPointers()) + if (typeHandle.IsCanonicalSubtype() || pFieldMT->ContainsPointers()) { - Object** handle = (Object**)field->GetStaticAddressHandle(field->GetBase()); - if (handle != nullptr && GCHeapUtilities::GetGCHeap()->IsInFrozenSegment(*handle)) - { - return ObjectToOBJECTREF(*handle); - } + return NULL; + } + + PTR_BYTE basePtr = field->GetBase(); + if (basePtr == nullptr) + { + return NULL; + } + + Object** handle = (Object**)field->GetStaticAddressHandle(basePtr); + if (handle != nullptr && GCHeapUtilities::GetGCHeap()->IsInFrozenSegment(*handle)) + { + return ObjectToOBJECTREF(*handle); } return NULL; } From 63e6387550eab8d33112cd194835e7c7047eb44d Mon Sep 17 00:00:00 2001 From: EgorBo Date: Wed, 2 Nov 2022 12:26:45 +0200 Subject: [PATCH 07/33] does this help --- src/coreclr/vm/jitinterface.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 780105d03d620..87cbaebb5ade8 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -1467,9 +1467,16 @@ static OBJECTREF getFrozenBoxedStatic(FieldDesc* field) return NULL; } + PTR_MethodTable fieldOwningMT = field->GetEnclosingMethodTable(); + if (!fieldOwningMT->IsRestored() || fieldOwningMT->GetModuleForStatics() == NULL || + fieldOwningMT->GetDomainLocalModule() == NULL || !fieldOwningMT->IsClassInited()) + { + return NULL; + } + TypeHandle typeHandle = field->GetFieldTypeHandleThrowing(); MethodTable* pFieldMT = typeHandle.GetMethodTable(); - if (typeHandle.IsCanonicalSubtype() || pFieldMT->ContainsPointers()) + if (typeHandle.IsCanonicalSubtype() || pFieldMT->IsSharedByGenericInstantiations() || pFieldMT->ContainsPointers()) { return NULL; } From ca4a77ed067435092cac4eb5e021a8c049912b64 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Wed, 2 Nov 2022 15:08:25 +0200 Subject: [PATCH 08/33] disable for foh forced 8byte alignment 32bit targets --- src/coreclr/vm/frozenobjectheap.cpp | 4 ++++ src/coreclr/vm/jitinterface.cpp | 7 ------- src/coreclr/vm/methodtable.cpp | 9 +++++++++ 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/coreclr/vm/frozenobjectheap.cpp b/src/coreclr/vm/frozenobjectheap.cpp index a1ed1c8b46dc4..8fe40c3cc37b6 100644 --- a/src/coreclr/vm/frozenobjectheap.cpp +++ b/src/coreclr/vm/frozenobjectheap.cpp @@ -37,6 +37,10 @@ Object* FrozenObjectHeapManager::TryAllocateObject(PTR_MethodTable type, size_t _ASSERT(type != nullptr); _ASSERT(FOH_COMMIT_SIZE >= MIN_OBJECT_SIZE); +#ifdef FEATURE_64BIT_ALIGNMENT + _ASSERT(!type->RequiresAlign8()); +#endif + // NOTE: objectSize is expected be the full size including header _ASSERT(objectSize >= MIN_OBJECT_SIZE); diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 87cbaebb5ade8..0b1c2e2fe7b84 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -1467,13 +1467,6 @@ static OBJECTREF getFrozenBoxedStatic(FieldDesc* field) return NULL; } - PTR_MethodTable fieldOwningMT = field->GetEnclosingMethodTable(); - if (!fieldOwningMT->IsRestored() || fieldOwningMT->GetModuleForStatics() == NULL || - fieldOwningMT->GetDomainLocalModule() == NULL || !fieldOwningMT->IsClassInited()) - { - return NULL; - } - TypeHandle typeHandle = field->GetFieldTypeHandleThrowing(); MethodTable* pFieldMT = typeHandle.GetMethodTable(); if (typeHandle.IsCanonicalSubtype() || pFieldMT->IsSharedByGenericInstantiations() || pFieldMT->ContainsPointers()) diff --git a/src/coreclr/vm/methodtable.cpp b/src/coreclr/vm/methodtable.cpp index 5a9bf92d81cb8..c35d56dddd271 100644 --- a/src/coreclr/vm/methodtable.cpp +++ b/src/coreclr/vm/methodtable.cpp @@ -3502,6 +3502,15 @@ void MethodTable::AllocateRegularStaticBoxes() LOG((LF_CLASSLOADER, LL_INFO10000, "\tInstantiating static of type %s\n", pFieldMT->GetDebugClassName())); bool canBeFrozen = !pFieldMT->ContainsPointers() && !Collectible(); + +#ifdef FEATURE_64BIT_ALIGNMENT + if (type->RequiresAlign8()) + { + // 64bit alignment is not yet supported in FOH for 32bit targets + canBeFrozen = false; + } +#endif + OBJECTREF obj = AllocateStaticBox(pFieldMT, HasFixedAddressVTStatics(), NULL, canBeFrozen); SetObjectReference( (OBJECTREF*)(pStaticBase + pField->GetOffset()), obj); From 16a24380b281df0bde497fe4cf123f24be1855da Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Wed, 2 Nov 2022 16:14:49 +0200 Subject: [PATCH 09/33] Update methodtable.cpp --- src/coreclr/vm/methodtable.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/vm/methodtable.cpp b/src/coreclr/vm/methodtable.cpp index c35d56dddd271..9990451e7bada 100644 --- a/src/coreclr/vm/methodtable.cpp +++ b/src/coreclr/vm/methodtable.cpp @@ -3504,7 +3504,7 @@ void MethodTable::AllocateRegularStaticBoxes() bool canBeFrozen = !pFieldMT->ContainsPointers() && !Collectible(); #ifdef FEATURE_64BIT_ALIGNMENT - if (type->RequiresAlign8()) + if (pFieldMT->RequiresAlign8()) { // 64bit alignment is not yet supported in FOH for 32bit targets canBeFrozen = false; From d38e42b518ea6ff3f859e80307cc3c00bbb16fdd Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Wed, 2 Nov 2022 21:29:28 +0200 Subject: [PATCH 10/33] Update src/coreclr/vm/jitinterface.cpp Co-authored-by: Jan Kotas --- src/coreclr/vm/jitinterface.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 0b1c2e2fe7b84..8a263679bdfba 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -1469,7 +1469,7 @@ static OBJECTREF getFrozenBoxedStatic(FieldDesc* field) TypeHandle typeHandle = field->GetFieldTypeHandleThrowing(); MethodTable* pFieldMT = typeHandle.GetMethodTable(); - if (typeHandle.IsCanonicalSubtype() || pFieldMT->IsSharedByGenericInstantiations() || pFieldMT->ContainsPointers()) + if (field->GetEnclosingMethodTable()->IsSharedByGenericInstantiations() || pFieldMT->ContainsPointers()) { return NULL; } From 9868525d05823964df16803e9cfe44ab75998680 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Wed, 2 Nov 2022 21:29:36 +0200 Subject: [PATCH 11/33] Update src/coreclr/vm/jitinterface.cpp Co-authored-by: Jan Kotas --- src/coreclr/vm/jitinterface.cpp | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 8a263679bdfba..352827940cfb9 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -1475,10 +1475,7 @@ static OBJECTREF getFrozenBoxedStatic(FieldDesc* field) } PTR_BYTE basePtr = field->GetBase(); - if (basePtr == nullptr) - { - return NULL; - } + _ASSERTE(basePtr != nullptr); Object** handle = (Object**)field->GetStaticAddressHandle(basePtr); if (handle != nullptr && GCHeapUtilities::GetGCHeap()->IsInFrozenSegment(*handle)) From 4b7ca2ba67954a9f44f9e5c49c9f270ce0f89e7d Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Wed, 2 Nov 2022 21:30:30 +0200 Subject: [PATCH 12/33] Update jitinterface.cpp --- src/coreclr/vm/jitinterface.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 352827940cfb9..c059a5b171121 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -1478,7 +1478,8 @@ static OBJECTREF getFrozenBoxedStatic(FieldDesc* field) _ASSERTE(basePtr != nullptr); Object** handle = (Object**)field->GetStaticAddressHandle(basePtr); - if (handle != nullptr && GCHeapUtilities::GetGCHeap()->IsInFrozenSegment(*handle)) + _ASSERT(handle != nullptr); + if (GCHeapUtilities::GetGCHeap()->IsInFrozenSegment(*handle)) { return ObjectToOBJECTREF(*handle); } From 8c45daa940b9389dea1320eff8bf79986abac700 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Wed, 2 Nov 2022 23:10:23 +0200 Subject: [PATCH 13/33] try fix --- src/coreclr/vm/jitinterface.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index c059a5b171121..f38c3770809e3 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -1467,9 +1467,10 @@ static OBJECTREF getFrozenBoxedStatic(FieldDesc* field) return NULL; } - TypeHandle typeHandle = field->GetFieldTypeHandleThrowing(); - MethodTable* pFieldMT = typeHandle.GetMethodTable(); - if (field->GetEnclosingMethodTable()->IsSharedByGenericInstantiations() || pFieldMT->ContainsPointers()) + MethodTable* fieldType = field->GetFieldTypeHandleThrowing().GetMethodTable(); + MethodTable* owningType = field->GetEnclosingMethodTable(); + if (owningType->IsSharedByGenericInstantiations() || fieldType->ContainsPointers() || + owningType->IsDynamicStatics()) { return NULL; } From 68a130bfb15545a2310dde0b7b7208901538ebd2 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Wed, 2 Nov 2022 23:53:35 +0200 Subject: [PATCH 14/33] Update jitinterface.cpp --- src/coreclr/vm/jitinterface.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index f38c3770809e3..c782ec1409240 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -1469,8 +1469,7 @@ static OBJECTREF getFrozenBoxedStatic(FieldDesc* field) MethodTable* fieldType = field->GetFieldTypeHandleThrowing().GetMethodTable(); MethodTable* owningType = field->GetEnclosingMethodTable(); - if (owningType->IsSharedByGenericInstantiations() || fieldType->ContainsPointers() || - owningType->IsDynamicStatics()) + if (owningType->IsSharedByGenericInstantiations() || fieldType->ContainsPointers()) { return NULL; } From 94ae8f10c4dd1f3f301333a7a1027009f2ef9910 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 4 Nov 2022 18:06:22 +0100 Subject: [PATCH 15/33] Revert to getFieldInfo --- src/coreclr/vm/jitinterface.cpp | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index c782ec1409240..30c924d45a2cd 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -1467,9 +1467,8 @@ static OBJECTREF getFrozenBoxedStatic(FieldDesc* field) return NULL; } - MethodTable* fieldType = field->GetFieldTypeHandleThrowing().GetMethodTable(); MethodTable* owningType = field->GetEnclosingMethodTable(); - if (owningType->IsSharedByGenericInstantiations() || fieldType->ContainsPointers()) + if (!owningType->IsClassInited() && owningType->IsSharedByGenericInstantiations()) { return NULL; } @@ -1479,9 +1478,14 @@ static OBJECTREF getFrozenBoxedStatic(FieldDesc* field) Object** handle = (Object**)field->GetStaticAddressHandle(basePtr); _ASSERT(handle != nullptr); - if (GCHeapUtilities::GetGCHeap()->IsInFrozenSegment(*handle)) + Object* frozenObj = *handle; + + // ContainsPointers here is unnecessary but it's cheaper than IsInFrozenSegment + // for structs containing gc handles + if (frozenObj != nullptr && !frozenObj->GetMethodTable()->ContainsPointers() && + GCHeapUtilities::GetGCHeap()->IsInFrozenSegment(frozenObj)) { - return ObjectToOBJECTREF(*handle); + return ObjectToOBJECTREF(frozenObj); } return NULL; } From 1739a55df59110fc0ae12f05a3120dc689fd9e86 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sat, 5 Nov 2022 02:23:32 +0100 Subject: [PATCH 16/33] Use fieldLookup --- src/coreclr/inc/corinfo.h | 2 +- src/coreclr/jit/importer.cpp | 25 +++++++++++++++++++++---- src/coreclr/vm/jitinterface.cpp | 30 +++++++++++++----------------- 3 files changed, 35 insertions(+), 22 deletions(-) diff --git a/src/coreclr/inc/corinfo.h b/src/coreclr/inc/corinfo.h index 4958de749672e..52dc71055f783 100644 --- a/src/coreclr/inc/corinfo.h +++ b/src/coreclr/inc/corinfo.h @@ -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; }; //---------------------------------------------------------------------------- diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index f6e194a76cb5b..1cda997922a97 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -4387,6 +4387,9 @@ GenTree* Compiler::impImportStaticFieldAccess(CORINFO_RESOLVED_TOKEN* pResolvedT FieldSeq::FieldKind fieldKind = isSharedStatic ? FieldSeq::FieldKind::SharedStatic : FieldSeq::FieldKind::SimpleStatic; + bool hasKnownDirectAddress = !opts.IsReadyToRun() && pFieldInfo->fieldLookup.addr != nullptr && + pFieldInfo->fieldLookup.accessType == IAT_VALUE; + FieldSeq* innerFldSeq; FieldSeq* outerFldSeq; if (isBoxedStatic) @@ -4402,7 +4405,14 @@ GenTree* Compiler::impImportStaticFieldAccess(CORINFO_RESOLVED_TOKEN* pResolvedT ssize_t offset; if (hasConstAddr) { - offset = reinterpret_cast(info.compCompHnd->getFieldAddress(pResolvedToken->hField)); + if (hasKnownDirectAddress) + { + offset = reinterpret_cast(pFieldInfo->fieldLookup.addr); + } + else + { + offset = reinterpret_cast(info.compCompHnd->getFieldAddress(pResolvedToken->hField)); + } assert(offset != 0); } else @@ -4508,17 +4518,16 @@ GenTree* Compiler::impImportStaticFieldAccess(CORINFO_RESOLVED_TOKEN* pResolvedT { // Do we need the address of a static field? // - if (access & CORINFO_ACCESS_ADDRESS) + if (!hasKnownDirectAddress && (access & CORINFO_ACCESS_ADDRESS)) { 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); + op1 = gtNewIconHandleNode(reinterpret_cast(fldAddr), handleKind, innerFldSeq); INDEBUG(op1->AsIntCon()->gtTargetHandle = reinterpret_cast(pResolvedToken->hField)); if (pFieldInfo->fieldFlags & CORINFO_FLG_FIELD_INITCLASS) @@ -4526,6 +4535,13 @@ GenTree* Compiler::impImportStaticFieldAccess(CORINFO_RESOLVED_TOKEN* pResolvedT op1->gtFlags |= GTF_ICON_INITCLASS; } } + else if (hasKnownDirectAddress) + { + assert(!isBoxedStatic); + op1 = gtNewIconHandleNode((size_t)pFieldInfo->fieldLookup.addr, GTF_ICON_STATIC_HDL, innerFldSeq); + INDEBUG(op1->AsIntCon()->gtTargetHandle = reinterpret_cast(pResolvedToken->hField)); + break; + } else // We need the value of a static field { // In future, it may be better to just create the right tree here instead of folding it later. @@ -4561,6 +4577,7 @@ GenTree* Compiler::impImportStaticFieldAccess(CORINFO_RESOLVED_TOKEN* pResolvedT if (isBoxedStatic) { + assert(!hasKnownDirectAddress); op1 = gtNewOperNode(GT_IND, TYP_REF, op1); op1->gtFlags |= (GTF_IND_INVARIANT | GTF_IND_NONFAULTING | GTF_IND_NONNULL); diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 30c924d45a2cd..af06aea717fa4 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -1468,7 +1468,7 @@ static OBJECTREF getFrozenBoxedStatic(FieldDesc* field) } MethodTable* owningType = field->GetEnclosingMethodTable(); - if (!owningType->IsClassInited() && owningType->IsSharedByGenericInstantiations()) + if (!owningType->IsClassInited() || owningType->IsSharedByGenericInstantiations()) { return NULL; } @@ -1517,6 +1517,8 @@ void CEEInfo::getFieldInfo (CORINFO_RESOLVED_TOKEN * pResolvedToken, DWORD fieldFlags = 0; pResult->offset = pField->GetOffset(); + pResult->fieldLookup.addr = nullptr; + if (pField->IsStatic()) { fieldFlags |= CORINFO_FLG_FIELD_STATIC; @@ -1553,7 +1555,14 @@ void CEEInfo::getFieldInfo (CORINFO_RESOLVED_TOKEN * pResolvedToken, if (pField->GetFieldType() == ELEMENT_TYPE_VALUETYPE) { GCX_COOP(); - if (getFrozenBoxedStatic(pField) == NULL) + Object* frozenBoxedStatic = OBJECTREFToObject(getFrozenBoxedStatic(pField)); + if (frozenBoxedStatic != nullptr) + { + // Skip pMT of the frozen object holding struct + pResult->fieldLookup.addr = (uint8_t*)frozenBoxedStatic + sizeof(void*); + pResult->fieldLookup.accessType = InfoAccessType::IAT_VALUE; + } + else { fieldFlags |= CORINFO_FLG_FIELD_STATIC_IN_HEAP; } @@ -11993,23 +12002,10 @@ void* CEEJitInfo::getFieldAddress(CORINFO_FIELD_HANDLE fieldHnd, GCX_COOP(); - // Check if the field holds a frozen boxed static, return its content in that case - result = OBJECTREFToObject(getFrozenBoxedStatic(field)); - if (result != NULL) - { - // Skip pMT - result = (uint8_t*)result + sizeof(void*); - } - else - { - base = (void*)field->GetBase(); - } + base = (void *) field->GetBase(); } - if (result == NULL) - { - result = field->GetStaticAddressHandle(base); - } + result = field->GetStaticAddressHandle(base); EE_TO_JIT_TRANSITION(); From 78448f13023bf00c85a5b68ce9ee94ba554a8bb6 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Tue, 8 Nov 2022 02:32:40 +0100 Subject: [PATCH 17/33] Run tests on CI (WIP), works locally --- src/coreclr/jit/assertionprop.cpp | 3 +- src/coreclr/jit/emitxarch.cpp | 2 + src/coreclr/jit/importer.cpp | 31 +++++-------- src/coreclr/jit/morph.cpp | 3 ++ .../JitInterface/CorInfoImpl.ReadyToRun.cs | 4 ++ .../JitInterface/CorInfoImpl.RyuJit.cs | 4 ++ src/coreclr/vm/jitinterface.cpp | 46 ++++++++++++------- 7 files changed, 54 insertions(+), 39 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 9a4be3c6e4e9e..a3797e0f51ac0 100644 --- 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(vnCns); - + simd32_t value = vnStore->ConstantValue(vnCns); GenTreeVecCon* vecCon = gtNewVconNode(tree->TypeGet()); vecCon->gtSimd32Val = value; diff --git a/src/coreclr/jit/emitxarch.cpp b/src/coreclr/jit/emitxarch.cpp index 71b2d2ef42ad4..533172b6fbc17 100644 --- a/src/coreclr/jit/emitxarch.cpp +++ b/src/coreclr/jit/emitxarch.cpp @@ -12828,6 +12828,8 @@ BYTE* emitter::emitOutputCV(BYTE* dst, instrDesc* id, code_t code, CnsVal* addc) } else { + // Should not be ever called here? + addr = (BYTE*)emitComp->info.compCompHnd->getFieldAddress(fldh, nullptr); if (addr == nullptr) { diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 1cda997922a97..d557d4a2d64a7 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -4387,8 +4387,8 @@ GenTree* Compiler::impImportStaticFieldAccess(CORINFO_RESOLVED_TOKEN* pResolvedT FieldSeq::FieldKind fieldKind = isSharedStatic ? FieldSeq::FieldKind::SharedStatic : FieldSeq::FieldKind::SimpleStatic; - bool hasKnownDirectAddress = !opts.IsReadyToRun() && pFieldInfo->fieldLookup.addr != nullptr && - pFieldInfo->fieldLookup.accessType == IAT_VALUE; + bool hasConstAddr = (pFieldInfo->fieldAccessor == CORINFO_FIELD_STATIC_ADDRESS) || + (pFieldInfo->fieldAccessor == CORINFO_FIELD_STATIC_RVA_ADDRESS); FieldSeq* innerFldSeq; FieldSeq* outerFldSeq; @@ -4399,21 +4399,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) { - if (hasKnownDirectAddress) - { - offset = reinterpret_cast(pFieldInfo->fieldLookup.addr); - } - else - { - offset = reinterpret_cast(info.compCompHnd->getFieldAddress(pResolvedToken->hField)); - } - assert(offset != 0); + assert(pFieldInfo->fieldLookup.accessType == IAT_VALUE); + offset = reinterpret_cast(pFieldInfo->fieldLookup.addr); } else { @@ -4518,12 +4508,11 @@ GenTree* Compiler::impImportStaticFieldAccess(CORINFO_RESOLVED_TOKEN* pResolvedT { // Do we need the address of a static field? // - if (!hasKnownDirectAddress && (access & CORINFO_ACCESS_ADDRESS)) + if (access & CORINFO_ACCESS_ADDRESS) { - void** pFldAddr = nullptr; - void* fldAddr = info.compCompHnd->getFieldAddress(pResolvedToken->hField, (void**)&pFldAddr); + assert(pFieldInfo->fieldLookup.accessType == IAT_VALUE); + void* fldAddr = pFieldInfo->fieldLookup.addr; // 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; @@ -4535,8 +4524,11 @@ GenTree* Compiler::impImportStaticFieldAccess(CORINFO_RESOLVED_TOKEN* pResolvedT op1->gtFlags |= GTF_ICON_INITCLASS; } } - else if (hasKnownDirectAddress) + else if (hasConstAddr && !isBoxedStatic) { + assert(pFieldInfo->fieldLookup.addr != nullptr); + assert(!isSharedStatic); + assert(pFieldInfo->fieldLookup.accessType == IAT_VALUE); assert(!isBoxedStatic); op1 = gtNewIconHandleNode((size_t)pFieldInfo->fieldLookup.addr, GTF_ICON_STATIC_HDL, innerFldSeq); INDEBUG(op1->AsIntCon()->gtTargetHandle = reinterpret_cast(pResolvedToken->hField)); @@ -4577,7 +4569,6 @@ GenTree* Compiler::impImportStaticFieldAccess(CORINFO_RESOLVED_TOKEN* pResolvedT if (isBoxedStatic) { - assert(!hasKnownDirectAddress); op1 = gtNewOperNode(GT_IND, TYP_REF, op1); op1->gtFlags |= (GTF_IND_INVARIANT | GTF_IND_NONFAULTING | GTF_IND_NONNULL); diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 18ba00e9add32..216a2e5da8708 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -5297,6 +5297,8 @@ GenTree* Compiler::fgMorphField(GenTree* tree, MorphAddrContext* mac) } else { + // TODO: delete the boxed-static part (move to importer) + // Normal static field reference // // If we can we access the static's address directly @@ -5315,6 +5317,7 @@ GenTree* Compiler::fgMorphField(GenTree* tree, MorphAddrContext* mac) bool isBoxedStatic = gtIsStaticFieldPtrToBoxedStruct(tree->TypeGet(), symHnd); if (!isBoxedStatic) { + assert(!"unreachable!"); // Only simple statics get importred as GT_FIELDs. fieldSeq = GetFieldSeqStore()->Create(symHnd, reinterpret_cast(fldAddr), FieldSeq::FieldKind::SimpleStatic); diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs index 974590abe9eba..773e4df50c234 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs @@ -1499,6 +1499,10 @@ private void getFieldInfo(ref CORINFO_RESOLVED_TOKEN pResolvedToken, CORINFO_MET // TODO: Handle the case when the RVA is in the TLS range fieldAccessor = CORINFO_FIELD_ACCESSOR.CORINFO_FIELD_STATIC_RVA_ADDRESS; + ISymbolNode node = _compilation.GetFieldRvaData(field); + pResult->fieldLookup.addr = (void*)ObjectToHandle(node); + pResult->fieldLookup.accessType = node.RepresentsIndirectionCell ? InfoAccessType.IAT_PVALUE : InfoAccessType.IAT_VALUE; + // We are not going through a helper. The constructor has to be triggered explicitly. if (!IsClassPreInited(field.OwningType)) { diff --git a/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs b/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs index 5fa43d0d527bc..f27348175b54b 100644 --- a/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs +++ b/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs @@ -2061,6 +2061,10 @@ private void getFieldInfo(ref CORINFO_RESOLVED_TOKEN pResolvedToken, CORINFO_MET // TODO: Handle the case when the RVA is in the TLS range fieldAccessor = CORINFO_FIELD_ACCESSOR.CORINFO_FIELD_STATIC_RVA_ADDRESS; + ISymbolNode node = _compilation.GetFieldRvaData(field); + pResult->fieldLookup.addr = (void*)ObjectToHandle(node); + pResult->fieldLookup.accessType = node.RepresentsIndirectionCell ? InfoAccessType.IAT_PVALUE : InfoAccessType.IAT_VALUE; + // We are not going through a helper. The constructor has to be triggered explicitly. if (_compilation.HasLazyStaticConstructor(field.OwningType)) { diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index af06aea717fa4..d1559166de244 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -1535,16 +1535,17 @@ void CEEInfo::getFieldInfo (CORINFO_RESOLVED_TOKEN * pResolvedToken, // Provide helper to use if the JIT is not able to emit the TLS access // as intrinsic pResult->helper = CORINFO_HELP_GETSTATICFIELDADDR_TLS; - pResult->offset = module->GetFieldTlsOffset(pResult->offset); } else { fieldAccessor = CORINFO_FIELD_STATIC_RVA_ADDRESS; + pResult->fieldLookup.addr = pField->GetStaticAddressHandle(NULL); + pResult->fieldLookup.accessType = IAT_VALUE; } // We are not going through a helper. The constructor has to be triggered explicitly. - if (!pFieldMT->IsClassPreInited()) + if (!pFieldMT->IsClassInited()) fieldFlags |= CORINFO_FLG_FIELD_INITCLASS; } else @@ -1553,20 +1554,7 @@ void CEEInfo::getFieldInfo (CORINFO_RESOLVED_TOKEN * pResolvedToken, CORINFO_FIELD_ACCESSOR intrinsicAccessor; if (pField->GetFieldType() == ELEMENT_TYPE_VALUETYPE) - { - GCX_COOP(); - Object* frozenBoxedStatic = OBJECTREFToObject(getFrozenBoxedStatic(pField)); - if (frozenBoxedStatic != nullptr) - { - // Skip pMT of the frozen object holding struct - pResult->fieldLookup.addr = (uint8_t*)frozenBoxedStatic + sizeof(void*); - pResult->fieldLookup.accessType = InfoAccessType::IAT_VALUE; - } - else - { - fieldFlags |= CORINFO_FLG_FIELD_STATIC_IN_HEAP; - } - } + fieldFlags |= CORINFO_FLG_FIELD_STATIC_IN_HEAP; if (pFieldMT->IsSharedByGenericInstantiations()) { @@ -1597,9 +1585,33 @@ void CEEInfo::getFieldInfo (CORINFO_RESOLVED_TOKEN * pResolvedToken, { fieldAccessor = CORINFO_FIELD_STATIC_ADDRESS; + // Allocate space for the local class if necessary, but don't trigger + // class construction. + DomainLocalModule* pLocalModule = pFieldMT->GetDomainLocalModule(); + pLocalModule->PopulateClass(pFieldMT); + // We are not going through a helper. The constructor has to be triggered explicitly. - if (!pFieldMT->IsClassPreInited()) + if (!pFieldMT->IsClassInited()) fieldFlags |= CORINFO_FLG_FIELD_INITCLASS; + + GCX_COOP(); + + pResult->fieldLookup.addr = pField->GetStaticAddressHandle((void*)pField->GetBase()); + pResult->fieldLookup.accessType = IAT_VALUE; + + if ((fieldFlags & (CORINFO_FLG_FIELD_STATIC_IN_HEAP | CORINFO_FLG_FIELD_INITCLASS)) == CORINFO_FLG_FIELD_STATIC_IN_HEAP) + { + Object* frozenObj = *(Object**)pResult->fieldLookup.addr; + + // ContainsPointers here is unnecessary but it's cheaper than IsInFrozenSegment + // for structs containing gc handles + if (frozenObj != nullptr && !frozenObj->GetMethodTable()->ContainsPointers() && + GCHeapUtilities::GetGCHeap()->IsInFrozenSegment(frozenObj)) + { + pResult->fieldLookup.addr = frozenObj->GetData(); + fieldFlags &= ~CORINFO_FLG_FIELD_STATIC_IN_HEAP; + } + } } } From faa5e51f4f62d6c8e4d7dcbb367695e221cf6eac Mon Sep 17 00:00:00 2001 From: EgorBo Date: Tue, 8 Nov 2022 12:41:08 +0100 Subject: [PATCH 18/33] Delete getFieldAddress --- src/coreclr/jit/emitarm.cpp | 5 +-- src/coreclr/jit/emitxarch.cpp | 16 +------ src/coreclr/jit/importer.cpp | 65 ++++------------------------ src/coreclr/jit/morph.cpp | 81 +---------------------------------- 4 files changed, 13 insertions(+), 154 deletions(-) diff --git a/src/coreclr/jit/emitarm.cpp b/src/coreclr/jit/emitarm.cpp index a546d5aaab940..741d2d50dfbac 100644 --- 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 index 533172b6fbc17..47a552be2ab8a 100644 --- a/src/coreclr/jit/emitxarch.cpp +++ b/src/coreclr/jit/emitxarch.cpp @@ -12822,20 +12822,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 - { - // Should not be ever called here? - - 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/importer.cpp b/src/coreclr/jit/importer.cpp index d557d4a2d64a7..4e1f5c14c1302 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -4506,62 +4506,16 @@ GenTree* Compiler::impImportStaticFieldAccess(CORINFO_RESOLVED_TOKEN* pResolvedT default: { - // Do we need the address of a static field? - // - if (access & CORINFO_ACCESS_ADDRESS) - { - assert(pFieldInfo->fieldLookup.accessType == IAT_VALUE); - void* fldAddr = pFieldInfo->fieldLookup.addr; - // We should always be able to access this static's address directly. - - // Create the address node. - GenTreeFlags handleKind = isBoxedStatic ? GTF_ICON_STATIC_BOX_PTR : GTF_ICON_STATIC_HDL; - op1 = gtNewIconHandleNode(reinterpret_cast(fldAddr), handleKind, innerFldSeq); - INDEBUG(op1->AsIntCon()->gtTargetHandle = reinterpret_cast(pResolvedToken->hField)); - - if (pFieldInfo->fieldFlags & CORINFO_FLG_FIELD_INITCLASS) - { - op1->gtFlags |= GTF_ICON_INITCLASS; - } - } - else if (hasConstAddr && !isBoxedStatic) + assert(hasConstAddr); + assert(pFieldInfo->fieldLookup.addr != nullptr); + assert(pFieldInfo->fieldLookup.accessType == IAT_VALUE); + size_t fldAddr = reinterpret_cast(pFieldInfo->fieldLookup.addr); + GenTreeFlags handleKind = isBoxedStatic ? GTF_ICON_STATIC_BOX_PTR : GTF_ICON_STATIC_HDL; + op1 = gtNewIconHandleNode(fldAddr, handleKind, innerFldSeq); + INDEBUG(op1->AsIntCon()->gtTargetHandle = reinterpret_cast(pResolvedToken->hField)); + if (pFieldInfo->fieldFlags & CORINFO_FLG_FIELD_INITCLASS) { - assert(pFieldInfo->fieldLookup.addr != nullptr); - assert(!isSharedStatic); - assert(pFieldInfo->fieldLookup.accessType == IAT_VALUE); - assert(!isBoxedStatic); - op1 = gtNewIconHandleNode((size_t)pFieldInfo->fieldLookup.addr, GTF_ICON_STATIC_HDL, innerFldSeq); - INDEBUG(op1->AsIntCon()->gtTargetHandle = reinterpret_cast(pResolvedToken->hField)); - break; - } - else // We need the value of a static field - { - // In future, it may be better to just create the right tree here instead of folding it later. - 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); - } - } - - return op1; + op1->gtFlags |= GTF_ICON_INITCLASS; } break; } @@ -4571,7 +4525,6 @@ GenTree* Compiler::impImportStaticFieldAccess(CORINFO_RESOLVED_TOKEN* pResolvedT { op1 = gtNewOperNode(GT_IND, TYP_REF, op1); op1->gtFlags |= (GTF_IND_INVARIANT | GTF_IND_NONFAULTING | GTF_IND_NONNULL); - op1 = gtNewOperNode(GT_ADD, TYP_BYREF, op1, gtNewIconNode(TARGET_POINTER_SIZE, outerFldSeq)); } diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 216a2e5da8708..2bcc22893ee2d 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -5297,86 +5297,7 @@ GenTree* Compiler::fgMorphField(GenTree* tree, MorphAddrContext* mac) } else { - // TODO: delete the boxed-static part (move to importer) - - // Normal static field reference - // - // If we can we access the static's address directly - // then pFldAddr will be NULL and - // fldAddr will be the actual address of the static field - // - void** pFldAddr = nullptr; - void* fldAddr = info.compCompHnd->getFieldAddress(symHnd, (void**)&pFldAddr); - - // We should always be able to access this static field address directly - // - assert(pFldAddr == nullptr); - - // For boxed statics, this direct address will be for the box. We have already added - // the indirection for the field itself and attached the sequence, in importation. - bool isBoxedStatic = gtIsStaticFieldPtrToBoxedStruct(tree->TypeGet(), symHnd); - if (!isBoxedStatic) - { - assert(!"unreachable!"); - // Only simple statics get importred as GT_FIELDs. - fieldSeq = GetFieldSeqStore()->Create(symHnd, reinterpret_cast(fldAddr), - FieldSeq::FieldKind::SimpleStatic); - } - - // TODO-CQ: enable this optimization for 32 bit targets. - bool isStaticReadOnlyInited = false; -#ifdef TARGET_64BIT - if (tree->TypeIs(TYP_REF) && !isBoxedStatic) - { - bool pIsSpeculative = true; - if (info.compCompHnd->getStaticFieldCurrentClass(symHnd, &pIsSpeculative) != NO_CLASS_HANDLE) - { - isStaticReadOnlyInited = !pIsSpeculative; - } - } -#endif // TARGET_64BIT - - GenTreeFlags handleKind = GTF_EMPTY; - if (isBoxedStatic) - { - handleKind = GTF_ICON_STATIC_BOX_PTR; - } - else if (isStaticReadOnlyInited) - { - handleKind = GTF_ICON_CONST_PTR; - } - else - { - handleKind = GTF_ICON_STATIC_HDL; - } - GenTreeIntCon* addr = gtNewIconHandleNode((size_t)fldAddr, handleKind, fieldSeq); - INDEBUG(addr->gtTargetHandle = reinterpret_cast(symHnd)); - - // Translate GTF_FLD_INITCLASS to GTF_ICON_INITCLASS, if we need to. - if (((tree->gtFlags & GTF_FLD_INITCLASS) != 0) && !isStaticReadOnlyInited) - { - tree->gtFlags &= ~GTF_FLD_INITCLASS; - addr->gtFlags |= GTF_ICON_INITCLASS; - } - - tree->SetOper(GT_IND); - tree->AsOp()->gtOp1 = addr; - - if (isBoxedStatic) - { - // The box for the static cannot be null, and is logically invariant, since it - // represents (a base for) the static's address. - tree->gtFlags |= (GTF_IND_INVARIANT | GTF_IND_NONFAULTING | GTF_IND_NONNULL); - } - else if (isStaticReadOnlyInited) - { - JITDUMP("Marking initialized static read-only field '%s' as invariant.\n", eeGetFieldName(symHnd)); - - // Static readonly field is not null at this point (see getStaticFieldCurrentClass impl). - tree->gtFlags |= (GTF_IND_INVARIANT | GTF_IND_NONFAULTING | GTF_IND_NONNULL); - } - - return fgMorphSmpOp(tree, /* mac */ nullptr); + assert(!"Normal statics are expected to be handled in the importer"); } } From 90b3e1b693989fafba439103286b24634041c4db Mon Sep 17 00:00:00 2001 From: EgorBo Date: Tue, 8 Nov 2022 13:03:38 +0100 Subject: [PATCH 19/33] Fix regression around invariant ref-type statics --- src/coreclr/jit/importer.cpp | 33 +++++++++++++++++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 4e1f5c14c1302..c09303c5bc7fe 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -4414,6 +4414,7 @@ GenTree* Compiler::impImportStaticFieldAccess(CORINFO_RESOLVED_TOKEN* pResolvedT outerFldSeq = nullptr; } + bool isStaticReadOnlyInitedRef = false; GenTree* op1; switch (pFieldInfo->fieldAccessor) { @@ -4506,12 +4507,36 @@ GenTree* Compiler::impImportStaticFieldAccess(CORINFO_RESOLVED_TOKEN* pResolvedT default: { + // TODO-CQ: enable this optimization for 32 bit targets. +#ifdef TARGET_64BIT + if (!isBoxedStatic && (lclTyp == TYP_REF)) + { + bool isSpeculative = true; + if ((info.compCompHnd->getStaticFieldCurrentClass(pResolvedToken->hField, &isSpeculative) != NO_CLASS_HANDLE)) + { + isStaticReadOnlyInitedRef = !isSpeculative; + } + } +#endif // TARGET_64BIT + assert(hasConstAddr); assert(pFieldInfo->fieldLookup.addr != nullptr); assert(pFieldInfo->fieldLookup.accessType == IAT_VALUE); size_t fldAddr = reinterpret_cast(pFieldInfo->fieldLookup.addr); - GenTreeFlags handleKind = isBoxedStatic ? GTF_ICON_STATIC_BOX_PTR : GTF_ICON_STATIC_HDL; - op1 = gtNewIconHandleNode(fldAddr, handleKind, innerFldSeq); + 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(pResolvedToken->hField)); if (pFieldInfo->fieldFlags & CORINFO_FLG_FIELD_INITCLASS) { @@ -4540,6 +4565,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; From 6dc5a18f0245eed67c73a5d46db3499d7e650525 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Tue, 8 Nov 2022 13:18:31 +0100 Subject: [PATCH 20/33] Remove getFieldAddress --- src/coreclr/inc/corinfo.h | 9 +- src/coreclr/inc/icorjitinfoimpl_generated.h | 4 - src/coreclr/inc/jiteeversionguid.h | 10 +- src/coreclr/jit/ICorJitInfo_names_generated.h | 1 - .../jit/ICorJitInfo_wrapper_generated.hpp | 10 -- src/coreclr/jit/importer.cpp | 9 +- .../tools/Common/JitInterface/CorInfoImpl.cs | 19 --- .../JitInterface/CorInfoImpl_generated.cs | 78 +++++------- .../ThunkGenerator/ThunkInput.txt | 1 - .../aot/jitinterface/jitinterface_generated.h | 11 -- .../tools/superpmi/superpmi-shared/agnostic.h | 6 - .../superpmi-shared/compileresult.cpp | 2 +- .../tools/superpmi/superpmi-shared/lwmlist.h | 1 - .../superpmi-shared/methodcontext.cpp | 35 ------ .../superpmi/superpmi-shared/methodcontext.h | 6 +- .../superpmi-shim-collector/icorjitinfo.cpp | 13 -- .../icorjitinfo_generated.cpp | 8 -- .../icorjitinfo_generated.cpp | 7 -- .../tools/superpmi/superpmi/icorjitinfo.cpp | 7 -- src/coreclr/vm/jitinterface.cpp | 112 ------------------ src/coreclr/vm/jitinterface.h | 1 - 21 files changed, 44 insertions(+), 306 deletions(-) diff --git a/src/coreclr/inc/corinfo.h b/src/coreclr/inc/corinfo.h index 52dc71055f783..b5b8848d968bf 100644 --- 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 - * This is a normal static field. Its address in memory is determined by getFieldAddress. (see + * This is a normal static field. Its address in memory is determined by getFieldInfo. (see also CORINFO_FLG_STATIC_IN_HEAP). @@ -3190,13 +3190,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/icorjitinfoimpl_generated.h b/src/coreclr/inc/icorjitinfoimpl_generated.h index 44c98ae5397f5..8039c664cfaab 100644 --- a/src/coreclr/inc/icorjitinfoimpl_generated.h +++ b/src/coreclr/inc/icorjitinfoimpl_generated.h @@ -612,10 +612,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 index 2f1bb07461e18..5da21703b886a 100644 --- 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 = { /* 77b6df16-d27f-4118-9dfd-d8073ff20fb6 */ - 0x77b6df16, - 0xd27f, - 0x4118, - {0x9d, 0xfd, 0xd8, 0x7, 0x3f, 0xf2, 0xf, 0xb6} +constexpr GUID JITEEVersionIdentifier = { /* 17451041-3be6-44ae-863b-ba5155476f90 */ + 0x17451041, + 0x3be6, + 0x44ae, + {0x86, 0x3b, 0xba, 0x51, 0x55, 0x47, 0x6f, 0x90} }; ////////////////////////////////////////////////////////////////////////////////////////////////////////// diff --git a/src/coreclr/jit/ICorJitInfo_names_generated.h b/src/coreclr/jit/ICorJitInfo_names_generated.h index 8c43eb4823aef..fac84ee23a328 100644 --- a/src/coreclr/jit/ICorJitInfo_names_generated.h +++ b/src/coreclr/jit/ICorJitInfo_names_generated.h @@ -154,7 +154,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 index 057f5ba417759..8dc71797af15f 100644 --- a/src/coreclr/jit/ICorJitInfo_wrapper_generated.hpp +++ b/src/coreclr/jit/ICorJitInfo_wrapper_generated.hpp @@ -1469,16 +1469,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/importer.cpp b/src/coreclr/jit/importer.cpp index c09303c5bc7fe..9764dcf0ed0ac 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -4414,7 +4414,7 @@ GenTree* Compiler::impImportStaticFieldAccess(CORINFO_RESOLVED_TOKEN* pResolvedT outerFldSeq = nullptr; } - bool isStaticReadOnlyInitedRef = false; + bool isStaticReadOnlyInitedRef = false; GenTree* op1; switch (pFieldInfo->fieldAccessor) { @@ -4507,12 +4507,13 @@ GenTree* Compiler::impImportStaticFieldAccess(CORINFO_RESOLVED_TOKEN* pResolvedT default: { - // TODO-CQ: enable this optimization for 32 bit targets. +// TODO-CQ: enable this optimization for 32 bit targets. #ifdef TARGET_64BIT if (!isBoxedStatic && (lclTyp == TYP_REF)) { bool isSpeculative = true; - if ((info.compCompHnd->getStaticFieldCurrentClass(pResolvedToken->hField, &isSpeculative) != NO_CLASS_HANDLE)) + if ((info.compCompHnd->getStaticFieldCurrentClass(pResolvedToken->hField, &isSpeculative) != + NO_CLASS_HANDLE)) { isStaticReadOnlyInitedRef = !isSpeculative; } @@ -4522,7 +4523,7 @@ GenTree* Compiler::impImportStaticFieldAccess(CORINFO_RESOLVED_TOKEN* pResolvedT assert(hasConstAddr); assert(pFieldInfo->fieldLookup.addr != nullptr); assert(pFieldInfo->fieldLookup.accessType == IAT_VALUE); - size_t fldAddr = reinterpret_cast(pFieldInfo->fieldLookup.addr); + size_t fldAddr = reinterpret_cast(pFieldInfo->fieldLookup.addr); GenTreeFlags handleKind; if (isBoxedStatic) { diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs index 376de7408b6bb..28f475bdbc71f 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs @@ -3314,25 +3314,6 @@ private bool isRIDClassDomainID(CORINFO_CLASS_STRUCT_* cls) private uint getClassDomainID(CORINFO_CLASS_STRUCT_* cls, ref void* ppIndirection) { throw new NotImplementedException("getClassDomainID"); } - private void* getFieldAddress(CORINFO_FIELD_STRUCT_* field, void** ppIndirection) - { - FieldDesc fieldDesc = HandleToObject(field); - Debug.Assert(fieldDesc.HasRva); - ISymbolNode node = _compilation.GetFieldRvaData(fieldDesc); - void *handle = (void *)ObjectToHandle(node); - if (node.RepresentsIndirectionCell) - { - *ppIndirection = handle; - return null; - } - else - { - if (ppIndirection != null) - *ppIndirection = null; - return handle; - } - } - private CORINFO_CLASS_STRUCT_* getStaticFieldCurrentClass(CORINFO_FIELD_STRUCT_* field, byte* pIsSpeculative) { if (pIsSpeculative != null) diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoImpl_generated.cs b/src/coreclr/tools/Common/JitInterface/CorInfoImpl_generated.cs index 4b830673d4a8c..5542a4a803272 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoImpl_generated.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoImpl_generated.cs @@ -2228,21 +2228,6 @@ private static uint _getClassDomainID(IntPtr thisHandle, IntPtr* ppException, CO } } - [UnmanagedCallersOnly] - private static void* _getFieldAddress(IntPtr thisHandle, IntPtr* ppException, CORINFO_FIELD_STRUCT_* field, void** ppIndirection) - { - var _this = GetThis(thisHandle); - try - { - return _this.getFieldAddress(field, ppIndirection); - } - catch (Exception ex) - { - *ppException = _this.AllocException(ex); - return default; - } - } - [UnmanagedCallersOnly] private static byte _getReadonlyStaticFieldValue(IntPtr thisHandle, IntPtr* ppException, CORINFO_FIELD_STRUCT_* field, byte* buffer, int bufferSize, byte ignoreMovableObjects) { @@ -2685,7 +2670,7 @@ private static uint _getJitFlags(IntPtr thisHandle, IntPtr* ppException, CORJIT_ private static IntPtr GetUnmanagedCallbacks() { - void** callbacks = (void**)Marshal.AllocCoTaskMem(sizeof(IntPtr) * 181); + void** callbacks = (void**)Marshal.AllocCoTaskMem(sizeof(IntPtr) * 180); callbacks[0] = (delegate* unmanaged)&_isIntrinsic; callbacks[1] = (delegate* unmanaged)&_getMethodAttribs; @@ -2837,37 +2822,36 @@ private static IntPtr GetUnmanagedCallbacks() callbacks[147] = (delegate* unmanaged)&_canAccessFamily; callbacks[148] = (delegate* unmanaged)&_isRIDClassDomainID; callbacks[149] = (delegate* unmanaged)&_getClassDomainID; - callbacks[150] = (delegate* unmanaged)&_getFieldAddress; - callbacks[151] = (delegate* unmanaged)&_getReadonlyStaticFieldValue; - callbacks[152] = (delegate* unmanaged)&_getStaticFieldCurrentClass; - callbacks[153] = (delegate* unmanaged)&_getVarArgsHandle; - callbacks[154] = (delegate* unmanaged)&_canGetVarArgsHandle; - callbacks[155] = (delegate* unmanaged)&_constructStringLiteral; - callbacks[156] = (delegate* unmanaged)&_emptyStringLiteral; - callbacks[157] = (delegate* unmanaged)&_getFieldThreadLocalStoreID; - callbacks[158] = (delegate* unmanaged)&_addActiveDependency; - callbacks[159] = (delegate* unmanaged)&_GetDelegateCtor; - callbacks[160] = (delegate* unmanaged)&_MethodCompileComplete; - callbacks[161] = (delegate* unmanaged)&_getTailCallHelpers; - callbacks[162] = (delegate* unmanaged)&_convertPInvokeCalliToCall; - callbacks[163] = (delegate* unmanaged)&_notifyInstructionSetUsage; - callbacks[164] = (delegate* unmanaged)&_updateEntryPointForTailCall; - callbacks[165] = (delegate* unmanaged)&_allocMem; - callbacks[166] = (delegate* unmanaged)&_reserveUnwindInfo; - callbacks[167] = (delegate* unmanaged)&_allocUnwindInfo; - callbacks[168] = (delegate* unmanaged)&_allocGCInfo; - callbacks[169] = (delegate* unmanaged)&_setEHcount; - callbacks[170] = (delegate* unmanaged)&_setEHinfo; - callbacks[171] = (delegate* unmanaged)&_logMsg; - callbacks[172] = (delegate* unmanaged)&_doAssert; - callbacks[173] = (delegate* unmanaged)&_reportFatalError; - callbacks[174] = (delegate* unmanaged)&_getPgoInstrumentationResults; - callbacks[175] = (delegate* unmanaged)&_allocPgoInstrumentationBySchema; - callbacks[176] = (delegate* unmanaged)&_recordCallSite; - callbacks[177] = (delegate* unmanaged)&_recordRelocation; - callbacks[178] = (delegate* unmanaged)&_getRelocTypeHint; - callbacks[179] = (delegate* unmanaged)&_getExpectedTargetArchitecture; - callbacks[180] = (delegate* unmanaged)&_getJitFlags; + callbacks[150] = (delegate* unmanaged)&_getReadonlyStaticFieldValue; + callbacks[151] = (delegate* unmanaged)&_getStaticFieldCurrentClass; + callbacks[152] = (delegate* unmanaged)&_getVarArgsHandle; + callbacks[153] = (delegate* unmanaged)&_canGetVarArgsHandle; + callbacks[154] = (delegate* unmanaged)&_constructStringLiteral; + callbacks[155] = (delegate* unmanaged)&_emptyStringLiteral; + callbacks[156] = (delegate* unmanaged)&_getFieldThreadLocalStoreID; + callbacks[157] = (delegate* unmanaged)&_addActiveDependency; + callbacks[158] = (delegate* unmanaged)&_GetDelegateCtor; + callbacks[159] = (delegate* unmanaged)&_MethodCompileComplete; + callbacks[160] = (delegate* unmanaged)&_getTailCallHelpers; + callbacks[161] = (delegate* unmanaged)&_convertPInvokeCalliToCall; + callbacks[162] = (delegate* unmanaged)&_notifyInstructionSetUsage; + callbacks[163] = (delegate* unmanaged)&_updateEntryPointForTailCall; + callbacks[164] = (delegate* unmanaged)&_allocMem; + callbacks[165] = (delegate* unmanaged)&_reserveUnwindInfo; + callbacks[166] = (delegate* unmanaged)&_allocUnwindInfo; + callbacks[167] = (delegate* unmanaged)&_allocGCInfo; + callbacks[168] = (delegate* unmanaged)&_setEHcount; + callbacks[169] = (delegate* unmanaged)&_setEHinfo; + callbacks[170] = (delegate* unmanaged)&_logMsg; + callbacks[171] = (delegate* unmanaged)&_doAssert; + callbacks[172] = (delegate* unmanaged)&_reportFatalError; + callbacks[173] = (delegate* unmanaged)&_getPgoInstrumentationResults; + callbacks[174] = (delegate* unmanaged)&_allocPgoInstrumentationBySchema; + callbacks[175] = (delegate* unmanaged)&_recordCallSite; + callbacks[176] = (delegate* unmanaged)&_recordRelocation; + callbacks[177] = (delegate* unmanaged)&_getRelocTypeHint; + callbacks[178] = (delegate* unmanaged)&_getExpectedTargetArchitecture; + callbacks[179] = (delegate* unmanaged)&_getJitFlags; return (IntPtr)callbacks; } diff --git a/src/coreclr/tools/Common/JitInterface/ThunkGenerator/ThunkInput.txt b/src/coreclr/tools/Common/JitInterface/ThunkGenerator/ThunkInput.txt index e120cacc136f6..b5996380ae181 100644 --- a/src/coreclr/tools/Common/JitInterface/ThunkGenerator/ThunkInput.txt +++ b/src/coreclr/tools/Common/JitInterface/ThunkGenerator/ThunkInput.txt @@ -306,7 +306,6 @@ FUNCTIONS bool canAccessFamily(CORINFO_METHOD_HANDLE hCaller, CORINFO_CLASS_HANDLE hInstanceType); bool isRIDClassDomainID(CORINFO_CLASS_HANDLE cls); unsigned getClassDomainID (CORINFO_CLASS_HANDLE cls, void **ppIndirection); - void* getFieldAddress(CORINFO_FIELD_HANDLE field, VOIDSTARSTAR ppIndirection); bool getReadonlyStaticFieldValue(CORINFO_FIELD_HANDLE field, uint8_t *buffer, int bufferSize, bool ignoreMovableObjects); CORINFO_CLASS_HANDLE getStaticFieldCurrentClass(CORINFO_FIELD_HANDLE field, BoolStar pIsSpeculative); CORINFO_VARARGS_HANDLE getVarArgsHandle(CORINFO_SIG_INFO *pSig, void **ppIndirection); diff --git a/src/coreclr/tools/aot/jitinterface/jitinterface_generated.h b/src/coreclr/tools/aot/jitinterface/jitinterface_generated.h index fb7889cbb9d9f..2be1c5397089f 100644 --- a/src/coreclr/tools/aot/jitinterface/jitinterface_generated.h +++ b/src/coreclr/tools/aot/jitinterface/jitinterface_generated.h @@ -161,7 +161,6 @@ struct JitInterfaceCallbacks bool (* canAccessFamily)(void * thisHandle, CorInfoExceptionClass** ppException, CORINFO_METHOD_HANDLE hCaller, CORINFO_CLASS_HANDLE hInstanceType); bool (* isRIDClassDomainID)(void * thisHandle, CorInfoExceptionClass** ppException, CORINFO_CLASS_HANDLE cls); unsigned (* getClassDomainID)(void * thisHandle, CorInfoExceptionClass** ppException, CORINFO_CLASS_HANDLE cls, void** ppIndirection); - void* (* getFieldAddress)(void * thisHandle, CorInfoExceptionClass** ppException, CORINFO_FIELD_HANDLE field, void** ppIndirection); bool (* getReadonlyStaticFieldValue)(void * thisHandle, CorInfoExceptionClass** ppException, CORINFO_FIELD_HANDLE field, uint8_t* buffer, int bufferSize, bool ignoreMovableObjects); CORINFO_CLASS_HANDLE (* getStaticFieldCurrentClass)(void * thisHandle, CorInfoExceptionClass** ppException, CORINFO_FIELD_HANDLE field, bool* pIsSpeculative); CORINFO_VARARGS_HANDLE (* getVarArgsHandle)(void * thisHandle, CorInfoExceptionClass** ppException, CORINFO_SIG_INFO* pSig, void** ppIndirection); @@ -1646,16 +1645,6 @@ class JitInterfaceWrapper : public ICorJitInfo return temp; } - virtual void* getFieldAddress( - CORINFO_FIELD_HANDLE field, - void** ppIndirection) -{ - CorInfoExceptionClass* pException = nullptr; - void* temp = _callbacks->getFieldAddress(_thisHandle, &pException, field, ppIndirection); - if (pException != nullptr) throw pException; - return temp; -} - virtual bool getReadonlyStaticFieldValue( CORINFO_FIELD_HANDLE field, uint8_t* buffer, diff --git a/src/coreclr/tools/superpmi/superpmi-shared/agnostic.h b/src/coreclr/tools/superpmi/superpmi-shared/agnostic.h index b4e16e8770393..6c9da5cb6b3d0 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/agnostic.h +++ b/src/coreclr/tools/superpmi/superpmi-shared/agnostic.h @@ -198,12 +198,6 @@ struct Agnostic_GetOSRInfo unsigned ilOffset; }; -struct Agnostic_GetFieldAddress -{ - DWORDLONG ppIndirection; - DWORDLONG fieldAddress; -}; - struct Agnostic_GetStaticFieldCurrentClass { DWORDLONG classHandle; diff --git a/src/coreclr/tools/superpmi/superpmi-shared/compileresult.cpp b/src/coreclr/tools/superpmi/superpmi-shared/compileresult.cpp index 327435391fc95..e0cb6a6020e3a 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/compileresult.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shared/compileresult.cpp @@ -926,7 +926,7 @@ void CompileResult::applyRelocs(RelocContext* rc, unsigned char* block1, ULONG b if (index == -1) { // See if the original address is in the replay address map. This happens for - // relocations on static field addresses found via getFieldAddress(). + // relocations on static field addresses found via getFieldInfo(). void* origAddr = repAddressMap((void*)tmp.target); if ((origAddr != (void*)-1) && (origAddr != nullptr)) { diff --git a/src/coreclr/tools/superpmi/superpmi-shared/lwmlist.h b/src/coreclr/tools/superpmi/superpmi-shared/lwmlist.h index dab1ac0c1a064..bf0f3eeb0f9d5 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/lwmlist.h +++ b/src/coreclr/tools/superpmi/superpmi-shared/lwmlist.h @@ -79,7 +79,6 @@ LWM(GetDefaultEqualityComparerClass, DWORDLONG, DWORDLONG) LWM(GetDelegateCtor, Agnostic_GetDelegateCtorIn, Agnostic_GetDelegateCtorOut) LWM(GetEEInfo, DWORD, Agnostic_CORINFO_EE_INFO) LWM(GetEHinfo, DLD, Agnostic_CORINFO_EH_CLAUSE) -LWM(GetFieldAddress, DWORDLONG, Agnostic_GetFieldAddress) LWM(GetReadonlyStaticFieldValue, DLDD, DD) LWM(GetStaticFieldCurrentClass, DWORDLONG, Agnostic_GetStaticFieldCurrentClass) LWM(GetFieldClass, DWORDLONG, DWORDLONG) diff --git a/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp b/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp index 0d4a17f65b837..5a0df73c3b45e 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp @@ -3628,41 +3628,6 @@ CORINFO_METHOD_HANDLE MethodContext::repEmbedMethodHandle(CORINFO_METHOD_HANDLE return (CORINFO_METHOD_HANDLE)value.B; } -void MethodContext::recGetFieldAddress(CORINFO_FIELD_HANDLE field, void** ppIndirection, void* result, CorInfoType cit) -{ - if (GetFieldAddress == nullptr) - GetFieldAddress = new LightWeightMap(); - - Agnostic_GetFieldAddress value; - if (ppIndirection == nullptr) - value.ppIndirection = 0; - else - value.ppIndirection = CastPointer(*ppIndirection); - value.fieldAddress = CastPointer(result); - - DWORDLONG key = CastHandle(field); - GetFieldAddress->Add(key, value); - DEBUG_REC(dmpGetFieldAddress(key, value)); -} -void MethodContext::dmpGetFieldAddress(DWORDLONG key, const Agnostic_GetFieldAddress& value) -{ - printf("GetFieldAddress key fld-%016llX, value ppi-%016llX addr-%016llX", key, value.ppIndirection, value.fieldAddress); -} -void* MethodContext::repGetFieldAddress(CORINFO_FIELD_HANDLE field, void** ppIndirection) -{ - DWORDLONG key = CastHandle(field); - AssertMapAndKeyExist(GetFieldAddress, key, ": key %016llX", key); - - Agnostic_GetFieldAddress value = GetFieldAddress->Get(key); - DEBUG_REP(dmpGetFieldAddress(key, value)); - - if (ppIndirection != nullptr) - { - *ppIndirection = (void*)value.ppIndirection; - } - return (void*)value.fieldAddress; -} - void MethodContext::recGetReadonlyStaticFieldValue(CORINFO_FIELD_HANDLE field, uint8_t* buffer, int bufferSize, bool ignoreMovableObjects, bool result) { if (GetReadonlyStaticFieldValue == nullptr) diff --git a/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.h b/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.h index c3e2fdabfde69..f8f009eb26cc2 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.h +++ b/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.h @@ -488,10 +488,6 @@ class MethodContext void dmpEmbedMethodHandle(DWORDLONG key, DLDL value); CORINFO_METHOD_HANDLE repEmbedMethodHandle(CORINFO_METHOD_HANDLE handle, void** ppIndirection); - void recGetFieldAddress(CORINFO_FIELD_HANDLE field, void** ppIndirection, void* result, CorInfoType cit); - void dmpGetFieldAddress(DWORDLONG key, const Agnostic_GetFieldAddress& value); - void* repGetFieldAddress(CORINFO_FIELD_HANDLE field, void** ppIndirection); - void recGetReadonlyStaticFieldValue(CORINFO_FIELD_HANDLE field, uint8_t* buffer, int bufferSize, bool ignoreMovableObjects, bool result); void dmpGetReadonlyStaticFieldValue(DLDD key, DD value); bool repGetReadonlyStaticFieldValue(CORINFO_FIELD_HANDLE field, uint8_t* buffer, int bufferSize, bool ignoreMovableObjects); @@ -1004,7 +1000,7 @@ enum mcPackets Packet_GetDelegateCtor = 49, Packet_GetEEInfo = 50, Packet_GetEHinfo = 51, - Packet_GetFieldAddress = 52, + //Packet_GetFieldAddress = 52, Packet_GetFieldClass = 53, Packet_GetFieldInClass = 54, Packet_GetFieldInfo = 55, diff --git a/src/coreclr/tools/superpmi/superpmi-shim-collector/icorjitinfo.cpp b/src/coreclr/tools/superpmi/superpmi-shim-collector/icorjitinfo.cpp index 75d5a6f43749c..30861364ca9d6 100644 --- a/src/coreclr/tools/superpmi/superpmi-shim-collector/icorjitinfo.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shim-collector/icorjitinfo.cpp @@ -1737,19 +1737,6 @@ unsigned interceptor_ICJI::getClassDomainID(CORINFO_CLASS_HANDLE cls, void** ppI return temp; } -// return the data's address (for static fields only) -void* interceptor_ICJI::getFieldAddress(CORINFO_FIELD_HANDLE field, void** ppIndirection) -{ - mc->cr->AddCall("getFieldAddress"); - void* temp = original_ICorJitInfo->getFieldAddress(field, ppIndirection); - - // Figure out the element type so we know how much we can load - CORINFO_CLASS_HANDLE cch; - CorInfoType cit = getFieldType(field, &cch, NULL); - mc->recGetFieldAddress(field, ppIndirection, temp, cit); - return temp; -} - bool interceptor_ICJI::getReadonlyStaticFieldValue(CORINFO_FIELD_HANDLE field, uint8_t* buffer, int bufferSize, bool ignoreMovableObjects) { mc->cr->AddCall("getReadonlyStaticFieldValue"); diff --git a/src/coreclr/tools/superpmi/superpmi-shim-counter/icorjitinfo_generated.cpp b/src/coreclr/tools/superpmi/superpmi-shim-counter/icorjitinfo_generated.cpp index c9bb221333a32..7b22661bb74df 100644 --- a/src/coreclr/tools/superpmi/superpmi-shim-counter/icorjitinfo_generated.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shim-counter/icorjitinfo_generated.cpp @@ -1203,14 +1203,6 @@ unsigned interceptor_ICJI::getClassDomainID( return original_ICorJitInfo->getClassDomainID(cls, ppIndirection); } -void* interceptor_ICJI::getFieldAddress( - CORINFO_FIELD_HANDLE field, - void** ppIndirection) -{ - mcs->AddCall("getFieldAddress"); - return original_ICorJitInfo->getFieldAddress(field, ppIndirection); -} - bool interceptor_ICJI::getReadonlyStaticFieldValue( CORINFO_FIELD_HANDLE field, uint8_t* buffer, diff --git a/src/coreclr/tools/superpmi/superpmi-shim-simple/icorjitinfo_generated.cpp b/src/coreclr/tools/superpmi/superpmi-shim-simple/icorjitinfo_generated.cpp index 6f5367b1fbff6..2370627d38007 100644 --- a/src/coreclr/tools/superpmi/superpmi-shim-simple/icorjitinfo_generated.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shim-simple/icorjitinfo_generated.cpp @@ -1053,13 +1053,6 @@ unsigned interceptor_ICJI::getClassDomainID( return original_ICorJitInfo->getClassDomainID(cls, ppIndirection); } -void* interceptor_ICJI::getFieldAddress( - CORINFO_FIELD_HANDLE field, - void** ppIndirection) -{ - return original_ICorJitInfo->getFieldAddress(field, ppIndirection); -} - bool interceptor_ICJI::getReadonlyStaticFieldValue( CORINFO_FIELD_HANDLE field, uint8_t* buffer, diff --git a/src/coreclr/tools/superpmi/superpmi/icorjitinfo.cpp b/src/coreclr/tools/superpmi/superpmi/icorjitinfo.cpp index cebb456c7afeb..62276060a0be4 100644 --- a/src/coreclr/tools/superpmi/superpmi/icorjitinfo.cpp +++ b/src/coreclr/tools/superpmi/superpmi/icorjitinfo.cpp @@ -1519,13 +1519,6 @@ unsigned MyICJI::getClassDomainID(CORINFO_CLASS_HANDLE cls, void** ppIndirection return jitInstance->mc->repGetClassDomainID(cls, ppIndirection); } -// return the data's address (for static fields only) -void* MyICJI::getFieldAddress(CORINFO_FIELD_HANDLE field, void** ppIndirection) -{ - jitInstance->mc->cr->AddCall("getFieldAddress"); - return jitInstance->mc->repGetFieldAddress(field, ppIndirection); -} - bool MyICJI::getReadonlyStaticFieldValue(CORINFO_FIELD_HANDLE field, uint8_t* buffer, int bufferSize, bool ignoreMovableObjects) { jitInstance->mc->cr->AddCall("getReadonlyStaticFieldValue"); diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index d1559166de244..dc9b19b571b6c 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -1453,43 +1453,6 @@ static CorInfoHelpFunc getInstanceFieldHelper(FieldDesc * pField, CORINFO_ACCESS return (CorInfoHelpFunc)helper; } -static OBJECTREF getFrozenBoxedStatic(FieldDesc* field) -{ - CONTRACTL { - THROWS; - GC_TRIGGERS; - MODE_COOPERATIVE; - } CONTRACTL_END; - - // These fields never hold frozen boxed statics - if (!field->IsStatic() || !field->IsByValue() || field->IsSpecialStatic()) - { - return NULL; - } - - MethodTable* owningType = field->GetEnclosingMethodTable(); - if (!owningType->IsClassInited() || owningType->IsSharedByGenericInstantiations()) - { - return NULL; - } - - PTR_BYTE basePtr = field->GetBase(); - _ASSERTE(basePtr != nullptr); - - Object** handle = (Object**)field->GetStaticAddressHandle(basePtr); - _ASSERT(handle != nullptr); - Object* frozenObj = *handle; - - // ContainsPointers here is unnecessary but it's cheaper than IsInFrozenSegment - // for structs containing gc handles - if (frozenObj != nullptr && !frozenObj->GetMethodTable()->ContainsPointers() && - GCHeapUtilities::GetGCHeap()->IsInFrozenSegment(frozenObj)) - { - return ObjectToOBJECTREF(frozenObj); - } - return NULL; -} - /*********************************************************************/ void CEEInfo::getFieldInfo (CORINFO_RESOLVED_TOKEN * pResolvedToken, CORINFO_METHOD_HANDLE callerHandle, @@ -11976,54 +11939,6 @@ InfoAccessType CEEJitInfo::emptyStringLiteral(void ** ppValue) return result; } -/*********************************************************************/ -void* CEEJitInfo::getFieldAddress(CORINFO_FIELD_HANDLE fieldHnd, - void **ppIndirection) -{ - CONTRACTL { - THROWS; - GC_TRIGGERS; - MODE_PREEMPTIVE; - } CONTRACTL_END; - - void *result = NULL; - - if (ppIndirection != NULL) - *ppIndirection = NULL; - - JIT_TO_EE_TRANSITION(); - - FieldDesc* field = (FieldDesc*) fieldHnd; - - MethodTable* pMT = field->GetEnclosingMethodTable(); - - _ASSERTE(!pMT->ContainsGenericVariables()); - - void *base = NULL; - - if (!field->IsRVA()) - { - // @todo: assert that the current method being compiled is unshared - // We must not call here for statics of collectible types. - _ASSERTE(!pMT->Collectible()); - - // Allocate space for the local class if necessary, but don't trigger - // class construction. - DomainLocalModule *pLocalModule = pMT->GetDomainLocalModule(); - pLocalModule->PopulateClass(pMT); - - GCX_COOP(); - - base = (void *) field->GetBase(); - } - - result = field->GetStaticAddressHandle(base); - - EE_TO_JIT_TRANSITION(); - - return result; -} - bool CEEInfo::getReadonlyStaticFieldValue(CORINFO_FIELD_HANDLE fieldHnd, uint8_t* buffer, int bufferSize, bool ignoreMovableObjects) { CONTRACTL { @@ -14575,33 +14490,6 @@ InfoAccessType CEEInfo::emptyStringLiteral(void ** ppValue) UNREACHABLE(); // only called on derived class. } -void* CEEInfo::getFieldAddress(CORINFO_FIELD_HANDLE fieldHnd, - void **ppIndirection) -{ - CONTRACTL{ - THROWS; - GC_TRIGGERS; - MODE_PREEMPTIVE; - } CONTRACTL_END; - - void *result = NULL; - - if (ppIndirection != NULL) - *ppIndirection = NULL; - - JIT_TO_EE_TRANSITION(); - - FieldDesc* field = (FieldDesc*)fieldHnd; - - _ASSERTE(field->IsRVA()); - - result = field->GetStaticAddressHandle(NULL); - - EE_TO_JIT_TRANSITION(); - - return result; -} - CORINFO_CLASS_HANDLE CEEInfo::getStaticFieldCurrentClass(CORINFO_FIELD_HANDLE fieldHnd, bool* pIsSpeculative) { diff --git a/src/coreclr/vm/jitinterface.h b/src/coreclr/vm/jitinterface.h index c0fcb0a92fb72..4460c413b76e6 100644 --- a/src/coreclr/vm/jitinterface.h +++ b/src/coreclr/vm/jitinterface.h @@ -944,7 +944,6 @@ class CEEJitInfo : public CEEInfo InfoAccessType constructStringLiteral(CORINFO_MODULE_HANDLE scopeHnd, mdToken metaTok, void **ppValue) override final; InfoAccessType emptyStringLiteral(void ** ppValue) override final; - void* getFieldAddress(CORINFO_FIELD_HANDLE field, void **ppIndirection) override final; CORINFO_CLASS_HANDLE getStaticFieldCurrentClass(CORINFO_FIELD_HANDLE field, bool* pIsSpeculative) override final; void* getMethodSync(CORINFO_METHOD_HANDLE ftnHnd, void **ppIndirection) override final; From bff3b1f9dd092d2305d5c160e94abc1e7593e102 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Tue, 8 Nov 2022 20:02:53 +0100 Subject: [PATCH 21/33] Address feedback --- src/coreclr/jit/gentree.cpp | 12 ++++++++++++ src/coreclr/jit/gentree.h | 6 ++++++ src/coreclr/jit/importer.cpp | 4 ++-- src/coreclr/vm/jitinterface.cpp | 2 +- 4 files changed, 21 insertions(+), 3 deletions(-) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 30abcb301f6b4..e125a3d5e5dad 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -18022,6 +18022,18 @@ CORINFO_CLASS_HANDLE Compiler::gtGetClassHandle(GenTree* tree, bool* pIsExact, b } } } + else if (tree->TypeIs(TYP_REF) && base->IsCnsIntOrI()) + { + if (base->IsIconHandle(GTF_ICON_CONST_PTR, GTF_ICON_STATIC_HDL) && + (base->AsIntCon()->gtFieldSeq != nullptr)) + { + CORINFO_FIELD_HANDLE fldHandle = base->AsIntCon()->gtFieldSeq->GetFieldHandle(); + if (fldHandle != nullptr) + { + objClass = gtGetFieldClassHandle(fldHandle, pIsExact, pIsNonNull); + } + } + } } break; diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index aa9a9c8d04516..10424aef76bfa 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -2210,6 +2210,12 @@ struct GenTree return (gtOper == GT_CNS_INT) && ((gtFlags & GTF_ICON_HDL_MASK) == handleType); } + template + 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 index 8ac373fe11936..487f44064d007 100644 --- 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) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index dc9b19b571b6c..3f5fc080d31aa 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -1562,7 +1562,7 @@ void CEEInfo::getFieldInfo (CORINFO_RESOLVED_TOKEN * pResolvedToken, pResult->fieldLookup.addr = pField->GetStaticAddressHandle((void*)pField->GetBase()); pResult->fieldLookup.accessType = IAT_VALUE; - if ((fieldFlags & (CORINFO_FLG_FIELD_STATIC_IN_HEAP | CORINFO_FLG_FIELD_INITCLASS)) == CORINFO_FLG_FIELD_STATIC_IN_HEAP) + if (fieldFlags & CORINFO_FLG_FIELD_STATIC_IN_HEAP) { Object* frozenObj = *(Object**)pResult->fieldLookup.addr; From cfdb18a6287d183cd62112d6dde76c8ad28f66b6 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Wed, 9 Nov 2022 14:22:45 +0100 Subject: [PATCH 22/33] Implement JIT/EE level cache for fields' addresses --- src/coreclr/jit/gentree.cpp | 11 +++------ src/coreclr/vm/jitinterface.cpp | 44 ++++++++++++++++++++++++--------- src/coreclr/vm/jitinterface.h | 13 ++++++++++ 3 files changed, 49 insertions(+), 19 deletions(-) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index e125a3d5e5dad..c3abb6a142080 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -18022,16 +18022,13 @@ CORINFO_CLASS_HANDLE Compiler::gtGetClassHandle(GenTree* tree, bool* pIsExact, b } } } - else if (tree->TypeIs(TYP_REF) && base->IsCnsIntOrI()) + else if (tree->TypeIs(TYP_REF) && base->IsIconHandle(GTF_ICON_CONST_PTR, GTF_ICON_STATIC_HDL)) { - if (base->IsIconHandle(GTF_ICON_CONST_PTR, GTF_ICON_STATIC_HDL) && - (base->AsIntCon()->gtFieldSeq != nullptr)) + FieldSeq* fldSeq = base->AsIntCon()->gtFieldSeq; + if ((fldSeq != nullptr) && (fldSeq->GetOffset() == base->AsIntCon()->IconValue())) { CORINFO_FIELD_HANDLE fldHandle = base->AsIntCon()->gtFieldSeq->GetFieldHandle(); - if (fldHandle != nullptr) - { - objClass = gtGetFieldClassHandle(fldHandle, pIsExact, pIsNonNull); - } + objClass = gtGetFieldClassHandle(fldHandle, pIsExact, pIsNonNull); } } } diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 3f5fc080d31aa..ef690216f63a3 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -1557,24 +1557,44 @@ void CEEInfo::getFieldInfo (CORINFO_RESOLVED_TOKEN * pResolvedToken, if (!pFieldMT->IsClassInited()) fieldFlags |= CORINFO_FLG_FIELD_INITCLASS; - GCX_COOP(); - - pResult->fieldLookup.addr = pField->GetStaticAddressHandle((void*)pField->GetBase()); - pResult->fieldLookup.accessType = IAT_VALUE; + if (m_fieldHandleAddressMap == nullptr) + { + m_fieldHandleAddressMap = new FieldHandleAddressMap(); + } - if (fieldFlags & CORINFO_FLG_FIELD_STATIC_IN_HEAP) + // For boxed statics, depending on field's owning type's initialization state, we return either + // address of its pinned handle or direct address of the object holding the boxed struct + // in case if it's frozen. However, we don't want to confuse JIT and send it different addresses + // for the same field as part of the same JIT compilation, hence, the cache. + FieldAddress fldAddr; + if (!m_fieldHandleAddressMap->Lookup(pResolvedToken->hField, &fldAddr)) { - Object* frozenObj = *(Object**)pResult->fieldLookup.addr; + GCX_COOP(); + + fldAddr.address = pField->GetStaticAddressHandle((void*)pField->GetBase()); + fldAddr.frozen = false; - // ContainsPointers here is unnecessary but it's cheaper than IsInFrozenSegment - // for structs containing gc handles - if (frozenObj != nullptr && !frozenObj->GetMethodTable()->ContainsPointers() && - GCHeapUtilities::GetGCHeap()->IsInFrozenSegment(frozenObj)) + if (fieldFlags & CORINFO_FLG_FIELD_STATIC_IN_HEAP) { - pResult->fieldLookup.addr = frozenObj->GetData(); - fieldFlags &= ~CORINFO_FLG_FIELD_STATIC_IN_HEAP; + Object* frozenObj = VolatileLoad((Object**)fldAddr.address); + + // ContainsPointers here is unnecessary but it's cheaper than IsInFrozenSegment + // for structs containing gc handles + if (frozenObj != nullptr && !frozenObj->GetMethodTable()->ContainsPointers() && + GCHeapUtilities::GetGCHeap()->IsInFrozenSegment(frozenObj)) + { + fldAddr.address = frozenObj->GetData(); + fldAddr.frozen = true; + } } } + + pResult->fieldLookup.addr = fldAddr.address; + pResult->fieldLookup.accessType = IAT_VALUE; + if (fldAddr.frozen) + { + fieldFlags &= ~CORINFO_FLG_FIELD_STATIC_IN_HEAP; + } } } diff --git a/src/coreclr/vm/jitinterface.h b/src/coreclr/vm/jitinterface.h index 4460c413b76e6..f9476886aa25d 100644 --- a/src/coreclr/vm/jitinterface.h +++ b/src/coreclr/vm/jitinterface.h @@ -494,6 +494,7 @@ class CEEInfo : public ICorJitInfo CORINFO_CLASS_HANDLE *clsRet = NULL /* optional out */ ); CEEInfo(MethodDesc * fd = NULL, bool fAllowInlining = true) : + m_fieldHandleAddressMap(nullptr), m_pJitHandles(nullptr), m_pMethodBeingCompiled(fd), m_pThread(GetThreadNULLOk()), @@ -524,6 +525,11 @@ class CEEInfo : public ICorJitInfo delete m_pJitHandles; m_pJitHandles = nullptr; } + + if (m_fieldHandleAddressMap != nullptr) + { + delete m_fieldHandleAddressMap; + } #endif } @@ -585,6 +591,13 @@ class CEEInfo : public ICorJitInfo #endif protected: + struct FieldAddress + { + void* address; + bool frozen; + }; + typedef MapSHash FieldHandleAddressMap; + FieldHandleAddressMap* m_fieldHandleAddressMap; SArray* m_pJitHandles; // GC handles used by JIT MethodDesc* m_pMethodBeingCompiled; // Top-level method being compiled Thread * m_pThread; // Cached current thread for faster JIT-EE transitions From f6f827d531f979c1e85784dee3b78241496319bc Mon Sep 17 00:00:00 2001 From: EgorBo Date: Wed, 9 Nov 2022 14:24:14 +0100 Subject: [PATCH 23/33] Implement JIT/EE level cache for fields' addresses --- src/coreclr/vm/jitinterface.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index ef690216f63a3..27d28a98b454e 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -1587,6 +1587,8 @@ void CEEInfo::getFieldInfo (CORINFO_RESOLVED_TOKEN * pResolvedToken, fldAddr.frozen = true; } } + + m_fieldHandleAddressMap->Add(pResolvedToken->hField, fldAddr); } pResult->fieldLookup.addr = fldAddr.address; From 3b5c44f31cb67729fbf3ec56db1d3d0882585557 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Wed, 9 Nov 2022 16:08:39 +0100 Subject: [PATCH 24/33] Fix CQ regression around EqualityComparer.Default --- src/coreclr/jit/fginline.cpp | 2 +- src/coreclr/jit/gentree.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/fginline.cpp b/src/coreclr/jit/fginline.cpp index 9922e2cec437c..b36ff633335f3 100644 --- a/src/coreclr/jit/fginline.cpp +++ b/src/coreclr/jit/fginline.cpp @@ -1608,7 +1608,7 @@ 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, GT_FIELD)) && ((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 index c3abb6a142080..f1ebaca50a785 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -18028,7 +18028,7 @@ CORINFO_CLASS_HANDLE Compiler::gtGetClassHandle(GenTree* tree, bool* pIsExact, b if ((fldSeq != nullptr) && (fldSeq->GetOffset() == base->AsIntCon()->IconValue())) { CORINFO_FIELD_HANDLE fldHandle = base->AsIntCon()->gtFieldSeq->GetFieldHandle(); - objClass = gtGetFieldClassHandle(fldHandle, pIsExact, pIsNonNull); + objClass = gtGetFieldClassHandle(fldHandle, pIsExact, pIsNonNull); } } } From c5d6b55b1bf2e9c1642a9749fa737de1a0bc7862 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Wed, 9 Nov 2022 21:08:32 +0100 Subject: [PATCH 25/33] Address Jan's feedback --- src/coreclr/inc/CrstTypes.def | 4 ++ src/coreclr/inc/crsttypes_generated.h | 47 +++++++++++--------- src/coreclr/vm/appdomain.cpp | 1 + src/coreclr/vm/appdomain.hpp | 7 +++ src/coreclr/vm/jitinterface.cpp | 50 ++++++++------------- src/coreclr/vm/jitinterface.h | 6 --- src/coreclr/vm/methodtable.cpp | 64 ++++++++++++++++++--------- src/coreclr/vm/methodtable.h | 1 + 8 files changed, 100 insertions(+), 80 deletions(-) diff --git a/src/coreclr/inc/CrstTypes.def b/src/coreclr/inc/CrstTypes.def index 2179a45cadf62..dc2e40e100daa 100644 --- a/src/coreclr/inc/CrstTypes.def +++ b/src/coreclr/inc/CrstTypes.def @@ -577,3 +577,7 @@ End Crst PgoData AcquiredBefore LoaderHeap End + +Crst StaticBoxInit + Unordered +End diff --git a/src/coreclr/inc/crsttypes_generated.h b/src/coreclr/inc/crsttypes_generated.h index 366e60cf9d29f..93cd272894ba0 100644 --- 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/vm/appdomain.cpp b/src/coreclr/vm/appdomain.cpp index fe4e901de5719..2a1f13fcc79cc 100644 --- a/src/coreclr/vm/appdomain.cpp +++ b/src/coreclr/vm/appdomain.cpp @@ -637,6 +637,7 @@ void BaseDomain::Init() m_NativeTypeLoadLock.Init(CrstInteropData, CrstFlags(CRST_REENTRANCY), TRUE); m_crstLoaderAllocatorReferences.Init(CrstLoaderAllocatorReferences); + m_crstStaticBoxInitLock.Init(CrstStaticBoxInit); // Has to switch thread to GC_NOTRIGGER while being held (see code:BaseDomain#AssemblyListLock) m_crstAssemblyList.Init(CrstAssemblyList, CrstFlags( CRST_GC_NOTRIGGER_WHEN_TAKEN | CRST_DEBUGGER_THREAD | CRST_TAKEN_DURING_SHUTDOWN)); diff --git a/src/coreclr/vm/appdomain.hpp b/src/coreclr/vm/appdomain.hpp index 0dbff5efbf6b7..d26165d53f0f5 100644 --- a/src/coreclr/vm/appdomain.hpp +++ b/src/coreclr/vm/appdomain.hpp @@ -1088,6 +1088,12 @@ class BaseDomain return &m_crstLoaderAllocatorReferences; } + CrstExplicitInit* GetStaticBoxInitLock() + { + LIMITED_METHOD_CONTRACT; + return &m_crstStaticBoxInitLock; + } + static CrstStatic* GetMethodTableExposedClassObjectLock() { LIMITED_METHOD_CONTRACT; @@ -1112,6 +1118,7 @@ class BaseDomain CrstExplicitInit m_DomainLocalBlockCrst; // Used to protect the reference lists in the collectible loader allocators attached to this appdomain CrstExplicitInit m_crstLoaderAllocatorReferences; + CrstExplicitInit m_crstStaticBoxInitLock; //#AssemblyListLock // Used to protect the assembly list. Taken also by GC or debugger thread, therefore we have to avoid diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 27d28a98b454e..ccfc5c57be33f 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -1557,46 +1557,32 @@ void CEEInfo::getFieldInfo (CORINFO_RESOLVED_TOKEN * pResolvedToken, if (!pFieldMT->IsClassInited()) fieldFlags |= CORINFO_FLG_FIELD_INITCLASS; - if (m_fieldHandleAddressMap == nullptr) - { - m_fieldHandleAddressMap = new FieldHandleAddressMap(); - } + GCX_COOP(); - // For boxed statics, depending on field's owning type's initialization state, we return either - // address of its pinned handle or direct address of the object holding the boxed struct - // in case if it's frozen. However, we don't want to confuse JIT and send it different addresses - // for the same field as part of the same JIT compilation, hence, the cache. - FieldAddress fldAddr; - if (!m_fieldHandleAddressMap->Lookup(pResolvedToken->hField, &fldAddr)) + pResult->fieldLookup.addr = pField->GetStaticAddressHandle((void*)pField->GetBase()); + if (fieldFlags & CORINFO_FLG_FIELD_STATIC_IN_HEAP) { - GCX_COOP(); - - fldAddr.address = pField->GetStaticAddressHandle((void*)pField->GetBase()); - fldAddr.frozen = false; + Object* frozenObj = VolatileLoad((Object**)pResult->fieldLookup.addr); - if (fieldFlags & CORINFO_FLG_FIELD_STATIC_IN_HEAP) + if (frozenObj == nullptr) { - Object* frozenObj = VolatileLoad((Object**)fldAddr.address); - - // ContainsPointers here is unnecessary but it's cheaper than IsInFrozenSegment - // for structs containing gc handles - if (frozenObj != nullptr && !frozenObj->GetMethodTable()->ContainsPointers() && - GCHeapUtilities::GetGCHeap()->IsInFrozenSegment(frozenObj)) - { - fldAddr.address = frozenObj->GetData(); - fldAddr.frozen = true; - } + // Boxed static is not yet set, allocate it + pFieldMT->AllocateRegularStaticBox(pField, (BYTE*)pResult->fieldLookup.addr); + frozenObj = VolatileLoad((Object**)pResult->fieldLookup.addr); } - m_fieldHandleAddressMap->Add(pResolvedToken->hField, fldAddr); - } + _ASSERT(frozenObj != nullptr); - pResult->fieldLookup.addr = fldAddr.address; - pResult->fieldLookup.accessType = IAT_VALUE; - if (fldAddr.frozen) - { - fieldFlags &= ~CORINFO_FLG_FIELD_STATIC_IN_HEAP; + // ContainsPointers here is unnecessary but it's cheaper than IsInFrozenSegment + // for structs containing gc handles + if (frozenObj != nullptr && !frozenObj->GetMethodTable()->ContainsPointers() && + GCHeapUtilities::GetGCHeap()->IsInFrozenSegment(frozenObj)) + { + pResult->fieldLookup.addr = frozenObj->GetData(); + fieldFlags &= ~CORINFO_FLG_FIELD_STATIC_IN_HEAP; + } } + pResult->fieldLookup.accessType = IAT_VALUE; } } diff --git a/src/coreclr/vm/jitinterface.h b/src/coreclr/vm/jitinterface.h index f9476886aa25d..f576e81d4d440 100644 --- a/src/coreclr/vm/jitinterface.h +++ b/src/coreclr/vm/jitinterface.h @@ -494,7 +494,6 @@ class CEEInfo : public ICorJitInfo CORINFO_CLASS_HANDLE *clsRet = NULL /* optional out */ ); CEEInfo(MethodDesc * fd = NULL, bool fAllowInlining = true) : - m_fieldHandleAddressMap(nullptr), m_pJitHandles(nullptr), m_pMethodBeingCompiled(fd), m_pThread(GetThreadNULLOk()), @@ -525,11 +524,6 @@ class CEEInfo : public ICorJitInfo delete m_pJitHandles; m_pJitHandles = nullptr; } - - if (m_fieldHandleAddressMap != nullptr) - { - delete m_fieldHandleAddressMap; - } #endif } diff --git a/src/coreclr/vm/methodtable.cpp b/src/coreclr/vm/methodtable.cpp index 9990451e7bada..e3578f2632b97 100644 --- a/src/coreclr/vm/methodtable.cpp +++ b/src/coreclr/vm/methodtable.cpp @@ -3484,7 +3484,6 @@ void MethodTable::AllocateRegularStaticBoxes() PTR_BYTE pStaticBase = GetGCStaticsBasePointer(); GCPROTECT_BEGININTERIOR(pStaticBase); - { FieldDesc *pField = HasGenericsStaticsInfo() ? GetGenericsStaticFieldDescs() : (GetApproxFieldDescListRaw() + GetNumIntroducedInstanceFields()); @@ -3492,34 +3491,59 @@ void MethodTable::AllocateRegularStaticBoxes() while (pField < pFieldEnd) { - _ASSERTE(pField->IsStatic()); - if (!pField->IsSpecialStatic() && pField->IsByValue()) { - TypeHandle th = pField->GetFieldTypeHandleThrowing(); - MethodTable* pFieldMT = th.GetMethodTable(); + AllocateRegularStaticBox(pField, pStaticBase); + } + pField++; + } + } + GCPROTECT_END(); +} - LOG((LF_CLASSLOADER, LL_INFO10000, "\tInstantiating static of type %s\n", pFieldMT->GetDebugClassName())); +void MethodTable::AllocateRegularStaticBox(FieldDesc* pField, BYTE* pStaticBase) +{ + CONTRACTL + { + THROWS; + GC_TRIGGERS; + MODE_COOPERATIVE; + CONTRACTL_END; + } + _ASSERT(pField->IsStatic() && !pField->IsSpecialStatic() && pField->IsByValue()); + _ASSERT(pStaticBase == GetGCStaticsBasePointer()); - bool canBeFrozen = !pFieldMT->ContainsPointers() && !Collectible(); + Object** boxedStaticHandle = (Object**)(pStaticBase + pField->GetOffset()); -#ifdef FEATURE_64BIT_ALIGNMENT - if (pFieldMT->RequiresAlign8()) - { - // 64bit alignment is not yet supported in FOH for 32bit targets - canBeFrozen = false; - } -#endif + if (VolatileLoad(boxedStaticHandle) != nullptr) + { + // Boxed static is already initialized + return; + } - OBJECTREF obj = AllocateStaticBox(pFieldMT, HasFixedAddressVTStatics(), NULL, canBeFrozen); + // Taking a lock since we might come here from multiple threads/places + CrstHolder crst(GetAppDomain()->GetStaticBoxInitLock()); - SetObjectReference( (OBJECTREF*)(pStaticBase + pField->GetOffset()), obj); - } + // double-checked locking + if (VolatileLoad(boxedStaticHandle) != nullptr) + { + // Boxed static is already initialized + return; + } - pField++; - } + TypeHandle th = pField->GetFieldTypeHandleThrowing(); + MethodTable* pFieldMT = th.GetMethodTable(); + LOG((LF_CLASSLOADER, LL_INFO10000, "\tInstantiating static of type %s\n", pFieldMT->GetDebugClassName())); + bool canBeFrozen = !pFieldMT->ContainsPointers() && !Collectible(); +#ifdef FEATURE_64BIT_ALIGNMENT + if (pFieldMT->RequiresAlign8()) + { + // 64bit alignment is not yet supported in FOH for 32bit targets + canBeFrozen = false; } - GCPROTECT_END(); +#endif + OBJECTREF obj = AllocateStaticBox(pFieldMT, HasFixedAddressVTStatics(), NULL, canBeFrozen); + SetObjectReference((OBJECTREF*)boxedStaticHandle, obj); } //========================================================================================== diff --git a/src/coreclr/vm/methodtable.h b/src/coreclr/vm/methodtable.h index de4808ecc80eb..0ba0143f80dad 100644 --- a/src/coreclr/vm/methodtable.h +++ b/src/coreclr/vm/methodtable.h @@ -831,6 +831,7 @@ class MethodTable // instantiations of the superclass or interfaces e.g. System.Int32 : IComparable void AllocateRegularStaticBoxes(); + void AllocateRegularStaticBox(FieldDesc* pField, BYTE* pStaticBase); static OBJECTREF AllocateStaticBox(MethodTable* pFieldMT, BOOL fPinned, OBJECTHANDLE* pHandle = 0, bool canBeFrozen = false); void CheckRestore(); From e25cfa3b604971955e12a6aabcdb362e9bedcc23 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Wed, 9 Nov 2022 21:48:02 +0100 Subject: [PATCH 26/33] Update methodtable.cpp --- src/coreclr/vm/methodtable.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/coreclr/vm/methodtable.cpp b/src/coreclr/vm/methodtable.cpp index e3578f2632b97..b46472e3fe3cb 100644 --- a/src/coreclr/vm/methodtable.cpp +++ b/src/coreclr/vm/methodtable.cpp @@ -3511,7 +3511,6 @@ void MethodTable::AllocateRegularStaticBox(FieldDesc* pField, BYTE* pStaticBase) CONTRACTL_END; } _ASSERT(pField->IsStatic() && !pField->IsSpecialStatic() && pField->IsByValue()); - _ASSERT(pStaticBase == GetGCStaticsBasePointer()); Object** boxedStaticHandle = (Object**)(pStaticBase + pField->GetOffset()); From 890ab36814b5bbc9a7389e6f33828aec67f7195c Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Wed, 9 Nov 2022 22:51:32 +0100 Subject: [PATCH 27/33] Apply suggestions from code review Co-authored-by: Jan Kotas --- src/coreclr/vm/jitinterface.cpp | 2 +- src/coreclr/vm/jitinterface.h | 7 ------- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index ccfc5c57be33f..3b75755d629b9 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -1575,7 +1575,7 @@ void CEEInfo::getFieldInfo (CORINFO_RESOLVED_TOKEN * pResolvedToken, // ContainsPointers here is unnecessary but it's cheaper than IsInFrozenSegment // for structs containing gc handles - if (frozenObj != nullptr && !frozenObj->GetMethodTable()->ContainsPointers() && + if (!frozenObj->GetMethodTable()->ContainsPointers() && GCHeapUtilities::GetGCHeap()->IsInFrozenSegment(frozenObj)) { pResult->fieldLookup.addr = frozenObj->GetData(); diff --git a/src/coreclr/vm/jitinterface.h b/src/coreclr/vm/jitinterface.h index f576e81d4d440..4460c413b76e6 100644 --- a/src/coreclr/vm/jitinterface.h +++ b/src/coreclr/vm/jitinterface.h @@ -585,13 +585,6 @@ class CEEInfo : public ICorJitInfo #endif protected: - struct FieldAddress - { - void* address; - bool frozen; - }; - typedef MapSHash FieldHandleAddressMap; - FieldHandleAddressMap* m_fieldHandleAddressMap; SArray* m_pJitHandles; // GC handles used by JIT MethodDesc* m_pMethodBeingCompiled; // Top-level method being compiled Thread * m_pThread; // Cached current thread for faster JIT-EE transitions From c100079bf0e2a61918a9446fc476907524b9ff75 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Wed, 9 Nov 2022 23:40:23 +0100 Subject: [PATCH 28/33] Address feedback --- src/coreclr/inc/CrstTypes.def | 2 +- src/coreclr/vm/methodtable.cpp | 10 ++++++---- src/coreclr/vm/methodtable.h | 2 +- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/coreclr/inc/CrstTypes.def b/src/coreclr/inc/CrstTypes.def index dc2e40e100daa..7ea873e18a374 100644 --- a/src/coreclr/inc/CrstTypes.def +++ b/src/coreclr/inc/CrstTypes.def @@ -579,5 +579,5 @@ Crst PgoData End Crst StaticBoxInit - Unordered + AcquiredBefore LoaderHeap FrozenObjectHeap End diff --git a/src/coreclr/vm/methodtable.cpp b/src/coreclr/vm/methodtable.cpp index b46472e3fe3cb..121774a513e68 100644 --- a/src/coreclr/vm/methodtable.cpp +++ b/src/coreclr/vm/methodtable.cpp @@ -3493,7 +3493,7 @@ void MethodTable::AllocateRegularStaticBoxes() { if (!pField->IsSpecialStatic() && pField->IsByValue()) { - AllocateRegularStaticBox(pField, pStaticBase); + AllocateRegularStaticBox(pField, pStaticBase + pField->GetOffset()); } pField++; } @@ -3501,7 +3501,7 @@ void MethodTable::AllocateRegularStaticBoxes() GCPROTECT_END(); } -void MethodTable::AllocateRegularStaticBox(FieldDesc* pField, BYTE* pStaticBase) +void MethodTable::AllocateRegularStaticBox(FieldDesc* pField, BYTE* fieldAddress) { CONTRACTL { @@ -3512,7 +3512,7 @@ void MethodTable::AllocateRegularStaticBox(FieldDesc* pField, BYTE* pStaticBase) } _ASSERT(pField->IsStatic() && !pField->IsSpecialStatic() && pField->IsByValue()); - Object** boxedStaticHandle = (Object**)(pStaticBase + pField->GetOffset()); + Object** boxedStaticHandle = reinterpret_cast(fieldAddress); if (VolatileLoad(boxedStaticHandle) != nullptr) { @@ -3520,6 +3520,9 @@ void MethodTable::AllocateRegularStaticBox(FieldDesc* pField, BYTE* pStaticBase) return; } + // Grab field's type handle before we enter lock + TypeHandle th = pField->GetFieldTypeHandleThrowing(); + // Taking a lock since we might come here from multiple threads/places CrstHolder crst(GetAppDomain()->GetStaticBoxInitLock()); @@ -3530,7 +3533,6 @@ void MethodTable::AllocateRegularStaticBox(FieldDesc* pField, BYTE* pStaticBase) return; } - TypeHandle th = pField->GetFieldTypeHandleThrowing(); MethodTable* pFieldMT = th.GetMethodTable(); LOG((LF_CLASSLOADER, LL_INFO10000, "\tInstantiating static of type %s\n", pFieldMT->GetDebugClassName())); bool canBeFrozen = !pFieldMT->ContainsPointers() && !Collectible(); diff --git a/src/coreclr/vm/methodtable.h b/src/coreclr/vm/methodtable.h index 0ba0143f80dad..e7e24c8d8ae6a 100644 --- a/src/coreclr/vm/methodtable.h +++ b/src/coreclr/vm/methodtable.h @@ -831,7 +831,7 @@ class MethodTable // instantiations of the superclass or interfaces e.g. System.Int32 : IComparable void AllocateRegularStaticBoxes(); - void AllocateRegularStaticBox(FieldDesc* pField, BYTE* pStaticBase); + void AllocateRegularStaticBox(FieldDesc* pField, BYTE* fieldAddress); static OBJECTREF AllocateStaticBox(MethodTable* pFieldMT, BOOL fPinned, OBJECTHANDLE* pHandle = 0, bool canBeFrozen = false); void CheckRestore(); From 89a99f07cc64834408022a6596cfaec2de73a52f Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 10 Nov 2022 00:56:51 +0100 Subject: [PATCH 29/33] Address SingleAccretion's feedback --- src/coreclr/jit/gentree.cpp | 3 ++- src/coreclr/jit/importer.cpp | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index f1ebaca50a785..934b74bb7a023 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -18022,8 +18022,9 @@ CORINFO_CLASS_HANDLE Compiler::gtGetClassHandle(GenTree* tree, bool* pIsExact, b } } } - else if (tree->TypeIs(TYP_REF) && base->IsIconHandle(GTF_ICON_CONST_PTR, GTF_ICON_STATIC_HDL)) + 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())) { diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 487f44064d007..346e0925ce648 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -4509,7 +4509,7 @@ GenTree* Compiler::impImportStaticFieldAccess(CORINFO_RESOLVED_TOKEN* pResolvedT { // TODO-CQ: enable this optimization for 32 bit targets. #ifdef TARGET_64BIT - if (!isBoxedStatic && (lclTyp == TYP_REF)) + if (!isBoxedStatic && (lclTyp == TYP_REF) && ((access & CORINFO_ACCESS_ADDRESS) == 0)) { bool isSpeculative = true; if ((info.compCompHnd->getStaticFieldCurrentClass(pResolvedToken->hField, &isSpeculative) != From 29ff0dee39164fd5c740f5b6884e46355166cfbf Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 10 Nov 2022 13:23:14 +0100 Subject: [PATCH 30/33] Fix FixedAddressValueType test - it didn't expect frozen statics --- .../FixedAddressValueType.cs | 77 ++++++++----------- 1 file changed, 33 insertions(+), 44 deletions(-) diff --git a/src/tests/baseservices/compilerservices/FixedAddressValueType/FixedAddressValueType.cs b/src/tests/baseservices/compilerservices/FixedAddressValueType/FixedAddressValueType.cs index a8a61132b1972..e8fc1843c3918 100644 --- a/src/tests/baseservices/compilerservices/FixedAddressValueType/FixedAddressValueType.cs +++ b/src/tests/baseservices/compilerservices/FixedAddressValueType/FixedAddressValueType.cs @@ -1,59 +1,48 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. // -using System; using System.Runtime.CompilerServices; -public struct Age { - public int years; - public int months; -} - -public class FreeClass +public struct Age { - public static Age FreeAge; - - public static unsafe IntPtr AddressOfFreeAge() - { - fixed (Age* pointer = &FreeAge) - { return (IntPtr) pointer; } - } + public int years; + public int months; } public class FixedClass { - [FixedAddressValueType] - public static Age FixedAge; - - public static unsafe IntPtr AddressOfFixedAge() - { - fixed (Age* pointer = &FixedAge) - { return (IntPtr) pointer; } - } + [FixedAddressValueType] + public static Age FixedAge; + + public static unsafe IntPtr AddressOfFixedAge() + { + fixed (Age* pointer = &FixedAge) + { + return (IntPtr)pointer; + } + } } public class Example { - public static int Main() - { - // Get addresses of static Age fields. - IntPtr freePtr1 = FreeClass.AddressOfFreeAge(); - - IntPtr fixedPtr1 = FixedClass.AddressOfFixedAge(); - - // Garbage collection. - GC.Collect(3, GCCollectionMode.Forced, true, true); - GC.WaitForPendingFinalizers(); - - // Get addresses of static Age fields after garbage collection. - IntPtr freePtr2 = FreeClass.AddressOfFreeAge(); - IntPtr fixedPtr2 = FixedClass.AddressOfFixedAge(); - - if(freePtr1 != freePtr2 && fixedPtr1 == fixedPtr2) - { - return 100; - } - - return -1; - } + public static int Main() + { + for (int i = 0; i < 1000; i++) + { + IntPtr fixedPtr1 = FixedClass.AddressOfFixedAge(); + + // Garbage collection. + GC.Collect(3, GCCollectionMode.Forced, true, true); + GC.WaitForPendingFinalizers(); + + // Get addresses of static Age fields after garbage collection. + IntPtr fixedPtr2 = FixedClass.AddressOfFixedAge(); + + if (fixedPtr1 != fixedPtr2) + { + return -1; + } + } + return 100; + } } From 71ff5745644eedc611b821fdfacec3a42d2bd249 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Thu, 10 Nov 2022 18:07:06 +0100 Subject: [PATCH 31/33] Update FixedAddressValueType.cs --- .../FixedAddressValueType/FixedAddressValueType.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/tests/baseservices/compilerservices/FixedAddressValueType/FixedAddressValueType.cs b/src/tests/baseservices/compilerservices/FixedAddressValueType/FixedAddressValueType.cs index e8fc1843c3918..56667c70cd292 100644 --- a/src/tests/baseservices/compilerservices/FixedAddressValueType/FixedAddressValueType.cs +++ b/src/tests/baseservices/compilerservices/FixedAddressValueType/FixedAddressValueType.cs @@ -1,6 +1,8 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. // + +using System; using System.Runtime.CompilerServices; public struct Age From c92a6c3a98eacf7999eb4016ddd9b58bba882b77 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sat, 12 Nov 2022 02:12:02 +0100 Subject: [PATCH 32/33] Resolve conflicts --- src/coreclr/inc/jiteeversionguid.h | 10 +- src/coreclr/jit/compiler.h | 1 - src/coreclr/jit/importer.cpp | 26 +---- src/coreclr/jit/morph.cpp | 98 +------------------ .../JitInterface/CorInfoImpl_generated.cs | 63 ++++++------ 5 files changed, 38 insertions(+), 160 deletions(-) diff --git a/src/coreclr/inc/jiteeversionguid.h b/src/coreclr/inc/jiteeversionguid.h index 601ba406d8334..b24885dcbeb37 100644 --- 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/compiler.h b/src/coreclr/jit/compiler.h index a1b860ed99c2e..4b66168ca1494 100644 --- 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/importer.cpp b/src/coreclr/jit/importer.cpp index 17aa6f31d9947..062181d15ea25 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -4505,31 +4505,7 @@ GenTree* Compiler::impImportStaticFieldAccess(CORINFO_RESOLVED_TOKEN* pResolvedT 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 assert(hasConstAddr); assert(pFieldInfo->fieldLookup.addr != nullptr); diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 16b28efe45a01..07d1e6cd4ccf7 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -5033,7 +5033,7 @@ GenTree* Compiler::fgMorphField(GenTree* tree, MorphAddrContext* mac) } else { - tree = fgMorphExpandStaticField(tree); + assert(!"Normal statics are expected to be handled in the importer"); } // Pass down the current mac; if non null we are computing an address @@ -5371,102 +5371,6 @@ GenTree* Compiler::fgMorphExpandTlsFieldAddr(GenTree* tree) return tree; } -//------------------------------------------------------------------------ -// fgMorphExpandStaticField: Expand a simple static field load. -// -// Transforms the field into an explicit indirection off of a constant -// address. -// -// Arguments: -// tree - The GT_FIELD tree -// -// Return Value: -// The expanded tree - a GT_IND. -// -GenTree* Compiler::fgMorphExpandStaticField(GenTree* tree) -{ - // Note we do not support "FIELD_ADDR"s for simple statics. - assert(tree->OperIs(GT_FIELD) && tree->AsField()->IsStatic()); - - // If we can we access the static's address directly - // then pFldAddr will be NULL and - // fldAddr will be the actual address of the static field - // - CORINFO_FIELD_HANDLE fieldHandle = tree->AsField()->gtFldHnd; - void** pFldAddr = nullptr; - void* fldAddr = info.compCompHnd->getFieldAddress(fieldHandle, (void**)&pFldAddr); - - // We should always be able to access this static field address directly - // - assert(pFldAddr == nullptr); - - // For boxed statics, this direct address will be for the box. We have already added - // the indirection for the field itself and attached the sequence, in importation. - FieldSeq* fieldSeq = nullptr; - bool isBoxedStatic = gtIsStaticFieldPtrToBoxedStruct(tree->TypeGet(), fieldHandle); - if (!isBoxedStatic) - { - // Only simple statics get importred as GT_FIELDs. - fieldSeq = GetFieldSeqStore()->Create(fieldHandle, reinterpret_cast(fldAddr), - FieldSeq::FieldKind::SimpleStatic); - } - - // TODO-CQ: enable this optimization for 32 bit targets. - bool isStaticReadOnlyInited = false; -#ifdef TARGET_64BIT - if (tree->TypeIs(TYP_REF) && !isBoxedStatic) - { - bool pIsSpeculative = true; - if (info.compCompHnd->getStaticFieldCurrentClass(fieldHandle, &pIsSpeculative) != NO_CLASS_HANDLE) - { - isStaticReadOnlyInited = !pIsSpeculative; - } - } -#endif // TARGET_64BIT - - GenTreeFlags handleKind = GTF_EMPTY; - if (isBoxedStatic) - { - handleKind = GTF_ICON_STATIC_BOX_PTR; - } - else if (isStaticReadOnlyInited) - { - handleKind = GTF_ICON_CONST_PTR; - } - else - { - handleKind = GTF_ICON_STATIC_HDL; - } - GenTreeIntCon* addr = gtNewIconHandleNode((size_t)fldAddr, handleKind, fieldSeq); - INDEBUG(addr->gtTargetHandle = reinterpret_cast(fieldHandle)); - - // Translate GTF_FLD_INITCLASS to GTF_ICON_INITCLASS, if we need to. - if (((tree->gtFlags & GTF_FLD_INITCLASS) != 0) && !isStaticReadOnlyInited) - { - tree->gtFlags &= ~GTF_FLD_INITCLASS; - addr->gtFlags |= GTF_ICON_INITCLASS; - } - - tree->SetOper(GT_IND); - tree->AsOp()->gtOp1 = addr; - - if (isBoxedStatic) - { - // The box for the static cannot be null, and is logically invariant, since it - // represents (a base for) the static's address. - tree->gtFlags |= (GTF_IND_INVARIANT | GTF_IND_NONFAULTING | GTF_IND_NONNULL); - } - else if (isStaticReadOnlyInited) - { - JITDUMP("Marking initialized static read-only field '%s' as invariant.\n", eeGetFieldName(fieldHandle)); - - // Static readonly field is not null at this point (see getStaticFieldCurrentClass impl). - tree->gtFlags |= (GTF_IND_INVARIANT | GTF_IND_NONFAULTING | GTF_IND_NONNULL); - } - - return tree; -} - //------------------------------------------------------------------------------ // fgMorphCallInline: attempt to inline a call // diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoImpl_generated.cs b/src/coreclr/tools/Common/JitInterface/CorInfoImpl_generated.cs index 7e454963fa8f2..ddc81d77eb7d0 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoImpl_generated.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoImpl_generated.cs @@ -2685,7 +2685,7 @@ private static uint _getJitFlags(IntPtr thisHandle, IntPtr* ppException, CORJIT_ private static IntPtr GetUnmanagedCallbacks() { - void** callbacks = (void**)Marshal.AllocCoTaskMem(sizeof(IntPtr) * 182); + void** callbacks = (void**)Marshal.AllocCoTaskMem(sizeof(IntPtr) * 181); callbacks[0] = (delegate* unmanaged)&_isIntrinsic; callbacks[1] = (delegate* unmanaged)&_getMethodAttribs; @@ -2838,37 +2838,36 @@ private static IntPtr GetUnmanagedCallbacks() callbacks[148] = (delegate* unmanaged)&_canAccessFamily; callbacks[149] = (delegate* unmanaged)&_isRIDClassDomainID; callbacks[150] = (delegate* unmanaged)&_getClassDomainID; - callbacks[151] = (delegate* unmanaged)&_getFieldAddress; - callbacks[152] = (delegate* unmanaged)&_getReadonlyStaticFieldValue; - callbacks[153] = (delegate* unmanaged)&_getStaticFieldCurrentClass; - callbacks[154] = (delegate* unmanaged)&_getVarArgsHandle; - callbacks[155] = (delegate* unmanaged)&_canGetVarArgsHandle; - callbacks[156] = (delegate* unmanaged)&_constructStringLiteral; - callbacks[157] = (delegate* unmanaged)&_emptyStringLiteral; - callbacks[158] = (delegate* unmanaged)&_getFieldThreadLocalStoreID; - callbacks[159] = (delegate* unmanaged)&_addActiveDependency; - callbacks[160] = (delegate* unmanaged)&_GetDelegateCtor; - callbacks[161] = (delegate* unmanaged)&_MethodCompileComplete; - callbacks[162] = (delegate* unmanaged)&_getTailCallHelpers; - callbacks[163] = (delegate* unmanaged)&_convertPInvokeCalliToCall; - callbacks[164] = (delegate* unmanaged)&_notifyInstructionSetUsage; - callbacks[165] = (delegate* unmanaged)&_updateEntryPointForTailCall; - callbacks[166] = (delegate* unmanaged)&_allocMem; - callbacks[167] = (delegate* unmanaged)&_reserveUnwindInfo; - callbacks[168] = (delegate* unmanaged)&_allocUnwindInfo; - callbacks[169] = (delegate* unmanaged)&_allocGCInfo; - callbacks[170] = (delegate* unmanaged)&_setEHcount; - callbacks[171] = (delegate* unmanaged)&_setEHinfo; - callbacks[172] = (delegate* unmanaged)&_logMsg; - callbacks[173] = (delegate* unmanaged)&_doAssert; - callbacks[174] = (delegate* unmanaged)&_reportFatalError; - callbacks[175] = (delegate* unmanaged)&_getPgoInstrumentationResults; - callbacks[176] = (delegate* unmanaged)&_allocPgoInstrumentationBySchema; - callbacks[177] = (delegate* unmanaged)&_recordCallSite; - callbacks[178] = (delegate* unmanaged)&_recordRelocation; - callbacks[179] = (delegate* unmanaged)&_getRelocTypeHint; - callbacks[180] = (delegate* unmanaged)&_getExpectedTargetArchitecture; - callbacks[181] = (delegate* unmanaged)&_getJitFlags; + callbacks[151] = (delegate* unmanaged)&_getReadonlyStaticFieldValue; + callbacks[152] = (delegate* unmanaged)&_getStaticFieldCurrentClass; + callbacks[153] = (delegate* unmanaged)&_getVarArgsHandle; + callbacks[154] = (delegate* unmanaged)&_canGetVarArgsHandle; + callbacks[155] = (delegate* unmanaged)&_constructStringLiteral; + callbacks[156] = (delegate* unmanaged)&_emptyStringLiteral; + callbacks[157] = (delegate* unmanaged)&_getFieldThreadLocalStoreID; + callbacks[158] = (delegate* unmanaged)&_addActiveDependency; + callbacks[159] = (delegate* unmanaged)&_GetDelegateCtor; + callbacks[160] = (delegate* unmanaged)&_MethodCompileComplete; + callbacks[161] = (delegate* unmanaged)&_getTailCallHelpers; + callbacks[162] = (delegate* unmanaged)&_convertPInvokeCalliToCall; + callbacks[163] = (delegate* unmanaged)&_notifyInstructionSetUsage; + callbacks[164] = (delegate* unmanaged)&_updateEntryPointForTailCall; + callbacks[165] = (delegate* unmanaged)&_allocMem; + callbacks[166] = (delegate* unmanaged)&_reserveUnwindInfo; + callbacks[167] = (delegate* unmanaged)&_allocUnwindInfo; + callbacks[168] = (delegate* unmanaged)&_allocGCInfo; + callbacks[169] = (delegate* unmanaged)&_setEHcount; + callbacks[170] = (delegate* unmanaged)&_setEHinfo; + callbacks[171] = (delegate* unmanaged)&_logMsg; + callbacks[172] = (delegate* unmanaged)&_doAssert; + callbacks[173] = (delegate* unmanaged)&_reportFatalError; + callbacks[174] = (delegate* unmanaged)&_getPgoInstrumentationResults; + callbacks[175] = (delegate* unmanaged)&_allocPgoInstrumentationBySchema; + callbacks[176] = (delegate* unmanaged)&_recordCallSite; + callbacks[177] = (delegate* unmanaged)&_recordRelocation; + callbacks[178] = (delegate* unmanaged)&_getRelocTypeHint; + callbacks[179] = (delegate* unmanaged)&_getExpectedTargetArchitecture; + callbacks[180] = (delegate* unmanaged)&_getJitFlags; return (IntPtr)callbacks; } From 3db452277836ee6d3a8e87ed352ba721406d78b6 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sun, 13 Nov 2022 15:44:14 +0100 Subject: [PATCH 33/33] Fix "is removable field" check --- src/coreclr/jit/fginline.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/fginline.cpp b/src/coreclr/jit/fginline.cpp index b36ff633335f3..dcd6a8b3bdd12 100644 --- 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->OperIs(GT_IND, 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",