Skip to content

Commit

Permalink
Revert "Allocate boxed static structs on Frozen Object Heap and remov…
Browse files Browse the repository at this point in the history
…e getFieldAddress (dotnet#77737)" (dotnet#78296)

This reverts commit cabb8b0.
  • Loading branch information
EgorBo committed Nov 14, 2022
1 parent b6deba5 commit 0de4f60
Show file tree
Hide file tree
Showing 39 changed files with 515 additions and 293 deletions.
4 changes: 0 additions & 4 deletions src/coreclr/inc/CrstTypes.def
Original file line number Diff line number Diff line change
Expand Up @@ -577,7 +577,3 @@ End
Crst PgoData
AcquiredBefore LoaderHeap
End

Crst StaticBoxInit
AcquiredBefore LoaderHeap FrozenObjectHeap
End
11 changes: 9 additions & 2 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 getFieldInfo. (see
* <NONE> This is a normal static field. Its address in memory is determined by getFieldAddress. (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;
CORINFO_CONST_LOOKUP fieldLookup; // Used by Ready-to-Run
};

//----------------------------------------------------------------------------
Expand Down Expand Up @@ -3200,6 +3200,13 @@ 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: 22 additions & 25 deletions src/coreclr/inc/crsttypes_generated.h
Original file line number Diff line number Diff line change
Expand Up @@ -112,29 +112,28 @@ enum CrstType
CrstSingleUseLock = 94,
CrstSpecialStatics = 95,
CrstStackSampler = 96,
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
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
};

#endif // __CRST_TYPES_INCLUDED
Expand Down Expand Up @@ -242,7 +241,6 @@ int g_rgCrstLevelMap[] =
5, // CrstSingleUseLock
0, // CrstSpecialStatics
0, // CrstStackSampler
-1, // CrstStaticBoxInit
-1, // CrstStressLog
5, // CrstStubCache
0, // CrstStubDispatchCache
Expand Down Expand Up @@ -366,7 +364,6 @@ LPCSTR g_rgCrstNameMap[] =
"CrstSingleUseLock",
"CrstSpecialStatics",
"CrstStackSampler",
"CrstStaticBoxInit",
"CrstStressLog",
"CrstStubCache",
"CrstStubDispatchCache",
Expand Down
4 changes: 4 additions & 0 deletions src/coreclr/inc/icorjitinfoimpl_generated.h
Original file line number Diff line number Diff line change
Expand Up @@ -616,6 +616,10 @@ 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 = { /* c0c94f6c-5358-4a3d-be83-b2a9d1b2545e */
0xc0c94f6c,
0x5358,
0x4a3d,
{0xbe, 0x83, 0xb2, 0xa9, 0xd1, 0xb2, 0x54, 0x5e}
constexpr GUID JITEEVersionIdentifier = { /* e3c98d96-6e67-459a-9750-ebe799cfb788 */
0xe3c98d96,
0x6e67,
0x459a,
{0x97, 0x50, 0xeb, 0xe7, 0x99, 0xcf, 0xb7, 0x88}
};

//////////////////////////////////////////////////////////////////////////////////////////////////////////
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/jit/ICorJitInfo_names_generated.h
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ 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: 10 additions & 0 deletions src/coreclr/jit/ICorJitInfo_wrapper_generated.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -1479,6 +1479,16 @@ 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: 2 additions & 1 deletion src/coreclr/jit/assertionprop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3197,7 +3197,8 @@ 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
1 change: 1 addition & 0 deletions src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -5733,6 +5733,7 @@ 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);
Expand Down
5 changes: 4 additions & 1 deletion src/coreclr/jit/emitarm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4012,7 +4012,10 @@ void emitter::emitIns_R_C(instruction ins, emitAttr attr, regNumber reg, CORINFO
}
else
{
assert(!"Normal statics are expected to be handled in the importer");
assert(!jitStaticFldIsGlobAddr(fldHnd));
addr = (ssize_t)emitComp->info.compCompHnd->getFieldAddress(fldHnd, NULL);
if (addr == NULL)
NO_WAY("could not obtain address of static field");
}

// We can use reg to load the constant address,
Expand Down
14 changes: 12 additions & 2 deletions src/coreclr/jit/emitxarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12977,8 +12977,18 @@ BYTE* emitter::emitOutputCV(BYTE* dst, instrDesc* id, code_t code, CnsVal* addc)
else
{
// Special case: mov reg, fs:[ddd] or mov reg, [ddd]
assert(jitStaticFldIsGlobAddr(fldh));
addr = nullptr;
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");
}
}
}

BYTE* target = (addr + offs);
Expand Down
3 changes: 1 addition & 2 deletions src/coreclr/jit/fginline.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1608,8 +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->OperIs(GT_IND) && op2->gtGetOp1()->IsIconHandle() &&
((op2->gtFlags & GTF_EXCEPT) == 0))
(op2->gtOper == GT_FIELD) && ((op2->gtFlags & GTF_EXCEPT) == 0))
{
JITDUMP("\nPerforming special dce on unused arg [%06u]:"
" actual arg [%06u] helper call [%06u]\n",
Expand Down
10 changes: 0 additions & 10 deletions src/coreclr/jit/gentree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18079,16 +18079,6 @@ CORINFO_CLASS_HANDLE Compiler::gtGetClassHandle(GenTree* tree, bool* pIsExact, b
}
}
}
else if (base->IsIconHandle(GTF_ICON_CONST_PTR, GTF_ICON_STATIC_HDL))
{
// Check if we have IND(ICON_HANDLE) that represents a static field
FieldSeq* fldSeq = base->AsIntCon()->gtFieldSeq;
if ((fldSeq != nullptr) && (fldSeq->GetOffset() == base->AsIntCon()->IconValue()))
{
CORINFO_FIELD_HANDLE fldHandle = base->AsIntCon()->gtFieldSeq->GetFieldHandle();
objClass = gtGetFieldClassHandle(fldHandle, pIsExact, pIsNonNull);
}
}
}

break;
Expand Down
6 changes: 0 additions & 6 deletions src/coreclr/jit/gentree.h
Original file line number Diff line number Diff line change
Expand Up @@ -2209,12 +2209,6 @@ 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
91 changes: 49 additions & 42 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->TypeIs(TYP_BYREF, TYP_I_IMPL));
assert(blockNodeAddr->gtType == TYP_BYREF);
GenTree* commaNode = parent;
commaNode->gtType = blockNodeAddr->gtType;
commaNode->gtType = TYP_BYREF;
commaNode->AsOp()->gtOp2 = blockNodeAddr;
blockNode->AsOp()->gtOp1 = commaNode;
if (parent == structVal)
Expand Down Expand Up @@ -4374,9 +4374,6 @@ 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 @@ -4386,11 +4383,14 @@ 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)
{
assert(pFieldInfo->fieldLookup.accessType == IAT_VALUE);
offset = reinterpret_cast<ssize_t>(pFieldInfo->fieldLookup.addr);
offset = reinterpret_cast<ssize_t>(info.compCompHnd->getFieldAddress(pResolvedToken->hField));
assert(offset != 0);
}
else
{
Expand All @@ -4401,7 +4401,6 @@ GenTree* Compiler::impImportStaticFieldAccess(CORINFO_RESOLVED_TOKEN* pResolvedT
outerFldSeq = nullptr;
}

bool isStaticReadOnlyInitedRef = false;
GenTree* op1;
switch (pFieldInfo->fieldAccessor)
{
Expand Down Expand Up @@ -4494,41 +4493,53 @@ 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) && ((access & CORINFO_ACCESS_ADDRESS) == 0))
// Do we need the address of a static field?
//
if (access & CORINFO_ACCESS_ADDRESS)
{
bool isSpeculative = true;
if ((info.compCompHnd->getStaticFieldCurrentClass(pResolvedToken->hField, &isSpeculative) !=
NO_CLASS_HANDLE))
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)
{
isStaticReadOnlyInitedRef = !isSpeculative;
op1->gtFlags |= GTF_ICON_INITCLASS;
}
}
#endif // TARGET_64BIT

assert(hasConstAddr);
assert(pFieldInfo->fieldLookup.addr != nullptr);
assert(pFieldInfo->fieldLookup.accessType == IAT_VALUE);
size_t fldAddr = reinterpret_cast<size_t>(pFieldInfo->fieldLookup.addr);
GenTreeFlags handleKind;
if (isBoxedStatic)
{
handleKind = GTF_ICON_STATIC_BOX_PTR;
}
else if (isStaticReadOnlyInitedRef)
{
handleKind = GTF_ICON_CONST_PTR;
}
else
{
handleKind = GTF_ICON_STATIC_HDL;
}
op1 = gtNewIconHandleNode(fldAddr, handleKind, innerFldSeq);
INDEBUG(op1->AsIntCon()->gtTargetHandle = reinterpret_cast<size_t>(pResolvedToken->hField));
if (pFieldInfo->fieldFlags & CORINFO_FLG_FIELD_INITCLASS)
else // We need the value of a static field
{
op1->gtFlags |= GTF_ICON_INITCLASS;
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;
}
break;
}
Expand All @@ -4552,10 +4563,6 @@ 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

0 comments on commit 0de4f60

Please sign in to comment.