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

Convert asserts in CEEInfo::getStaticFieldContent() to 'if' checks #100320

Merged
merged 5 commits into from
Mar 27, 2024
Merged
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
2 changes: 2 additions & 0 deletions src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -5591,6 +5591,8 @@ class Compiler
void fgValueNumberFieldStore(
GenTree* storeNode, GenTree* baseAddr, FieldSeq* fieldSeq, ssize_t offset, unsigned storeSize, ValueNum value);

static bool fgGetStaticFieldSeqAndAddress(ValueNumStore* vnStore, GenTree* tree, ssize_t* byteOffset, FieldSeq** pFseq);

bool fgValueNumberConstLoad(GenTreeIndir* tree);

// Compute the value number for a byref-exposed load of the given type via the given pointerVN.
Expand Down
6 changes: 3 additions & 3 deletions src/coreclr/jit/fginline.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -590,9 +590,9 @@ class SubstitutePlaceholdersAndDevirtualizeWalker : public GenTreeVisitor<Substi

if (lcl->lvSingleDef)
{
bool isExact = false;
bool isNonNull = false;
CORINFO_CLASS_HANDLE newClass = m_compiler->gtGetClassHandle(value, &isExact, &isNonNull);
bool isExact;
bool isNonNull;
CORINFO_CLASS_HANDLE newClass = m_compiler->gtGetClassHandle(value, &isExact, &isNonNull);

if (newClass != NO_CLASS_HANDLE)
{
Expand Down
66 changes: 35 additions & 31 deletions src/coreclr/jit/gentree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14124,13 +14124,13 @@ GenTree* Compiler::gtFoldTypeCompare(GenTree* tree)
objOp = opOther->AsCall()->gtArgs.GetThisArg()->GetNode();
}

bool pIsExact = false;
bool pIsNonNull = false;
CORINFO_CLASS_HANDLE objCls = gtGetClassHandle(objOp, &pIsExact, &pIsNonNull);
bool isExact = false;
bool isNonNull = false;
CORINFO_CLASS_HANDLE objCls = gtGetClassHandle(objOp, &isExact, &isNonNull);

// if both classes are "final" (e.g. System.String[]) we can replace the comparison
// with `true/false` + null check.
if ((objCls != NO_CLASS_HANDLE) && (pIsExact || info.compCompHnd->isExactType(objCls)))
if ((objCls != NO_CLASS_HANDLE) && (isExact || info.compCompHnd->isExactType(objCls)))
{
TypeCompareState tcs = info.compCompHnd->compareTypesForEquality(objCls, clsHnd);
if (tcs != TypeCompareState::May)
Expand All @@ -14139,7 +14139,7 @@ GenTree* Compiler::gtFoldTypeCompare(GenTree* tree)
const bool typesAreEqual = tcs == TypeCompareState::Must;
GenTree* compareResult = gtNewIconNode((operatorIsEQ ^ typesAreEqual) ? 0 : 1);

if (!pIsNonNull)
if (!isNonNull)
{
// we still have to emit a null-check
// obj.GetType == typeof() -> (nullcheck) true/false
Expand Down Expand Up @@ -18398,24 +18398,23 @@ bool Compiler::gtStoreDefinesField(
// otherwise actual type may be a subtype.
// *pIsNonNull set true if tree value is known not to be null,
// otherwise a null value is possible.

//
CORINFO_CLASS_HANDLE Compiler::gtGetClassHandle(GenTree* tree, bool* pIsExact, bool* pIsNonNull)
{
// Set default values for our out params.
*pIsNonNull = false;
*pIsExact = false;
CORINFO_CLASS_HANDLE objClass = nullptr;
*pIsNonNull = false;
*pIsExact = false;

// Bail out if the tree is not a ref type.
var_types treeType = tree->TypeGet();
if (treeType != TYP_REF)
if (!tree->TypeIs(TYP_REF))
{
return objClass;
return NO_CLASS_HANDLE;
}

// Tunnel through commas.
GenTree* obj = tree->gtEffectiveVal();
const genTreeOps objOp = obj->OperGet();
GenTree* obj = tree->gtEffectiveVal();
const genTreeOps objOp = obj->OperGet();
CORINFO_CLASS_HANDLE objClass = NO_CLASS_HANDLE;

switch (objOp)
{
Expand Down Expand Up @@ -18564,7 +18563,6 @@ CORINFO_CLASS_HANDLE Compiler::gtGetClassHandle(GenTree* tree, bool* pIsExact, b
assert(runtimeType != NO_CLASS_HANDLE);

objClass = runtimeType;
*pIsExact = false;
*pIsNonNull = true;
}

Expand Down Expand Up @@ -18608,9 +18606,6 @@ CORINFO_CLASS_HANDLE Compiler::gtGetClassHandle(GenTree* tree, bool* pIsExact, b
{
objClass = gtGetArrayElementClassHandle(base->AsArrElem()->gtArrObj);
}

*pIsExact = false;
*pIsNonNull = false;
}
else if (base->OperGet() == GT_ADD)
{
Expand Down Expand Up @@ -18647,7 +18642,7 @@ CORINFO_CLASS_HANDLE Compiler::gtGetClassHandle(GenTree* tree, bool* pIsExact, b
FieldSeq* fldSeq = base->AsIntCon()->gtFieldSeq;
if ((fldSeq != nullptr) && (fldSeq->GetOffset() == base->AsIntCon()->IconValue()))
{
CORINFO_FIELD_HANDLE fldHandle = base->AsIntCon()->gtFieldSeq->GetFieldHandle();
CORINFO_FIELD_HANDLE fldHandle = fldSeq->GetFieldHandle();
objClass = gtGetFieldClassHandle(fldHandle, pIsExact, pIsNonNull);
}
}
Expand Down Expand Up @@ -18714,7 +18709,7 @@ CORINFO_CLASS_HANDLE Compiler::gtGetClassHandle(GenTree* tree, bool* pIsExact, b
// Return Value:
// nullptr if helper call result is not a ref class, or the class handle
// is unknown, otherwise the class handle.

//
CORINFO_CLASS_HANDLE Compiler::gtGetHelperCallClassHandle(GenTreeCall* call, bool* pIsExact, bool* pIsNonNull)
{
assert(call->gtCallType == CT_HELPER);
Expand Down Expand Up @@ -18856,7 +18851,7 @@ CORINFO_CLASS_HANDLE Compiler::gtGetHelperCallClassHandle(GenTreeCall* call, boo
//
// Return Value:
// nullptr if element class handle is unknown, otherwise the class handle.

//
CORINFO_CLASS_HANDLE Compiler::gtGetArrayElementClassHandle(GenTree* array)
{
bool isArrayExact = false;
Expand Down Expand Up @@ -18898,9 +18893,12 @@ CORINFO_CLASS_HANDLE Compiler::gtGetArrayElementClassHandle(GenTree* array)
// is unknown, otherwise the class handle.
//
// May examine runtime state of static field instances.

//
CORINFO_CLASS_HANDLE Compiler::gtGetFieldClassHandle(CORINFO_FIELD_HANDLE fieldHnd, bool* pIsExact, bool* pIsNonNull)
{
*pIsExact = false;
*pIsNonNull = false;

CORINFO_CLASS_HANDLE fieldClass = NO_CLASS_HANDLE;
CorInfoType fieldCorType = info.compCompHnd->getFieldType(fieldHnd, &fieldClass);

Expand All @@ -18913,11 +18911,14 @@ CORINFO_CLASS_HANDLE Compiler::gtGetFieldClassHandle(CORINFO_FIELD_HANDLE fieldH
if (queryForCurrentClass)
{
#if DEBUG
char fieldNameBuffer[128];
char classNameBuffer[128];
JITDUMP("\nQuerying runtime about current class of field %s (declared as %s)\n",
eeGetFieldName(fieldHnd, true, fieldNameBuffer, sizeof(fieldNameBuffer)),
eeGetClassName(fieldClass, classNameBuffer, sizeof(classNameBuffer)));
if (verbose || JitConfig.EnableExtraSuperPmiQueries())
{
char fieldNameBuffer[128];
char classNameBuffer[128];
const char* fieldName = eeGetFieldName(fieldHnd, true, fieldNameBuffer, sizeof(fieldNameBuffer));
const char* className = eeGetClassName(fieldClass, classNameBuffer, sizeof(classNameBuffer));
JITDUMP("\nQuerying runtime about current class of field %s (declared as %s)\n", fieldName, className);
}
#endif // DEBUG

// Is this a fully initialized init-only static field?
Expand All @@ -18932,10 +18933,13 @@ CORINFO_CLASS_HANDLE Compiler::gtGetFieldClassHandle(CORINFO_FIELD_HANDLE fieldH
*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
if (verbose || JitConfig.EnableExtraSuperPmiQueries())
{
char classNameBuffer2[128];
const char* className2 = eeGetClassName(fieldClass, classNameBuffer2, sizeof(classNameBuffer2));
JITDUMP("Runtime reports field is init-only and initialized and has class %s\n", className2);
}
#endif // DEBUG
}
else
{
Expand Down
11 changes: 8 additions & 3 deletions src/coreclr/jit/valuenum.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10939,7 +10939,11 @@ void Compiler::fgValueNumberSsaVarDef(GenTreeLclVarCommon* lcl)
// Return Value:
// true if the given tree is a static field address
//
static bool GetStaticFieldSeqAndAddress(ValueNumStore* vnStore, GenTree* tree, ssize_t* byteOffset, FieldSeq** pFseq)
/* static */
bool Compiler::fgGetStaticFieldSeqAndAddress(ValueNumStore* vnStore,
GenTree* tree,
ssize_t* byteOffset,
FieldSeq** pFseq)
{
VNFuncApp funcApp;
if (vnStore->GetVNFunc(tree->gtVNPair.GetLiberal(), &funcApp) && (funcApp.m_func == VNF_PtrToStatic))
Expand All @@ -10954,7 +10958,6 @@ static bool GetStaticFieldSeqAndAddress(ValueNumStore* vnStore, GenTree* tree, s
return true;
}
}
ssize_t val = 0;

// Special cases for NativeAOT:
// ADD(ICON_STATIC, CNS_INT) // nonGC-static base
Expand All @@ -10972,6 +10975,7 @@ static bool GetStaticFieldSeqAndAddress(ValueNumStore* vnStore, GenTree* tree, s
}

// Accumulate final offset
ssize_t val = 0;
while (tree->OperIs(GT_ADD))
{
GenTree* op1 = tree->gtGetOp1();
Expand Down Expand Up @@ -11011,6 +11015,7 @@ static bool GetStaticFieldSeqAndAddress(ValueNumStore* vnStore, GenTree* tree, s
return true;
}
}

return false;
}

Expand Down Expand Up @@ -11094,7 +11099,7 @@ bool Compiler::fgValueNumberConstLoad(GenTreeIndir* tree)
const int maxElementSize = sizeof(simd_t);

if (!tree->TypeIs(TYP_BYREF, TYP_STRUCT) &&
GetStaticFieldSeqAndAddress(vnStore, tree->gtGetOp1(), &byteOffset, &fieldSeq))
fgGetStaticFieldSeqAndAddress(vnStore, tree->gtGetOp1(), &byteOffset, &fieldSeq))
{
CORINFO_FIELD_HANDLE fieldHandle = fieldSeq->GetFieldHandle();
if ((fieldHandle != nullptr) && (size > 0) && (size <= maxElementSize) && ((size_t)byteOffset < INT_MAX))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2336,12 +2336,16 @@ private bool getStaticFieldContent(CORINFO_FIELD_STRUCT_* fieldHandle, byte* buf

if (value == null)
{
Debug.Assert(valueOffset == 0);
Debug.Assert(bufferSize == targetPtrSize);

// Write "null" to buffer
new Span<byte>(buffer, targetPtrSize).Clear();
return true;
if ((valueOffset == 0) && (bufferSize == targetPtrSize))
{
// Write "null" to buffer
new Span<byte>(buffer, targetPtrSize).Clear();
return true;
}
else
{
return false;
}
}

if (value.GetRawData(_compilation.NodeFactory, out object data))
Expand All @@ -2357,13 +2361,14 @@ private bool getStaticFieldContent(CORINFO_FIELD_STRUCT_* fieldHandle, byte* buf
return false;

case FrozenObjectNode:
Debug.Assert(valueOffset == 0);
Debug.Assert(bufferSize == targetPtrSize);

// save handle's value to buffer
nint handle = ObjectToHandle(data);
new Span<byte>(&handle, targetPtrSize).CopyTo(new Span<byte>(buffer, targetPtrSize));
return true;
if ((valueOffset == 0) && (bufferSize == targetPtrSize))
{
// save handle's value to buffer
nint handle = ObjectToHandle(data);
new Span<byte>(&handle, targetPtrSize).CopyTo(new Span<byte>(buffer, targetPtrSize));
return true;
}
return false;
}
}
}
Expand Down
12 changes: 6 additions & 6 deletions src/coreclr/vm/jitinterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11777,13 +11777,13 @@ bool CEEInfo::getStaticFieldContent(CORINFO_FIELD_HANDLE fieldHnd, uint8_t* buff
{
if (field->IsObjRef())
{
GCX_COOP();

_ASSERT(!field->IsRVA());
_ASSERT(valueOffset == 0); // there is no point in returning a chunk of a gc handle
_ASSERT((UINT)bufferSize == field->GetSize());
// there is no point in returning a chunk of a gc handle
BruceForstall marked this conversation as resolved.
Show resolved Hide resolved
if ((valueOffset == 0) && (sizeof(CORINFO_OBJECT_HANDLE) <= (UINT)bufferSize) && !field->IsRVA())
{
GCX_COOP();

result = getStaticObjRefContent(field->GetStaticOBJECTREF(), buffer, ignoreMovableObjects);
result = getStaticObjRefContent(field->GetStaticOBJECTREF(), buffer, ignoreMovableObjects);
}
}
else
{
Expand Down
Loading