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 1 commit
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
3 changes: 3 additions & 0 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 @@ -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
3 changes: 3 additions & 0 deletions src/coreclr/inc/icorjitinfoimpl_generated.h
Original file line number Diff line number Diff line change
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
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
9 changes: 9 additions & 0 deletions src/coreclr/jit/ICorJitInfo_wrapper_generated.hpp
Original file line number Diff line number Diff line change
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
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
12 changes: 8 additions & 4 deletions src/coreclr/jit/importer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4344,19 +4344,23 @@ 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)
{
tree = gtNewNull();
}
else
else if ((ptr & 1) == 0) // Check the lowest bit for "Is frozen" mark
EgorBo marked this conversation as resolved.
Show resolved Hide resolved
{
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);
}
else
{
return nullptr;
}
break;
}
Expand Down
50 changes: 49 additions & 1 deletion src/coreclr/jit/valuenum.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8685,7 +8685,21 @@ void Compiler::fgValueNumberTree(GenTree* tree)
VNFunc loadFunc =
((tree->gtFlags & GTF_IND_NONNULL) != 0) ? VNF_InvariantNonNullLoad : VNF_InvariantLoad;

tree->gtVNPair = vnStore->VNPairForFunc(tree->TypeGet(), loadFunc, addrNvnp);
// Special case: for initialized non-null 'static readonly' fields we want to keep field
// sequence
// to be able to fold their value
ValueNum fieldSeqVN;
if ((loadFunc == VNF_InvariantNonNullLoad) && addr->IsIconHandle(GTF_ICON_CONST_PTR))
{
fieldSeqVN = vnStore->VNForFieldSeq(addr->AsIntCon()->gtFieldSeq);
}
else
{
fieldSeqVN = vnStore->VNForNull();
EgorBo marked this conversation as resolved.
Show resolved Hide resolved
}

tree->gtVNPair = vnStore->VNPairForFunc(tree->TypeGet(), loadFunc, addrNvnp,
ValueNumPair(fieldSeqVN, fieldSeqVN));
tree->gtVNPair = vnStore->VNPWithExc(tree->gtVNPair, addrXvnp);
}
}
Expand Down Expand Up @@ -8774,6 +8788,40 @@ void Compiler::fgValueNumberTree(GenTree* tree)
}
else // Look up the VNFunc for the node
{
if (tree->OperIs(GT_ARR_LENGTH))
{
ValueNum addressVN = tree->gtGetOp1()->gtVNPair.GetLiberal();
VNFuncApp funcApp;
if (vnStore->GetVNFunc(addressVN, &funcApp) && (funcApp.m_func == VNF_InvariantNonNullLoad))
{
ValueNum fieldSeqVN = funcApp.m_args[1];
if ((fieldSeqVN != vnStore->VNForNull()) && (fieldSeqVN != ValueNumStore::NoVN))
{
FieldSeq* fieldSeq = vnStore->FieldSeqVNToFieldSeq(fieldSeqVN);
if (fieldSeq != nullptr)
{
CORINFO_FIELD_HANDLE field = fieldSeq->GetFieldHandle();
if (field != NULL)
{
uint8_t buffer[TARGET_POINTER_SIZE] = {0};
if (this->info.compCompHnd->getReadonlyStaticFieldValue(field, buffer,
TARGET_POINTER_SIZE))
{
CORINFO_OBJECT_HANDLE objHandle;
memcpy(&objHandle, buffer, TARGET_POINTER_SIZE);
EgorBo marked this conversation as resolved.
Show resolved Hide resolved
int len = this->info.compCompHnd->getArrayLength(objHandle);
if (len >= 0)
{
tree->gtVNPair.SetBoth(vnStore->VNForIntCon(len));
EgorBo marked this conversation as resolved.
Show resolved Hide resolved
return;
}
}
}
}
}
}
}

VNFunc vnf = GetVNFuncForNode(tree);

if (ValueNumStore::VNFuncIsLegal(vnf))
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/jit/valuenumfuncs.h
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,8 @@ ValueNumFuncDef(Box, 3, false, false, false)
ValueNumFuncDef(BoxNullable, 3, false, false, false)

ValueNumFuncDef(LazyStrCns, 2, false, true, false) // Lazy-initialized string literal (helper)
ValueNumFuncDef(InvariantLoad, 1, false, false, false) // Args: 0: (VN of) the address.
ValueNumFuncDef(InvariantNonNullLoad, 1, false, true, false) // Args: 0: (VN of) the address.
ValueNumFuncDef(InvariantLoad, 2, false, false, false) // Args: 0: (VN of) the address, 1: field sequence (if any).
ValueNumFuncDef(InvariantNonNullLoad, 2, false, true, false) // Args: 0: (VN of) the address, 1: field sequence (if any).
ValueNumFuncDef(Unbox, 2, false, true, false)

ValueNumFuncDef(LT_UN, 2, false, false, false) // unsigned or unordered comparisons
Expand Down
10 changes: 10 additions & 0 deletions src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2881,6 +2881,16 @@ private bool isFieldStatic(CORINFO_FIELD_STRUCT_* fldHnd)
return HandleToObject(fldHnd).IsStatic;
}

#pragma warning disable CA1822 // Mark members as static
private int getArrayLength(CORINFO_FIELD_STRUCT_* objHnd)
#pragma warning restore CA1822 // Mark members as static
{
// This is not used on crossgen and NativeAOT.
// For NativeAOT we'll need a different API, e.g. "getArrayLength(void* frozenObjPtr)"
// We'll add it as part of "allocate static arrays on FOH" effort for CoreCLR
return -1;
}

private void getBoundaries(CORINFO_METHOD_STRUCT_* ftn, ref uint cILOffsets, ref uint* pILOffsets, BoundaryTypes* implicitBoundaries)
{
// TODO: Debugging
Expand Down
Loading