Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allocate boxed static structs on Frozen Object Heap #77737

Merged
merged 41 commits into from
Nov 13, 2022
Merged
Show file tree
Hide file tree
Changes from 32 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
77a42b7
Smarter approach suggested by SingleAccretion
EgorBo Nov 1, 2022
2b2b2ec
Address Jan's feedback, Jakob noticed invalid contract in AllocateSta…
EgorBo Nov 1, 2022
b524f0d
Update src/coreclr/vm/methodtable.cpp
EgorBo Nov 2, 2022
a96a664
Address feedback
EgorBo Nov 2, 2022
7ebf214
Clean up
EgorBo Nov 2, 2022
ec21c92
give up if GetBase() returns nullptr
EgorBo Nov 2, 2022
63e6387
does this help
EgorBo Nov 2, 2022
ca4a77e
disable for foh forced 8byte alignment 32bit targets
EgorBo Nov 2, 2022
16a2438
Update methodtable.cpp
EgorBo Nov 2, 2022
d38e42b
Update src/coreclr/vm/jitinterface.cpp
EgorBo Nov 2, 2022
9868525
Update src/coreclr/vm/jitinterface.cpp
EgorBo Nov 2, 2022
4b7ca2b
Update jitinterface.cpp
EgorBo Nov 2, 2022
8c45daa
try fix
EgorBo Nov 2, 2022
68a130b
Update jitinterface.cpp
EgorBo Nov 2, 2022
20127aa
Merge branch 'main' of github.com:dotnet/runtime into boxed-statics-foh
EgorBo Nov 3, 2022
94ae8f1
Revert to getFieldInfo
EgorBo Nov 4, 2022
1739a55
Use fieldLookup
EgorBo Nov 5, 2022
78448f1
Run tests on CI (WIP), works locally
EgorBo Nov 8, 2022
faa5e51
Delete getFieldAddress
EgorBo Nov 8, 2022
90b3e1b
Fix regression around invariant ref-type statics
EgorBo Nov 8, 2022
6dc5a18
Remove getFieldAddress
EgorBo Nov 8, 2022
8d9f172
Merge branch 'main' of github.com:dotnet/runtime into boxed-statics-foh
EgorBo Nov 8, 2022
bff3b1f
Address feedback
EgorBo Nov 8, 2022
a35f7d1
Merge branch 'main' of github.com:dotnet/runtime into boxed-statics-foh
EgorBo Nov 9, 2022
cfdb18a
Implement JIT/EE level cache for fields' addresses
EgorBo Nov 9, 2022
f6f827d
Implement JIT/EE level cache for fields' addresses
EgorBo Nov 9, 2022
5b3d4fe
Merge branch 'main' of github.com:dotnet/runtime into boxed-statics-foh
EgorBo Nov 9, 2022
3b5c44f
Fix CQ regression around EqualityComparer.Default
EgorBo Nov 9, 2022
c5d6b55
Address Jan's feedback
EgorBo Nov 9, 2022
e25cfa3
Update methodtable.cpp
EgorBo Nov 9, 2022
890ab36
Apply suggestions from code review
EgorBo Nov 9, 2022
c100079
Address feedback
EgorBo Nov 9, 2022
89a99f0
Address SingleAccretion's feedback
EgorBo Nov 9, 2022
29ff0de
Fix FixedAddressValueType test - it didn't expect frozen statics
EgorBo Nov 10, 2022
ff5b6e7
Merge branch 'main' of github.com:dotnet/runtime into boxed-statics-foh
EgorBo Nov 10, 2022
71ff574
Update FixedAddressValueType.cs
EgorBo Nov 10, 2022
d8a7d46
Merge branch 'boxed-statics-foh' of github.com:EgorBo/runtime-1 into …
EgorBo Nov 11, 2022
c6bdcd7
Merge branch 'main' of github.com:dotnet/runtime into boxed-statics-foh
EgorBo Nov 12, 2022
c92a6c3
Resolve conflicts
EgorBo Nov 12, 2022
43c5724
Merge branch 'main' of github.com:dotnet/runtime into boxed-statics-foh
EgorBo Nov 13, 2022
3db4522
Fix "is removable field" check
EgorBo Nov 13, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/coreclr/inc/CrstTypes.def
Original file line number Diff line number Diff line change
Expand Up @@ -577,3 +577,7 @@ End
Crst PgoData
AcquiredBefore LoaderHeap
End

Crst StaticBoxInit
AcquiredBefore LoaderHeap FrozenObjectHeap
End
11 changes: 2 additions & 9 deletions src/coreclr/inc/corinfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ The first 4 options are mutually exclusive
have managed thread local statics, which work through the HELPER. Support for this is considered
legacy, and going forward, the EE should

* <NONE> This is a normal static field. Its address in memory is determined by getFieldAddress. (see
* <NONE> This is a normal static field. Its address in memory is determined by getFieldInfo. (see
also CORINFO_FLG_STATIC_IN_HEAP).


Expand Down Expand Up @@ -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;
};

//----------------------------------------------------------------------------
Expand Down Expand Up @@ -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.
Expand Down
47 changes: 25 additions & 22 deletions src/coreclr/inc/crsttypes_generated.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -241,6 +242,7 @@ int g_rgCrstLevelMap[] =
5, // CrstSingleUseLock
0, // CrstSpecialStatics
0, // CrstStackSampler
-1, // CrstStaticBoxInit
-1, // CrstStressLog
5, // CrstStubCache
0, // CrstStubDispatchCache
Expand Down Expand Up @@ -364,6 +366,7 @@ LPCSTR g_rgCrstNameMap[] =
"CrstSingleUseLock",
"CrstSpecialStatics",
"CrstStackSampler",
"CrstStaticBoxInit",
"CrstStressLog",
"CrstStubCache",
"CrstStubDispatchCache",
Expand Down
4 changes: 0 additions & 4 deletions src/coreclr/inc/icorjitinfoimpl_generated.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
10 changes: 5 additions & 5 deletions src/coreclr/inc/jiteeversionguid.h
Original file line number Diff line number Diff line change
Expand Up @@ -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}
};

//////////////////////////////////////////////////////////////////////////////////////////////////////////
Expand Down
1 change: 0 additions & 1 deletion src/coreclr/jit/ICorJitInfo_names_generated.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
10 changes: 0 additions & 10 deletions src/coreclr/jit/ICorJitInfo_wrapper_generated.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
3 changes: 1 addition & 2 deletions src/coreclr/jit/assertionprop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3197,8 +3197,7 @@ GenTree* Compiler::optVNConstantPropOnTree(BasicBlock* block, GenTree* tree)

case TYP_SIMD32:
{
simd32_t value = vnStore->ConstantValue<simd32_t>(vnCns);

simd32_t value = vnStore->ConstantValue<simd32_t>(vnCns);
GenTreeVecCon* vecCon = gtNewVconNode(tree->TypeGet());
vecCon->gtSimd32Val = value;

Expand Down
5 changes: 1 addition & 4 deletions src/coreclr/jit/emitarm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
14 changes: 2 additions & 12 deletions src/coreclr/jit/emitxarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12981,18 +12981,8 @@ BYTE* emitter::emitOutputCV(BYTE* dst, instrDesc* id, code_t code, CnsVal* addc)
else
{
// Special case: mov reg, fs:[ddd] or mov reg, [ddd]
if (jitStaticFldIsGlobAddr(fldh))
{
addr = nullptr;
}
else
{
addr = (BYTE*)emitComp->info.compCompHnd->getFieldAddress(fldh, nullptr);
if (addr == nullptr)
{
NO_WAY("could not obtain address of static field");
}
}
assert(jitStaticFldIsGlobAddr(fldh));
addr = nullptr;
}

BYTE* target = (addr + offs);
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/fginline.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
9 changes: 9 additions & 0 deletions src/coreclr/jit/gentree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18022,6 +18022,15 @@ 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))
EgorBo marked this conversation as resolved.
Show resolved Hide resolved
{
FieldSeq* fldSeq = base->AsIntCon()->gtFieldSeq;
if ((fldSeq != nullptr) && (fldSeq->GetOffset() == base->AsIntCon()->IconValue()))
{
CORINFO_FIELD_HANDLE fldHandle = base->AsIntCon()->gtFieldSeq->GetFieldHandle();
objClass = gtGetFieldClassHandle(fldHandle, pIsExact, pIsNonNull);
}
}
}

break;
Expand Down
6 changes: 6 additions & 0 deletions src/coreclr/jit/gentree.h
Original file line number Diff line number Diff line change
Expand Up @@ -2210,6 +2210,12 @@ struct GenTree
return (gtOper == GT_CNS_INT) && ((gtFlags & GTF_ICON_HDL_MASK) == handleType);
}

template <typename... T>
bool IsIconHandle(GenTreeFlags handleType, T... rest) const
{
return IsIconHandle(handleType) || IsIconHandle(rest...);
}

// Return just the part of the flags corresponding to the GTF_ICON_*_HDL flag.
// For non-icon handle trees, returns GTF_EMPTY.
GenTreeFlags GetIconHandleFlag() const
Expand Down
93 changes: 42 additions & 51 deletions src/coreclr/jit/importer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -4387,6 +4387,9 @@ GenTree* Compiler::impImportStaticFieldAccess(CORINFO_RESOLVED_TOKEN* pResolvedT
FieldSeq::FieldKind fieldKind =
isSharedStatic ? FieldSeq::FieldKind::SharedStatic : FieldSeq::FieldKind::SimpleStatic;

bool hasConstAddr = (pFieldInfo->fieldAccessor == CORINFO_FIELD_STATIC_ADDRESS) ||
(pFieldInfo->fieldAccessor == CORINFO_FIELD_STATIC_RVA_ADDRESS);

FieldSeq* innerFldSeq;
FieldSeq* outerFldSeq;
if (isBoxedStatic)
Expand All @@ -4396,14 +4399,11 @@ GenTree* Compiler::impImportStaticFieldAccess(CORINFO_RESOLVED_TOKEN* pResolvedT
}
EgorBo marked this conversation as resolved.
Show resolved Hide resolved
else
{
bool hasConstAddr = (pFieldInfo->fieldAccessor == CORINFO_FIELD_STATIC_ADDRESS) ||
(pFieldInfo->fieldAccessor == CORINFO_FIELD_STATIC_RVA_ADDRESS);

ssize_t offset;
if (hasConstAddr)
{
offset = reinterpret_cast<ssize_t>(info.compCompHnd->getFieldAddress(pResolvedToken->hField));
assert(offset != 0);
assert(pFieldInfo->fieldLookup.accessType == IAT_VALUE);
offset = reinterpret_cast<ssize_t>(pFieldInfo->fieldLookup.addr);
}
else
{
Expand All @@ -4414,6 +4414,7 @@ GenTree* Compiler::impImportStaticFieldAccess(CORINFO_RESOLVED_TOKEN* pResolvedT
outerFldSeq = nullptr;
}

bool isStaticReadOnlyInitedRef = false;
GenTree* op1;
switch (pFieldInfo->fieldAccessor)
{
Expand Down Expand Up @@ -4506,54 +4507,41 @@ GenTree* Compiler::impImportStaticFieldAccess(CORINFO_RESOLVED_TOKEN* pResolvedT

default:
{
// Do we need the address of a static field?
//
if (access & CORINFO_ACCESS_ADDRESS)
// TODO-CQ: enable this optimization for 32 bit targets.
EgorBo marked this conversation as resolved.
Show resolved Hide resolved
#ifdef TARGET_64BIT
if (!isBoxedStatic && (lclTyp == TYP_REF))
EgorBo marked this conversation as resolved.
Show resolved Hide resolved
{
void** pFldAddr = nullptr;
void* fldAddr = info.compCompHnd->getFieldAddress(pResolvedToken->hField, (void**)&pFldAddr);

// We should always be able to access this static's address directly.
assert(pFldAddr == nullptr);

// Create the address node.
GenTreeFlags handleKind = isBoxedStatic ? GTF_ICON_STATIC_BOX_PTR : GTF_ICON_STATIC_HDL;
op1 = gtNewIconHandleNode((size_t)fldAddr, handleKind, innerFldSeq);
INDEBUG(op1->AsIntCon()->gtTargetHandle = reinterpret_cast<size_t>(pResolvedToken->hField));

if (pFieldInfo->fieldFlags & CORINFO_FLG_FIELD_INITCLASS)
bool isSpeculative = true;
if ((info.compCompHnd->getStaticFieldCurrentClass(pResolvedToken->hField, &isSpeculative) !=
NO_CLASS_HANDLE))
{
op1->gtFlags |= GTF_ICON_INITCLASS;
isStaticReadOnlyInitedRef = !isSpeculative;
}
}
else // We need the value of a static field
{
// 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);
}
}
#endif // TARGET_64BIT

return op1;
assert(hasConstAddr);
assert(pFieldInfo->fieldLookup.addr != nullptr);
assert(pFieldInfo->fieldLookup.accessType == IAT_VALUE);
size_t fldAddr = reinterpret_cast<size_t>(pFieldInfo->fieldLookup.addr);
GenTreeFlags handleKind;
if (isBoxedStatic)
{
handleKind = GTF_ICON_STATIC_BOX_PTR;
}
else if (isStaticReadOnlyInitedRef)
{
handleKind = GTF_ICON_CONST_PTR;
}
else
{
handleKind = GTF_ICON_STATIC_HDL;
}
op1 = gtNewIconHandleNode(fldAddr, handleKind, innerFldSeq);
EgorBo marked this conversation as resolved.
Show resolved Hide resolved
INDEBUG(op1->AsIntCon()->gtTargetHandle = reinterpret_cast<size_t>(pResolvedToken->hField));
if (pFieldInfo->fieldFlags & CORINFO_FLG_FIELD_INITCLASS)
{
op1->gtFlags |= GTF_ICON_INITCLASS;
}
break;
}
Expand All @@ -4563,7 +4551,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));
}

Expand All @@ -4579,6 +4566,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;
Expand Down
Loading