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

Implement getStaticFieldCurrentClass for NAOT #96982

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 3 additions & 12 deletions src/coreclr/inc/corinfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -3268,20 +3268,11 @@ class ICorDynamicInfo : public ICorStaticInfo
int valueOffset
) = 0;

// If pIsSpeculative is NULL, return the class handle for the value of ref-class typed
// static readonly fields, if there is a unique location for the static and the class
// is already initialized.
//
// If pIsSpeculative is not NULL, fetch the class handle for the value of all ref-class
// typed static fields, if there is a unique location for the static and the field is
// not null.
//
// Set *pIsSpeculative true if this type may change over time (field is not readonly or
// is readonly but class has not yet finished initialization). Set *pIsSpeculative false
// if this type will not change.
// Return field's exact type if its value is known to be never changed and
// is not null (for reference types). Returns nullptr otherwise.
virtual CORINFO_CLASS_HANDLE getStaticFieldCurrentClass(
CORINFO_FIELD_HANDLE field,
bool *pIsSpeculative = NULL
bool *pIsSpeculative = NULL // unused, to be deleted
) = 0;

// registers a vararg sig & returns a VM cookie for it (which can contain other stuff)
Expand Down
56 changes: 17 additions & 39 deletions src/coreclr/jit/gentree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18789,49 +18789,27 @@ CORINFO_CLASS_HANDLE Compiler::gtGetArrayElementClassHandle(GenTree* array)

CORINFO_CLASS_HANDLE Compiler::gtGetFieldClassHandle(CORINFO_FIELD_HANDLE fieldHnd, bool* pIsExact, bool* pIsNonNull)
{
CORINFO_CLASS_HANDLE fieldClass = nullptr;
CorInfoType fieldCorType = info.compCompHnd->getFieldType(fieldHnd, &fieldClass);

if (fieldCorType == CORINFO_TYPE_CLASS)
CORINFO_CLASS_HANDLE fieldClass = nullptr;
if (info.compCompHnd->getFieldType(fieldHnd, &fieldClass) == CORINFO_TYPE_CLASS)
{
// Optionally, look at the actual type of the field's value
bool queryForCurrentClass = true;
INDEBUG(queryForCurrentClass = (JitConfig.JitQueryCurrentStaticFieldClass() > 0););
JITDUMP("Querying runtime about current class of field %s (declared as %s)\n", eeGetFieldName(fieldHnd, true),
eeGetClassName(fieldClass));

if (queryForCurrentClass)
// Is this a fully initialized init-only static field?
CORINFO_CLASS_HANDLE currentClass = info.compCompHnd->getStaticFieldCurrentClass(fieldHnd);
if (currentClass != NO_CLASS_HANDLE)
{
#if DEBUG
char fieldNameBuffer[128];
char classNameBuffer[128];
JITDUMP("Querying runtime about current class of field %s (declared as %s)\n",
eeGetFieldName(fieldHnd, true, fieldNameBuffer, sizeof(fieldNameBuffer)),
eeGetClassName(fieldClass, classNameBuffer, sizeof(classNameBuffer)));
#endif // DEBUG

// Is this a fully initialized init-only static field?
//
// Note we're not asking for speculative results here, yet.
CORINFO_CLASS_HANDLE currentClass = info.compCompHnd->getStaticFieldCurrentClass(fieldHnd);

if (currentClass != NO_CLASS_HANDLE)
{
// Yes! We know the class exactly and can rely on this to always be true.
fieldClass = currentClass;
*pIsExact = true;
*pIsNonNull = true;
#ifdef DEBUG
char buffer[128];
JITDUMP("Runtime reports field is init-only and initialized and has class %s\n",
eeGetClassName(fieldClass, buffer, sizeof(buffer)));
#endif
}
else
{
JITDUMP("Field's current class not available\n");
}

return fieldClass;
fieldClass = currentClass;
*pIsExact = true;
*pIsNonNull = true;
JITDUMP("Runtime reports field is init-only and initialized and has class %s\n",
eeGetClassName(fieldClass));
}
else
{
JITDUMP("Field's current class is not available\n");
}
return fieldClass;
}

return NO_CLASS_HANDLE;
Expand Down
30 changes: 14 additions & 16 deletions src/coreclr/jit/importer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3935,19 +3935,17 @@ GenTree* Compiler::impImportStaticFieldAddress(CORINFO_RESOLVED_TOKEN* pResolved

default:
{
bool isStaticReadOnlyInitedRef = false;
bool isStaticReadOnlyInited = false;
bool isStaticReadNotNull = false;

#ifdef TARGET_64BIT
// TODO-CQ: enable this optimization for 32 bit targets.
if (!isBoxedStatic && (lclTyp == TYP_REF) && ((access & CORINFO_ACCESS_GET) != 0) &&
((*pIndirFlags & GTF_IND_VOLATILE) == 0))
if (!isBoxedStatic && ((access & CORINFO_ACCESS_GET) != 0) && ((*pIndirFlags & GTF_IND_VOLATILE) == 0) &&
(info.compCompHnd->getStaticFieldCurrentClass(pResolvedToken->hField) != NO_CLASS_HANDLE))
{
bool isSpeculative = true;
if ((info.compCompHnd->getStaticFieldCurrentClass(pResolvedToken->hField, &isSpeculative) !=
NO_CLASS_HANDLE))
{
isStaticReadOnlyInitedRef = !isSpeculative;
}
isStaticReadOnlyInited = true;
isStaticReadNotNull = lclTyp == TYP_REF;
// if it's not TYP_REF then its value could be 0 so don't append GTF_IND_NONNULL flag.
}
#endif // TARGET_64BIT

Expand All @@ -3958,7 +3956,7 @@ GenTree* Compiler::impImportStaticFieldAddress(CORINFO_RESOLVED_TOKEN* pResolved
{
handleKind = GTF_ICON_STATIC_BOX_PTR;
}
else if (isStaticReadOnlyInitedRef)
else if (isStaticReadOnlyInited)
{
handleKind = GTF_ICON_CONST_PTR;
}
Expand All @@ -3970,13 +3968,13 @@ GenTree* Compiler::impImportStaticFieldAddress(CORINFO_RESOLVED_TOKEN* pResolved
op1 = gtNewIconHandleNode(fldAddr, handleKind, innerFldSeq);
INDEBUG(op1->AsIntCon()->gtTargetHandle = reinterpret_cast<size_t>(pResolvedToken->hField));

if (pFieldInfo->fieldFlags & CORINFO_FLG_FIELD_INITCLASS)
{
indirFlags |= GTF_IND_INITCLASS;
}
if (isStaticReadOnlyInitedRef)
if (isStaticReadOnlyInited)
{
indirFlags |= (GTF_IND_INVARIANT | GTF_IND_NONNULL);
indirFlags |= GTF_IND_INVARIANT;
if (isStaticReadNotNull)
{
indirFlags |= GTF_IND_NONNULL;
}
}
break;
}
Expand Down
1 change: 0 additions & 1 deletion src/coreclr/jit/jitconfigvalues.h
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,6 @@ CONFIG_INTEGER(JitNoStructPromotion, W("JitNoStructPromotion"), 0) // Disables s
// params.
CONFIG_INTEGER(JitNoUnroll, W("JitNoUnroll"), 0)
CONFIG_INTEGER(JitOrder, W("JitOrder"), 0)
CONFIG_INTEGER(JitQueryCurrentStaticFieldClass, W("JitQueryCurrentStaticFieldClass"), 1)
CONFIG_INTEGER(JitReportFastTailCallDecisions, W("JitReportFastTailCallDecisions"), 0)
CONFIG_INTEGER(JitPInvokeCheckEnabled, W("JITPInvokeCheckEnabled"), 0)
CONFIG_INTEGER(JitPInvokeEnabled, W("JITPInvokeEnabled"), 1)
Expand Down
9 changes: 0 additions & 9 deletions src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3526,15 +3526,6 @@ private CORINFO_CONST_LOOKUP CreateConstLookupToSymbol(ISymbolNode symbol)

private uint getClassDomainID(CORINFO_CLASS_STRUCT_* cls, ref void* ppIndirection)
{ throw new NotImplementedException("getClassDomainID"); }

private CORINFO_CLASS_STRUCT_* getStaticFieldCurrentClass(CORINFO_FIELD_STRUCT_* field, byte* pIsSpeculative)
{
if (pIsSpeculative != null)
*pIsSpeculative = 1;

return null;
}

private IntPtr getVarArgsHandle(CORINFO_SIG_INFO* pSig, ref void* ppIndirection)
{ throw new NotImplementedException("getVarArgsHandle"); }
private bool canGetVarArgsHandle(CORINFO_SIG_INFO* pSig)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3366,5 +3366,11 @@ private bool notifyMethodInfoUsage(CORINFO_METHOD_STRUCT_* ftn)
// stable e.g. mark calls as no-return if their IL has no rets.
return _compilation.NodeFactory.CompilationModuleGroup.VersionsWithMethodBody(method);
}

private CORINFO_CLASS_STRUCT_* getStaticFieldCurrentClass(CORINFO_FIELD_STRUCT_* field, byte* pIsSpeculative)
{
Debug.Assert(pIsSpeculative == null); // to be deleted
return null;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2395,5 +2395,36 @@ private bool notifyMethodInfoUsage(CORINFO_METHOD_STRUCT_* ftn)
{
return true;
}

private CORINFO_CLASS_STRUCT_* getStaticFieldCurrentClass(CORINFO_FIELD_STRUCT_* field, byte* pIsSpeculative)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit confused about the expected semantics of this API.

// Return field's exact type if its value is known to be never changed and
// is not null (for reference types). Returns nullptr otherwise.

Like Jan said - we don't check the initonly part - neither for the RVA nor for non-RVA statics. Should this always return null for !initonly?
We also return the type of the field, not the type of the value stored in the field (in the reference type case).

{
Debug.Assert(pIsSpeculative == null); // To be deleted

FieldDesc fieldDesc = HandleToObject(field);

if (fieldDesc.IsStatic && !fieldDesc.IsThreadStatic && fieldDesc.OwningType is MetadataType owningType &&
!owningType.IsCanonicalSubtype(CanonicalFormKind.Any))
{
// Generally, this API is only used for reference types to provide better information
// to the JIT for possible devirtualizations. Value types are typically handled
// separately and folded to constants via getStaticFieldContent. However, some
// fields aren't foldable (e.g. ExternSymbolMappedField), so let's tell the JIT
// it can rely on them being invariant too.
if (fieldDesc.HasRva)
{
// Read-only RVA fields need no "is class initialized" check.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do you know that the RVA field is read-only?

Copy link
Member Author

@EgorBo EgorBo Jan 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do you know that the RVA field is read-only?

Can it be not so? I thought RVA can be mutable only for C++/CLI. Here we check that the field is final.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have several of these compiler generated RVA fields. Some of them are read-only, some of them are mutable.

Here we check that the field is final.

Where do we check that?

Debug.Assert(fieldDesc.FieldType.IsValueType);
return ObjectToHandle(fieldDesc.FieldType);
}

PreinitializationManager preinitManager = _compilation.NodeFactory.PreinitializationManager;
if (!fieldDesc.FieldType.IsValueType && preinitManager.IsPreinitialized(owningType) &&
preinitManager.GetPreinitializationInfo(owningType).GetFieldValue(fieldDesc) != null)
{
return ObjectToHandle(fieldDesc.FieldType);
}
}
return null;
}
}
}
39 changes: 3 additions & 36 deletions src/coreclr/vm/jitinterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11806,22 +11806,17 @@ CORINFO_CLASS_HANDLE CEEJitInfo::getStaticFieldCurrentClass(CORINFO_FIELD_HANDLE

CORINFO_CLASS_HANDLE result = NULL;

if (pIsSpeculative != NULL)
{
*pIsSpeculative = true;
}
_ASSERT(pIsSpeculative == nullptr); // to be deleted

JIT_TO_EE_TRANSITION();

FieldDesc* field = (FieldDesc*) fieldHnd;
bool isClassInitialized = false;

// We're only interested in ref class typed static fields
// where the field handle specifies a unique location.
if (field->IsStatic() && field->IsObjRef() && !field->IsThreadStatic())
{
MethodTable* pEnclosingMT = field->GetEnclosingMethodTable();

if (!pEnclosingMT->IsSharedByGenericInstantiations())
{
// Allocate space for the local class if necessary, but don't trigger
Expand All @@ -11834,37 +11829,9 @@ CORINFO_CLASS_HANDLE CEEJitInfo::getStaticFieldCurrentClass(CORINFO_FIELD_HANDLE
OBJECTREF fieldObj = field->GetStaticOBJECTREF();
VALIDATEOBJECTREF(fieldObj);

// Check for initialization before looking at the value
isClassInitialized = !!pEnclosingMT->IsClassInited();

if (fieldObj != NULL)
{
MethodTable *pObjMT = fieldObj->GetMethodTable();

// TODO: Check if the jit is allowed to embed this handle in jitted code.
// Note for the initonly cases it probably won't embed.
result = (CORINFO_CLASS_HANDLE) pObjMT;
}
}
}

// Did we find a class?
if (result != NULL)
{
// Figure out what to report back.
bool isResultImmutable = isClassInitialized && IsFdInitOnly(field->GetAttributes());

if (pIsSpeculative != NULL)
{
// Caller is ok with potentially mutable results.
*pIsSpeculative = !isResultImmutable;
}
else
{
// Caller only wants to see immutable results.
if (!isResultImmutable)
if (fieldObj != NULL && pEnclosingMT->IsClassInited() && IsFdInitOnly(field->GetAttributes()))
{
result = NULL;
result = (CORINFO_CLASS_HANDLE)fieldObj->GetMethodTable();
}
}
}
Expand Down
Loading