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

Fold bound checks for static readonly arrays/strings #77593

Merged
merged 44 commits into from
Nov 1, 2022
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
0fe7042
Initial impl of Jan's suggestions
EgorBo Oct 29, 2022
e428363
Address part of the feedback
EgorBo Oct 29, 2022
774af69
Address feedback
EgorBo Oct 29, 2022
28b9be3
Remove "is frozen" check from jit
EgorBo Oct 29, 2022
9f92bfb
NativeAOT support
EgorBo Oct 29, 2022
687d5cb
Use CORINFO_OBJECT_HANDLE in more places
EgorBo Oct 29, 2022
872058d
Free jit handles upon exit
EgorBo Oct 29, 2022
860d706
Update src/coreclr/vm/jitinterface.cpp
EgorBo Oct 29, 2022
bffb8e6
Clean up
EgorBo Oct 29, 2022
f0a35cc
clean up in getArrayLength
EgorBo Oct 29, 2022
bfaf6ee
Update jitinterface.h
EgorBo Oct 29, 2022
a3ccc91
Remove unnecessary DestroyHandle
EgorBo Oct 29, 2022
7e041c1
Rename to getArrayOrStringLength, free jit handles in destructor
EgorBo Oct 29, 2022
1bba1ec
fix compilation
EgorBo Oct 29, 2022
ee0117e
Use IND instead of LoadVector
EgorBo Oct 30, 2022
b20f1fb
Update hwintrinsicxarch.cpp
EgorBo Oct 30, 2022
2a12b10
Update hwintrinsicxarch.cpp
EgorBo Oct 30, 2022
504acf3
Apply suggestions from code review
EgorBo Oct 30, 2022
b56a67f
Address feedback
EgorBo Oct 30, 2022
15a39a6
Address feedback
EgorBo Oct 30, 2022
1683f29
Address feedback
EgorBo Oct 30, 2022
43a32c9
Move "Is frozen" check to getJitHandleForObject
EgorBo Oct 30, 2022
d10f4eb
Update src/coreclr/vm/jitinterface.cpp
EgorBo Oct 30, 2022
4443383
Update src/coreclr/vm/jitinterface.cpp
EgorBo Oct 30, 2022
429b4d1
use OBJECTREF everywhere
EgorBo Oct 30, 2022
49c32c4
fix bad merge
EgorBo Oct 30, 2022
5dbbef4
Cannonize more loads
EgorBo Oct 30, 2022
62f5cbb
check ISAs
EgorBo Oct 30, 2022
a096211
Fix comments
EgorBo Oct 30, 2022
efd71e9
Handle stores
EgorBo Oct 31, 2022
27a2619
Fix compilation & address feedback around redundant IsFrozenSegment call
EgorBo Oct 31, 2022
1f95212
Merge branch 'use-ind-for-loads' of github.com:EgorBo/runtime-1 into …
EgorBo Oct 31, 2022
d5c3cab
DescriptionDescription -> Description
EgorBo Oct 31, 2022
7621045
Merge branch 'main' of github.com:dotnet/runtime into fold-static-rea…
EgorBo Oct 31, 2022
96397b1
Update valuenum.cpp
EgorBo Oct 31, 2022
076d4cd
Add comments
EgorBo Oct 31, 2022
1ecceca
Merge branch 'main' of github.com:dotnet/runtime into fold-static-rea…
EgorBo Oct 31, 2022
752c840
Revert unrelated SIMD changes (accidentally pushed)
EgorBo Oct 31, 2022
ea8e369
Use Exception sets
EgorBo Oct 31, 2022
2fab46b
Address feedback
EgorBo Oct 31, 2022
07d42e5
Address feedback
EgorBo Oct 31, 2022
8c2ec20
Update src/coreclr/jit/valuenum.cpp
EgorBo Oct 31, 2022
93c11c9
Address feedback
EgorBo Oct 31, 2022
19df3e6
Apply suggestions from code review
EgorBo Oct 31, 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
26 changes: 14 additions & 12 deletions src/coreclr/inc/corinfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -1056,6 +1056,7 @@ typedef struct CORINFO_DEPENDENCY_STRUCT_* CORINFO_DEPENDENCY_HANDLE;
typedef struct CORINFO_CLASS_STRUCT_* CORINFO_CLASS_HANDLE;
typedef struct CORINFO_METHOD_STRUCT_* CORINFO_METHOD_HANDLE;
typedef struct CORINFO_FIELD_STRUCT_* CORINFO_FIELD_HANDLE;
typedef struct CORINFO_OBJECT_STRUCT_* CORINFO_OBJECT_HANDLE;
EgorBo marked this conversation as resolved.
Show resolved Hide resolved
typedef struct CORINFO_ARG_LIST_STRUCT_* CORINFO_ARG_LIST_HANDLE; // represents a list of argument types
typedef struct CORINFO_JUST_MY_CODE_HANDLE_*CORINFO_JUST_MY_CODE_HANDLE;
typedef struct CORINFO_PROFILING_STRUCT_* CORINFO_PROFILING_HANDLE; // a handle guaranteed to be unique per process
Expand Down Expand Up @@ -2284,7 +2285,7 @@ class ICorStaticInfo
// Bytes written to the given buffer, the range is [0..bufferSize)
//
virtual size_t printObjectDescription (
void* handle, /* IN */
CORINFO_OBJECT_HANDLE handle, /* IN */
char* buffer, /* OUT */
size_t bufferSize, /* IN */
size_t* pRequiredBufferSize = nullptr /* OUT */
Expand Down Expand Up @@ -2500,7 +2501,7 @@ class ICorStaticInfo
CORINFO_CLASS_HANDLE cls
) = 0;

virtual void* getRuntimeTypePointer(
virtual CORINFO_OBJECT_HANDLE getRuntimeTypePointer(
CORINFO_CLASS_HANDLE cls
) = 0;

Expand All @@ -2514,7 +2515,7 @@ class ICorStaticInfo
// Returns true if object is known to be immutable
//
virtual bool isObjectImmutable(
void* objPtr
CORINFO_OBJECT_HANDLE objPtr
) = 0;

//------------------------------------------------------------------------------
Expand All @@ -2527,7 +2528,7 @@ class ICorStaticInfo
// Returns CORINFO_CLASS_HANDLE handle that represents given object's type
//
virtual CORINFO_CLASS_HANDLE getObjectType(
void* objPtr
CORINFO_OBJECT_HANDLE objPtr
) = 0;

virtual bool getReadyToRunHelper(
Expand Down Expand Up @@ -2729,6 +2730,8 @@ class ICorStaticInfo
// Returns true iff "fldHnd" represents a static field.
virtual bool isFieldStatic(CORINFO_FIELD_HANDLE fldHnd) = 0;

virtual int getArrayLength(CORINFO_OBJECT_HANDLE objHnd) = 0;
EgorBo marked this conversation as resolved.
Show resolved Hide resolved

/*********************************************************************************/
//
// ICorDebugInfo
Expand Down Expand Up @@ -3196,23 +3199,22 @@ class ICorDynamicInfo : public ICorStaticInfo

//------------------------------------------------------------------------------
// getReadonlyStaticFieldValue: returns true and the actual field's value if the given
// field represents a statically initialized readonly field of any type, it might be:
// * integer/floating point primitive
// * null
// * frozen object reference (string, array or object)
// field represents a statically initialized readonly field of any type.
//
// Arguments:
// field - field handle
// buffer - buffer field's value will be stored to
// bufferSize - size of buffer
// field - field handle
// buffer - buffer field's value will be stored to
// bufferSize - size of buffer
// ignoreMovableObjects - ignore movable reference types or not
//
// Return Value:
// Returns true if field's constant value was available and successfully copied to buffer
//
virtual bool getReadonlyStaticFieldValue(
CORINFO_FIELD_HANDLE field,
uint8_t *buffer,
int bufferSize
int bufferSize,
bool ignoreMovableObjects = true
) = 0;

// If pIsSpeculative is NULL, return the class handle for the value of ref-class typed
Expand Down
14 changes: 9 additions & 5 deletions src/coreclr/inc/icorjitinfoimpl_generated.h
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ int getStringLiteral(
int startIndex) override;

size_t printObjectDescription(
void* handle,
CORINFO_OBJECT_HANDLE handle,
char* buffer,
size_t bufferSize,
size_t* pRequiredBufferSize) override;
Expand Down Expand Up @@ -287,14 +287,14 @@ CorInfoHelpFunc getBoxHelper(
CorInfoHelpFunc getUnBoxHelper(
CORINFO_CLASS_HANDLE cls) override;

void* getRuntimeTypePointer(
CORINFO_OBJECT_HANDLE getRuntimeTypePointer(
CORINFO_CLASS_HANDLE cls) override;

bool isObjectImmutable(
void* objPtr) override;
CORINFO_OBJECT_HANDLE objPtr) override;

CORINFO_CLASS_HANDLE getObjectType(
void* objPtr) override;
CORINFO_OBJECT_HANDLE objPtr) override;

bool getReadyToRunHelper(
CORINFO_RESOLVED_TOKEN* pResolvedToken,
Expand Down Expand Up @@ -404,6 +404,9 @@ void getFieldInfo(
bool isFieldStatic(
CORINFO_FIELD_HANDLE fldHnd) override;

int getArrayLength(
CORINFO_OBJECT_HANDLE objHnd) override;

void getBoundaries(
CORINFO_METHOD_HANDLE ftn,
unsigned int* cILOffsets,
Expand Down Expand Up @@ -616,7 +619,8 @@ void* getFieldAddress(
bool getReadonlyStaticFieldValue(
CORINFO_FIELD_HANDLE field,
uint8_t* buffer,
int bufferSize) override;
int bufferSize,
bool ignoreMovableObjects) override;

CORINFO_CLASS_HANDLE getStaticFieldCurrentClass(
CORINFO_FIELD_HANDLE field,
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 = { /* 2ed4cd12-48ed-4a0e-892e-d24d004a5e4c */
0x2ed4cd12,
0x48ed,
0x4a0e,
{0x89, 0x2e, 0xd2, 0x4d, 0x00, 0x4a, 0x5e, 0x4c}
constexpr GUID JITEEVersionIdentifier = { /* 77b6df16-d27f-4118-9dfd-d8073ff20fb6 */
0x77b6df16,
0xd27f,
0x4118,
{0x9d, 0xfd, 0xd8, 0x7, 0x3f, 0xf2, 0xf, 0xb6}
};

//////////////////////////////////////////////////////////////////////////////////////////////////////////
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 @@ -102,6 +102,7 @@ DEF_CLR_API(getFieldType)
DEF_CLR_API(getFieldOffset)
DEF_CLR_API(getFieldInfo)
DEF_CLR_API(isFieldStatic)
DEF_CLR_API(getArrayLength)
DEF_CLR_API(getBoundaries)
DEF_CLR_API(setBoundaries)
DEF_CLR_API(getVars)
Expand Down
24 changes: 17 additions & 7 deletions src/coreclr/jit/ICorJitInfo_wrapper_generated.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,7 @@ int WrapICorJitInfo::getStringLiteral(
}

size_t WrapICorJitInfo::printObjectDescription(
void* handle,
CORINFO_OBJECT_HANDLE handle,
char* buffer,
size_t bufferSize,
size_t* pRequiredBufferSize)
Expand Down Expand Up @@ -665,17 +665,17 @@ CorInfoHelpFunc WrapICorJitInfo::getUnBoxHelper(
return temp;
}

void* WrapICorJitInfo::getRuntimeTypePointer(
CORINFO_OBJECT_HANDLE WrapICorJitInfo::getRuntimeTypePointer(
CORINFO_CLASS_HANDLE cls)
{
API_ENTER(getRuntimeTypePointer);
void* temp = wrapHnd->getRuntimeTypePointer(cls);
CORINFO_OBJECT_HANDLE temp = wrapHnd->getRuntimeTypePointer(cls);
API_LEAVE(getRuntimeTypePointer);
return temp;
}

bool WrapICorJitInfo::isObjectImmutable(
void* objPtr)
CORINFO_OBJECT_HANDLE objPtr)
{
API_ENTER(isObjectImmutable);
bool temp = wrapHnd->isObjectImmutable(objPtr);
Expand All @@ -684,7 +684,7 @@ bool WrapICorJitInfo::isObjectImmutable(
}

CORINFO_CLASS_HANDLE WrapICorJitInfo::getObjectType(
void* objPtr)
CORINFO_OBJECT_HANDLE objPtr)
{
API_ENTER(getObjectType);
CORINFO_CLASS_HANDLE temp = wrapHnd->getObjectType(objPtr);
Expand Down Expand Up @@ -965,6 +965,15 @@ bool WrapICorJitInfo::isFieldStatic(
return temp;
}

int WrapICorJitInfo::getArrayLength(
CORINFO_OBJECT_HANDLE objHnd)
{
API_ENTER(getArrayLength);
int temp = wrapHnd->getArrayLength(objHnd);
API_LEAVE(getArrayLength);
return temp;
}

void WrapICorJitInfo::getBoundaries(
CORINFO_METHOD_HANDLE ftn,
unsigned int* cILOffsets,
Expand Down Expand Up @@ -1473,10 +1482,11 @@ void* WrapICorJitInfo::getFieldAddress(
bool WrapICorJitInfo::getReadonlyStaticFieldValue(
CORINFO_FIELD_HANDLE field,
uint8_t* buffer,
int bufferSize)
int bufferSize,
bool ignoreMovableObjects)
{
API_ENTER(getReadonlyStaticFieldValue);
bool temp = wrapHnd->getReadonlyStaticFieldValue(field, buffer, bufferSize);
bool temp = wrapHnd->getReadonlyStaticFieldValue(field, buffer, bufferSize, ignoreMovableObjects);
API_LEAVE(getReadonlyStaticFieldValue);
return temp;
}
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/jit/assertionprop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6127,6 +6127,7 @@ Compiler::fgWalkResult Compiler::optVNConstantPropCurStmt(BasicBlock* block, Sta
case GT_NEG:
case GT_CAST:
case GT_INTRINSIC:
case GT_ARR_LENGTH:
break;

case GT_IND:
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -7779,7 +7779,7 @@ class Compiler
const char* eeGetFieldName(CORINFO_FIELD_HANDLE fieldHnd, const char** classNamePtr = nullptr);

#if defined(DEBUG)
void eePrintObjectDescriptionDescription(const char* prefix, size_t handle);
void eePrintObjectDescriptionDescription(const char* prefix, CORINFO_OBJECT_HANDLE handle);
unsigned eeTryGetClassSize(CORINFO_CLASS_HANDLE clsHnd);
const char16_t* eeGetShortClassName(CORINFO_CLASS_HANDLE clsHnd);
#endif
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/jit/ee_il_dll.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1615,15 +1615,15 @@ const char16_t* Compiler::eeGetShortClassName(CORINFO_CLASS_HANDLE clsHnd)
return param.classNameWidePtr;
}

void Compiler::eePrintObjectDescriptionDescription(const char* prefix, size_t handle)
void Compiler::eePrintObjectDescriptionDescription(const char* prefix, CORINFO_OBJECT_HANDLE handle)
{
const size_t maxStrSize = 64;
char str[maxStrSize];
size_t actualLen = 0;

// Ignore potential SPMI failures
bool success = eeRunFunctorWithSPMIErrorTrap(
[&]() { actualLen = this->info.compCompHnd->printObjectDescription((void*)handle, str, maxStrSize); });
[&]() { actualLen = this->info.compCompHnd->printObjectDescription(handle, str, maxStrSize); });

if (!success)
{
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/emit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4135,7 +4135,7 @@ void emitter::emitDispCommentForHandle(size_t handle, size_t cookie, GenTreeFlag
else if (flag == GTF_ICON_OBJ_HDL)
{
#ifdef DEBUG
emitComp->eePrintObjectDescriptionDescription(commentPrefix, handle);
emitComp->eePrintObjectDescriptionDescription(commentPrefix, (CORINFO_OBJECT_HANDLE)handle);
#else
str = "frozen object handle";
#endif
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/jit/gentree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11134,7 +11134,7 @@ void Compiler::gtDispConst(GenTree* tree)
}
else if (tree->IsIconHandle(GTF_ICON_OBJ_HDL))
{
eePrintObjectDescriptionDescription(" ", tree->AsIntCon()->gtIconVal);
eePrintObjectDescriptionDescription(" ", (CORINFO_OBJECT_HANDLE)tree->AsIntCon()->gtIconVal);
EgorBo marked this conversation as resolved.
Show resolved Hide resolved
}
else
{
Expand Down Expand Up @@ -17619,7 +17619,7 @@ CORINFO_CLASS_HANDLE Compiler::gtGetClassHandle(GenTree* tree, bool* pIsExact, b
{
if (tree->IsIconHandle(GTF_ICON_OBJ_HDL))
{
objClass = info.compCompHnd->getObjectType((void*)tree->AsIntCon()->IconValue());
objClass = info.compCompHnd->getObjectType((CORINFO_OBJECT_HANDLE)tree->AsIntCon()->IconValue());
if (objClass != NO_CLASS_HANDLE)
{
// if we managed to get a class handle it's definitely not null
Expand Down
6 changes: 3 additions & 3 deletions src/coreclr/jit/importer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4344,7 +4344,7 @@ GenTree* Compiler::impImportCnsTreeFromBuffer(uint8_t* buffer, var_types valueTy
}
case TYP_REF:
{
void* ptr;
size_t ptr;
memcpy(&ptr, buffer, sizeof(ssize_t));

if (ptr == 0)
Expand All @@ -4354,9 +4354,9 @@ GenTree* Compiler::impImportCnsTreeFromBuffer(uint8_t* buffer, var_types valueTy
else
{
setMethodHasFrozenObjects();
tree = gtNewIconEmbHndNode(ptr, nullptr, GTF_ICON_OBJ_HDL, nullptr);
tree = gtNewIconEmbHndNode((void*)ptr, nullptr, GTF_ICON_OBJ_HDL, nullptr);
tree->gtType = TYP_REF;
INDEBUG(tree->AsIntCon()->gtTargetHandle = (size_t)ptr);
INDEBUG(tree->AsIntCon()->gtTargetHandle = ptr);
}
break;
}
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/lower.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7180,7 +7180,7 @@ void Lowering::LowerStoreIndirCommon(GenTreeStoreInd* ind)
if (ind->Data()->IsIconHandle(GTF_ICON_OBJ_HDL))
{
const ssize_t handle = ind->Data()->AsIntCon()->IconValue();
if (!comp->info.compCompHnd->isObjectImmutable(reinterpret_cast<void*>(handle)))
if (!comp->info.compCompHnd->isObjectImmutable(reinterpret_cast<CORINFO_OBJECT_HANDLE>(handle)))
{
// On platforms with weaker memory model we need to make sure we use a store with the release semantic
// when we publish a potentially mutable object
Expand Down
6 changes: 3 additions & 3 deletions src/coreclr/jit/morph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8298,11 +8298,11 @@ GenTree* Compiler::fgMorphCall(GenTreeCall* call)
CORINFO_CLASS_HANDLE hClass = gtGetHelperArgClassHandle(argNode);
if ((hClass != NO_CLASS_HANDLE) && !gtIsActiveCSE_Candidate(argNode))
{
void* ptr = info.compCompHnd->getRuntimeTypePointer(hClass);
if (ptr != nullptr)
CORINFO_OBJECT_HANDLE ptr = info.compCompHnd->getRuntimeTypePointer(hClass);
if (ptr != NULL)
{
setMethodHasFrozenObjects();
GenTree* retNode = gtNewIconEmbHndNode(ptr, nullptr, GTF_ICON_OBJ_HDL, nullptr);
GenTree* retNode = gtNewIconEmbHndNode((void*)ptr, nullptr, GTF_ICON_OBJ_HDL, nullptr);
retNode->gtType = TYP_REF;
INDEBUG(retNode->AsIntCon()->gtTargetHandle = (size_t)ptr);
return fgMorphTree(retNode);
Expand Down
Loading