From 3699de56bca63da4217aefaa381cbb44047e2890 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Wed, 26 Apr 2023 14:35:06 +0200 Subject: [PATCH 1/5] Introduce new handle kinds --- src/coreclr/jit/assertionprop.cpp | 5 +-- src/coreclr/jit/codegenxarch.cpp | 2 +- src/coreclr/jit/compiler.cpp | 52 +++++++++---------------------- src/coreclr/jit/emit.cpp | 8 ++--- src/coreclr/jit/emitarm64.cpp | 5 +-- src/coreclr/jit/fgdiagnostic.cpp | 8 +++-- src/coreclr/jit/gentree.cpp | 33 +++++++++----------- src/coreclr/jit/gentree.h | 52 +++++++++++++++---------------- src/coreclr/jit/importer.cpp | 11 +++---- src/coreclr/jit/morph.cpp | 11 ++----- src/coreclr/jit/optcse.cpp | 4 +-- src/coreclr/jit/optimizer.cpp | 4 ++- src/coreclr/jit/valuenum.cpp | 6 ++++ 13 files changed, 89 insertions(+), 112 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index d7369b3adcce26..aeaf811a540e1d 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -2855,7 +2855,7 @@ GenTree* Compiler::optVNConstantPropOnTree(BasicBlock* block, GenTree* tree) bool Compiler::optIsProfitableToSubstitute(GenTree* dest, BasicBlock* destBlock, GenTree* value) { // Giving up on these kinds of handles demonstrated size improvements - if (value->IsIconHandle(GTF_ICON_STATIC_HDL, GTF_ICON_CLASS_HDL)) + if (value->IsIconHandle(GTF_ICON_STATIC_HDL, GTF_ICON_STATIC_HDL_CCTOR, GTF_ICON_CLASS_HDL)) { return false; } @@ -2956,7 +2956,8 @@ GenTree* Compiler::optConstantAssertionProp(AssertionDsc* curAssertion, // Don't propagate handles if we need to report relocs. if (opts.compReloc && curAssertion->op2.HasIconFlag() && curAssertion->op2.u1.iconVal != 0) { - if (curAssertion->op2.GetIconFlag() == GTF_ICON_STATIC_HDL) + if ((curAssertion->op2.GetIconFlag() == GTF_ICON_STATIC_HDL) || + (curAssertion->op2.GetIconFlag() == GTF_ICON_STATIC_HDL_CCTOR)) { propagateType = true; } diff --git a/src/coreclr/jit/codegenxarch.cpp b/src/coreclr/jit/codegenxarch.cpp index 4d46570346cc24..ade10c23effc05 100644 --- a/src/coreclr/jit/codegenxarch.cpp +++ b/src/coreclr/jit/codegenxarch.cpp @@ -5418,7 +5418,7 @@ void CodeGen::genCodeForIndir(GenTreeIndir* tree) emitter* emit = GetEmitter(); GenTree* addr = tree->Addr(); - if (addr->IsIconHandle(GTF_ICON_TLS_HDL)) + if (addr->IsIconHandle(GTF_ICON_TLS_HDL, GTF_ICON_TLS_HDL_CCTOR)) { noway_assert(EA_ATTR(genTypeSize(targetType)) == EA_PTRSIZE); #if TARGET_64BIT diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index 6cf88c87b0a237..04a6b0287a68ec 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -9854,97 +9854,75 @@ void cTreeFlags(Compiler* comp, GenTree* tree) switch (handleKind) { - case GTF_ICON_SCOPE_HDL: - chars += printf("[ICON_SCOPE_HDL]"); break; - case GTF_ICON_CLASS_HDL: - chars += printf("[ICON_CLASS_HDL]"); break; - case GTF_ICON_METHOD_HDL: - chars += printf("[ICON_METHOD_HDL]"); break; - case GTF_ICON_FIELD_HDL: - chars += printf("[ICON_FIELD_HDL]"); break; - case GTF_ICON_STATIC_HDL: - chars += printf("[ICON_STATIC_HDL]"); break; - + case GTF_ICON_STATIC_HDL_CCTOR: + chars += printf("[ICON_STATIC_HDL_CCTOR]"); + break; case GTF_ICON_STR_HDL: - chars += printf("[ICON_STR_HDL]"); break; - case GTF_ICON_OBJ_HDL: - chars += printf("[ICON_OBJ_HDL]"); break; - case GTF_ICON_CONST_PTR: - chars += printf("[ICON_CONST_PTR]"); break; - + case GTF_ICON_CONST_PTR_CCTOR: + chars += printf("[ICON_CONST_PTR_CCTOR]"); + break; case GTF_ICON_GLOBAL_PTR: - chars += printf("[ICON_GLOBAL_PTR]"); break; - + case GTF_ICON_GLOBAL_PTR_CCTOR: + chars += printf("[ICON_GLOBAL_PTR_CCTOR]"); + break; case GTF_ICON_VARG_HDL: - chars += printf("[ICON_VARG_HDL]"); break; - case GTF_ICON_PINVKI_HDL: - chars += printf("[ICON_PINVKI_HDL]"); break; - case GTF_ICON_TOKEN_HDL: - chars += printf("[ICON_TOKEN_HDL]"); break; - case GTF_ICON_TLS_HDL: - chars += printf("[ICON_TLD_HDL]"); break; - + case GTF_ICON_TLS_HDL_CCTOR: + chars += printf("[ICON_TLD_HDL_CCTOR]"); + break; case GTF_ICON_FTN_ADDR: - chars += printf("[ICON_FTN_ADDR]"); break; - case GTF_ICON_CIDMID_HDL: - chars += printf("[ICON_CIDMID_HDL]"); break; - case GTF_ICON_BBC_PTR: - chars += printf("[ICON_BBC_PTR]"); break; - case GTF_ICON_STATIC_BOX_PTR: - chars += printf("[GTF_ICON_STATIC_BOX_PTR]"); break; - + case GTF_ICON_STATIC_BOX_PTR_CCTOR: + chars += printf("[GTF_ICON_STATIC_BOX_PTR_CCTOR]"); + break; case GTF_ICON_STATIC_ADDR_PTR: - chars += printf("[GTF_ICON_STATIC_ADDR_PTR]"); break; - default: assert(!"a forgotten handle flag"); break; diff --git a/src/coreclr/jit/emit.cpp b/src/coreclr/jit/emit.cpp index 97de8d503606c9..ec1d03c68a4d20 100644 --- a/src/coreclr/jit/emit.cpp +++ b/src/coreclr/jit/emit.cpp @@ -4396,7 +4396,7 @@ void emitter::emitDispCommentForHandle(size_t handle, size_t cookie, GenTreeFlag return; } - if ((flag == GTF_ICON_STATIC_HDL) || (flag == GTF_ICON_STATIC_BOX_PTR)) + if ((flag == GTF_ICON_STATIC_HDL) || (flag == GTF_ICON_STATIC_HDL_CCTOR) || (flag == GTF_ICON_STATIC_BOX_PTR)) { const char* fieldName = emitComp->eeGetFieldName(reinterpret_cast(cookie), true, buffer, sizeof(buffer)); @@ -4433,11 +4433,11 @@ void emitter::emitDispCommentForHandle(size_t handle, size_t cookie, GenTreeFlag { str = emitComp->eeGetClassName(reinterpret_cast(handle)); } - else if (flag == GTF_ICON_CONST_PTR) + else if ((flag == GTF_ICON_CONST_PTR) || (flag == GTF_ICON_CONST_PTR_CCTOR)) { str = "const ptr"; } - else if (flag == GTF_ICON_GLOBAL_PTR) + else if ((flag == GTF_ICON_GLOBAL_PTR) || (flag == GTF_ICON_GLOBAL_PTR_CCTOR)) { str = "global ptr"; } @@ -4445,7 +4445,7 @@ void emitter::emitDispCommentForHandle(size_t handle, size_t cookie, GenTreeFlag { str = emitComp->eeGetFieldName(reinterpret_cast(handle), true, buffer, sizeof(buffer)); } - else if (flag == GTF_ICON_STATIC_HDL) + else if ((flag == GTF_ICON_STATIC_HDL) || (flag == GTF_ICON_STATIC_HDL_CCTOR)) { str = "static handle"; } diff --git a/src/coreclr/jit/emitarm64.cpp b/src/coreclr/jit/emitarm64.cpp index 657f9645de04c4..8d63533cec7c29 100644 --- a/src/coreclr/jit/emitarm64.cpp +++ b/src/coreclr/jit/emitarm64.cpp @@ -13804,7 +13804,8 @@ void emitter::emitInsLoadStoreOp(instruction ins, emitAttr attr, regNumber dataR if (addr->isContained()) { - assert(addr->OperIs(GT_CLS_VAR_ADDR, GT_LCL_ADDR, GT_LEA) || (addr->IsIconHandle(GTF_ICON_TLS_HDL))); + assert(addr->OperIs(GT_CLS_VAR_ADDR, GT_LCL_ADDR, GT_LEA) || + (addr->IsIconHandle(GTF_ICON_TLS_HDL, GTF_ICON_TLS_HDL_CCTOR))); int offset = 0; DWORD lsl = 0; @@ -13927,7 +13928,7 @@ void emitter::emitInsLoadStoreOp(instruction ins, emitAttr attr, regNumber dataR emitIns_R_S(ins, attr, dataReg, lclNum, offset); } } - else if (addr->IsIconHandle(GTF_ICON_TLS_HDL)) + else if (addr->IsIconHandle(GTF_ICON_TLS_HDL, GTF_ICON_TLS_HDL_CCTOR)) { // On Arm64, TEB is in r18, so load from the r18 as base. emitIns_R_R_I(ins, attr, dataReg, REG_R18, addr->AsIntCon()->IconValue()); diff --git a/src/coreclr/jit/fgdiagnostic.cpp b/src/coreclr/jit/fgdiagnostic.cpp index d1b1adb3b0d376..ae06bfbdc90181 100644 --- a/src/coreclr/jit/fgdiagnostic.cpp +++ b/src/coreclr/jit/fgdiagnostic.cpp @@ -3106,9 +3106,11 @@ void Compiler::fgDebugCheckFlags(GenTree* tree) GenTreeFlags handleKind = op1->GetIconHandleFlag(); // Some of these aren't handles to invariant data... - if ((handleKind == GTF_ICON_STATIC_HDL) || // Pointer to a mutable class Static variable - (handleKind == GTF_ICON_BBC_PTR) || // Pointer to a mutable basic block count value - (handleKind == GTF_ICON_GLOBAL_PTR)) // Pointer to mutable data from the VM state + if ((handleKind == GTF_ICON_STATIC_HDL) || // Pointer to a mutable class Static variable + (handleKind == GTF_ICON_STATIC_HDL_CCTOR) || // Pointer to a mutable class Static variable + (handleKind == GTF_ICON_BBC_PTR) || // Pointer to a mutable basic block count value + (handleKind == GTF_ICON_GLOBAL_PTR) || // Pointer to mutable data from the VM state + (handleKind == GTF_ICON_GLOBAL_PTR_CCTOR)) // Pointer to mutable data from the VM state { // For statics, we expect the GTF_GLOB_REF to be set. However, we currently // fail to set it in a number of situations, and so this check is disabled. diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 454bdccb53c91e..20007e1a87b3e2 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -7203,9 +7203,11 @@ GenTree* Compiler::gtNewIndOfIconHandleNode(var_types indType, size_t addr, GenT if (isInvariant) { - assert(iconFlags != GTF_ICON_STATIC_HDL); // Pointer to a mutable class Static variable - assert(iconFlags != GTF_ICON_BBC_PTR); // Pointer to a mutable basic block count value - assert(iconFlags != GTF_ICON_GLOBAL_PTR); // Pointer to mutable data from the VM state + assert(iconFlags != GTF_ICON_STATIC_HDL); // Pointer to a mutable class Static variable + assert(iconFlags != GTF_ICON_STATIC_HDL_CCTOR); // Pointer to a mutable class Static variable + assert(iconFlags != GTF_ICON_BBC_PTR); // Pointer to a mutable basic block count value + assert(iconFlags != GTF_ICON_GLOBAL_PTR); // Pointer to mutable data from the VM state + assert(iconFlags != GTF_ICON_GLOBAL_PTR_CCTOR); // Pointer to mutable data from the VM state // This indirection also is invariant. indirFlags |= GTF_IND_INVARIANT; @@ -10762,19 +10764,9 @@ void Compiler::gtDispNode(GenTree* tree, IndentStack* indentStack, _In_ _In_opt_ case GT_CNS_INT: if (tree->IsIconHandle()) { - if ((tree->gtFlags & GTF_ICON_INITCLASS) != 0) - { - printf("I"); // Static Field handle with INITCLASS requirement - --msgLength; - break; - } - else - { - // Some other handle - printf("H"); - --msgLength; - break; - } + printf("H"); + --msgLength; + break; } goto DASH; @@ -11456,6 +11448,7 @@ void Compiler::gtDispConst(GenTree* tree) printf(" field"); break; case GTF_ICON_STATIC_HDL: + case GTF_ICON_STATIC_HDL_CCTOR: printf(" static"); break; case GTF_ICON_OBJ_HDL: @@ -11463,9 +11456,11 @@ void Compiler::gtDispConst(GenTree* tree) unreached(); // These cases are handled above break; case GTF_ICON_CONST_PTR: + case GTF_ICON_CONST_PTR_CCTOR: printf(" const ptr"); break; case GTF_ICON_GLOBAL_PTR: + case GTF_ICON_GLOBAL_PTR_CCTOR: printf(" global ptr"); break; case GTF_ICON_VARG_HDL: @@ -11478,6 +11473,7 @@ void Compiler::gtDispConst(GenTree* tree) printf(" token"); break; case GTF_ICON_TLS_HDL: + case GTF_ICON_TLS_HDL_CCTOR: printf(" tls"); break; case GTF_ICON_FTN_ADDR: @@ -17782,7 +17778,7 @@ bool GenTree::IsFieldAddr(Compiler* comp, GenTree** pBaseAddr, FieldSeq** pFldSe return false; } } - else if (IsIconHandle(GTF_ICON_STATIC_HDL)) + else if (IsIconHandle(GTF_ICON_STATIC_HDL, GTF_ICON_STATIC_HDL_CCTOR)) { fldSeq = AsIntCon()->gtFieldSeq; offset = AsIntCon()->IconValue(); @@ -18160,7 +18156,8 @@ CORINFO_CLASS_HANDLE Compiler::gtGetClassHandle(GenTree* tree, bool* pIsExact, b } } } - else if (base->IsIconHandle(GTF_ICON_CONST_PTR, GTF_ICON_STATIC_HDL)) + else if (base->IsIconHandle(GTF_ICON_CONST_PTR, GTF_ICON_CONST_PTR_CCTOR, GTF_ICON_STATIC_HDL, + GTF_ICON_STATIC_HDL_CCTOR)) { // Check if we have IND(ICON_HANDLE) that represents a static field FieldSeq* fldSeq = base->AsIntCon()->gtFieldSeq; diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 9c7cc95947617a..055c92fce24641 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -518,36 +518,34 @@ enum GenTreeFlags : unsigned int GTF_ARR_ADDR_NONNULL = 0x80000000, // GT_ARR_ADDR -- this array's address is not null - GTF_ICON_HDL_MASK = 0xFF000000, // Bits used by handle types below - GTF_ICON_SCOPE_HDL = 0x01000000, // GT_CNS_INT -- constant is a scope handle - GTF_ICON_CLASS_HDL = 0x02000000, // GT_CNS_INT -- constant is a class handle - GTF_ICON_METHOD_HDL = 0x03000000, // GT_CNS_INT -- constant is a method handle - GTF_ICON_FIELD_HDL = 0x04000000, // GT_CNS_INT -- constant is a field handle - GTF_ICON_STATIC_HDL = 0x05000000, // GT_CNS_INT -- constant is a handle to static data - GTF_ICON_STR_HDL = 0x06000000, // GT_CNS_INT -- constant is a pinned handle pointing to a string object - GTF_ICON_OBJ_HDL = 0x12000000, // GT_CNS_INT -- constant is an object handle (e.g. frozen string or Type object) - GTF_ICON_CONST_PTR = 0x07000000, // GT_CNS_INT -- constant is a pointer to immutable data, (e.g. IAT_PPVALUE) - GTF_ICON_GLOBAL_PTR = 0x08000000, // GT_CNS_INT -- constant is a pointer to mutable data (e.g. from the VM state) - GTF_ICON_VARG_HDL = 0x09000000, // GT_CNS_INT -- constant is a var arg cookie handle - GTF_ICON_PINVKI_HDL = 0x0A000000, // GT_CNS_INT -- constant is a pinvoke calli handle - GTF_ICON_TOKEN_HDL = 0x0B000000, // GT_CNS_INT -- constant is a token handle (other than class, method or field) - GTF_ICON_TLS_HDL = 0x0C000000, // GT_CNS_INT -- constant is a TLS ref with offset - GTF_ICON_FTN_ADDR = 0x0D000000, // GT_CNS_INT -- constant is a function address - GTF_ICON_CIDMID_HDL = 0x0E000000, // GT_CNS_INT -- constant is a class ID or a module ID - GTF_ICON_BBC_PTR = 0x0F000000, // GT_CNS_INT -- constant is a basic block count pointer - GTF_ICON_STATIC_BOX_PTR = 0x10000000, // GT_CNS_INT -- constant is an address of the box for a STATIC_IN_HEAP field - GTF_ICON_FIELD_SEQ = 0x11000000, // <--------> -- constant is a FieldSeq* (used only as VNHandle) - GTF_ICON_STATIC_ADDR_PTR = 0x13000000, // GT_CNS_INT -- constant is a pointer to a static base address + GTF_ICON_HDL_MASK = 0xFF000000, // Bits used by handle types below + GTF_ICON_SCOPE_HDL = 0x01000000, // GT_CNS_INT -- constant is a scope handle + GTF_ICON_CLASS_HDL = 0x02000000, // GT_CNS_INT -- constant is a class handle + GTF_ICON_METHOD_HDL = 0x03000000, // GT_CNS_INT -- constant is a method handle + GTF_ICON_FIELD_HDL = 0x04000000, // GT_CNS_INT -- constant is a field handle + GTF_ICON_STATIC_HDL = 0x05000000, // GT_CNS_INT -- constant is a handle to static data + GTF_ICON_STATIC_HDL_CCTOR = 0x06000000, // GT_CNS_INT -- constant is a handle to cctor dependent static data + GTF_ICON_STR_HDL = 0x07000000, // GT_CNS_INT -- constant is a pinned handle pointing to a string object + GTF_ICON_OBJ_HDL = 0x08000000, // GT_CNS_INT -- constant is an object handle (e.g. frozen string or Type object) + GTF_ICON_CONST_PTR = 0x09000000, // GT_CNS_INT -- constant is a pointer to immutable data, (e.g. IAT_PPVALUE) + GTF_ICON_CONST_PTR_CCTOR = 0x0A000000, // GT_CNS_INT -- constant is a pointer to cctor dependent immutable data + GTF_ICON_GLOBAL_PTR = 0x0B000000, // GT_CNS_INT -- constant is a pointer to mutable data (e.g. from the VM state) + GTF_ICON_GLOBAL_PTR_CCTOR = 0x18000000, // GT_CNS_INT -- constant is a pointer to cctor dependent mutable data (e.g. from the VM state) + GTF_ICON_VARG_HDL = 0x0C000000, // GT_CNS_INT -- constant is a var arg cookie handle + GTF_ICON_PINVKI_HDL = 0x0D000000, // GT_CNS_INT -- constant is a pinvoke calli handle + GTF_ICON_TOKEN_HDL = 0x0E000000, // GT_CNS_INT -- constant is a token handle (other than class, method or field) + GTF_ICON_TLS_HDL = 0x0F000000, // GT_CNS_INT -- constant is a TLS ref with offset + GTF_ICON_TLS_HDL_CCTOR = 0x10000000, // GT_CNS_INT -- constant is a cctor dependent TLS ref with offset + GTF_ICON_FTN_ADDR = 0x11000000, // GT_CNS_INT -- constant is a function address + GTF_ICON_CIDMID_HDL = 0x12000000, // GT_CNS_INT -- constant is a class ID or a module ID + GTF_ICON_BBC_PTR = 0x13000000, // GT_CNS_INT -- constant is a basic block count pointer + GTF_ICON_STATIC_BOX_PTR = 0x14000000, // GT_CNS_INT -- constant is an address of the box for a STATIC_IN_HEAP field + GTF_ICON_STATIC_BOX_PTR_CCTOR = 0x15000000, // GT_CNS_INT -- constant is an address of the box for a cctor dependent STATIC_IN_HEAP field + GTF_ICON_FIELD_SEQ = 0x16000000, // GT_CNS_INT -- constant is a FieldSeq* (used only as VNHandle) + GTF_ICON_STATIC_ADDR_PTR = 0x17000000, // GT_CNS_INT -- constant is a pointer to a static base address // GTF_ICON_REUSE_REG_VAL = 0x00800000 // GT_CNS_INT -- GTF_REUSE_REG_VAL, defined above GTF_ICON_SIMD_COUNT = 0x00200000, // GT_CNS_INT -- constant is Vector.Count - GTF_ICON_INITCLASS = 0x00100000, // GT_CNS_INT -- Constant is used to access a static that requires preceding - // class/static init helper. In some cases, the constant is - // the address of the static field itself, and in other cases - // there's an extra layer of indirection and it is the address - // of the cell that the runtime will fill in with the address - // of the static field; in both of those cases, the constant - // is what gets flagged. GTF_OVERFLOW = 0x10000000, // Supported for: GT_ADD, GT_SUB, GT_MUL and GT_CAST. // Requires an overflow check. Use gtOverflow(Ex)() to check this flag. diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index ad16d27762867d..52d7d70000a1bc 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -4331,25 +4331,22 @@ GenTree* Compiler::impImportStaticFieldAccess(CORINFO_RESOLVED_TOKEN* pResolvedT assert(pFieldInfo->fieldLookup.accessType == IAT_VALUE); size_t fldAddr = reinterpret_cast(pFieldInfo->fieldLookup.addr); GenTreeFlags handleKind; + bool cctorDependent = (pFieldInfo->fieldFlags & CORINFO_FLG_FIELD_INITCLASS) != 0; if (isBoxedStatic) { - handleKind = GTF_ICON_STATIC_BOX_PTR; + handleKind = cctorDependent ? GTF_ICON_STATIC_BOX_PTR_CCTOR : GTF_ICON_STATIC_BOX_PTR; } else if (isStaticReadOnlyInitedRef) { - handleKind = GTF_ICON_CONST_PTR; + handleKind = cctorDependent ? GTF_ICON_CONST_PTR_CCTOR : GTF_ICON_CONST_PTR; } else { - handleKind = GTF_ICON_STATIC_HDL; + handleKind = cctorDependent ? GTF_ICON_STATIC_HDL_CCTOR : GTF_ICON_STATIC_HDL; } isHoistable = true; op1 = gtNewIconHandleNode(fldAddr, handleKind, innerFldSeq); INDEBUG(op1->AsIntCon()->gtTargetHandle = reinterpret_cast(pResolvedToken->hField)); - if (pFieldInfo->fieldFlags & CORINFO_FLG_FIELD_INITCLASS) - { - op1->gtFlags |= GTF_ICON_INITCLASS; - } break; } } diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index ffe382e1952ee4..f85b2e09dec429 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -5345,14 +5345,9 @@ GenTree* Compiler::fgMorphExpandTlsFieldAddr(GenTree* tree) #define WIN32_TLS_SLOTS (0x2C) // Offset from fs:[0] where the pointer to the slots resides // Mark this ICON as a TLS_HDL, codegen will use FS:[cns] - GenTree* tlsRef = gtNewIconHandleNode(WIN32_TLS_SLOTS, GTF_ICON_TLS_HDL); - - // Translate GTF_FLD_INITCLASS to GTF_ICON_INITCLASS - if ((tree->gtFlags & GTF_FLD_INITCLASS) != 0) - { - tree->gtFlags &= ~GTF_FLD_INITCLASS; - tlsRef->gtFlags |= GTF_ICON_INITCLASS; - } + GenTreeFlags handleType = ((tree->gtFlags & GTF_FLD_INITCLASS) != 0) ? GTF_ICON_TLS_HDL_CCTOR : GTF_ICON_TLS_HDL; + GenTree* tlsRef = gtNewIconHandleNode(WIN32_TLS_SLOTS, handleType); + tree->gtFlags &= ~GTF_FLD_INITCLASS; tlsRef = gtNewIndir(TYP_I_IMPL, tlsRef, GTF_IND_NONFAULTING | GTF_IND_INVARIANT); diff --git a/src/coreclr/jit/optcse.cpp b/src/coreclr/jit/optcse.cpp index 9f8e2ee023491a..b598a795f4889c 100644 --- a/src/coreclr/jit/optcse.cpp +++ b/src/coreclr/jit/optcse.cpp @@ -766,8 +766,8 @@ bool Compiler::optValnumCSE_Locate() { if (!enableConstCSE && // Unconditionally allow these constant handles to be CSE'd - !tree->IsIconHandle(GTF_ICON_STATIC_HDL) && !tree->IsIconHandle(GTF_ICON_CLASS_HDL) && - !tree->IsIconHandle(GTF_ICON_STR_HDL) && !tree->IsIconHandle(GTF_ICON_OBJ_HDL)) + !tree->IsIconHandle(GTF_ICON_STATIC_HDL, GTF_ICON_STATIC_HDL_CCTOR, GTF_ICON_CLASS_HDL, + GTF_ICON_STR_HDL, GTF_ICON_OBJ_HDL)) { continue; } diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp index e154aba816f469..fd94757f8a88b1 100644 --- a/src/coreclr/jit/optimizer.cpp +++ b/src/coreclr/jit/optimizer.cpp @@ -7543,7 +7543,9 @@ void Compiler::optHoistLoopBlocks(unsigned loopNum, ArrayStack* blo // isCctorDependent and isAddressWhoseDereferenceWouldBeCctorDependent, but we don't for // simplicity/throughput; the constant itself would be considered non-hoistable anyway, since // optIsCSEcandidate returns false for constants. - bool treeIsCctorDependent = tree->OperIs(GT_CNS_INT) && ((tree->gtFlags & GTF_ICON_INITCLASS) != 0); + bool treeIsCctorDependent = tree->OperIs(GT_CNS_INT) && + tree->IsIconHandle(GTF_ICON_STATIC_HDL_CCTOR, GTF_ICON_CONST_PTR_CCTOR, + GTF_ICON_TLS_HDL_CCTOR, GTF_ICON_STATIC_BOX_PTR_CCTOR); bool treeIsInvariant = true; bool treeHasHoistableChildren = false; int childCount; diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 3d258bdb8263ea..3091755714611a 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -5896,10 +5896,16 @@ GenTreeFlags ValueNumStore::GetFoldedArithOpResultHandleFlags(ValueNum vn) case GTF_ICON_STATIC_BOX_PTR: case GTF_ICON_STATIC_ADDR_PTR: return GTF_ICON_CONST_PTR; + case GTF_ICON_CONST_PTR_CCTOR: + case GTF_ICON_TLS_HDL_CCTOR: + return GTF_ICON_CONST_PTR_CCTOR; case GTF_ICON_STATIC_HDL: case GTF_ICON_GLOBAL_PTR: case GTF_ICON_BBC_PTR: return GTF_ICON_GLOBAL_PTR; + case GTF_ICON_STATIC_HDL_CCTOR: + case GTF_ICON_GLOBAL_PTR_CCTOR: + return GTF_ICON_GLOBAL_PTR_CCTOR; default: assert(!"Unexpected handle type"); return flags; From ee80bdbbe6446eb8a831cd46413fde6f82c0c83a Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Wed, 26 Apr 2023 14:35:15 +0200 Subject: [PATCH 2/5] Revert "Introduce new handle kinds" This reverts commit 3699de56bca63da4217aefaa381cbb44047e2890. --- src/coreclr/jit/assertionprop.cpp | 5 ++- src/coreclr/jit/codegenxarch.cpp | 2 +- src/coreclr/jit/compiler.cpp | 52 ++++++++++++++++++++++--------- src/coreclr/jit/emit.cpp | 8 ++--- src/coreclr/jit/emitarm64.cpp | 5 ++- src/coreclr/jit/fgdiagnostic.cpp | 8 ++--- src/coreclr/jit/gentree.cpp | 33 +++++++++++--------- src/coreclr/jit/gentree.h | 52 ++++++++++++++++--------------- src/coreclr/jit/importer.cpp | 11 ++++--- src/coreclr/jit/morph.cpp | 11 +++++-- src/coreclr/jit/optcse.cpp | 4 +-- src/coreclr/jit/optimizer.cpp | 4 +-- src/coreclr/jit/valuenum.cpp | 6 ---- 13 files changed, 112 insertions(+), 89 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index aeaf811a540e1d..d7369b3adcce26 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -2855,7 +2855,7 @@ GenTree* Compiler::optVNConstantPropOnTree(BasicBlock* block, GenTree* tree) bool Compiler::optIsProfitableToSubstitute(GenTree* dest, BasicBlock* destBlock, GenTree* value) { // Giving up on these kinds of handles demonstrated size improvements - if (value->IsIconHandle(GTF_ICON_STATIC_HDL, GTF_ICON_STATIC_HDL_CCTOR, GTF_ICON_CLASS_HDL)) + if (value->IsIconHandle(GTF_ICON_STATIC_HDL, GTF_ICON_CLASS_HDL)) { return false; } @@ -2956,8 +2956,7 @@ GenTree* Compiler::optConstantAssertionProp(AssertionDsc* curAssertion, // Don't propagate handles if we need to report relocs. if (opts.compReloc && curAssertion->op2.HasIconFlag() && curAssertion->op2.u1.iconVal != 0) { - if ((curAssertion->op2.GetIconFlag() == GTF_ICON_STATIC_HDL) || - (curAssertion->op2.GetIconFlag() == GTF_ICON_STATIC_HDL_CCTOR)) + if (curAssertion->op2.GetIconFlag() == GTF_ICON_STATIC_HDL) { propagateType = true; } diff --git a/src/coreclr/jit/codegenxarch.cpp b/src/coreclr/jit/codegenxarch.cpp index ade10c23effc05..4d46570346cc24 100644 --- a/src/coreclr/jit/codegenxarch.cpp +++ b/src/coreclr/jit/codegenxarch.cpp @@ -5418,7 +5418,7 @@ void CodeGen::genCodeForIndir(GenTreeIndir* tree) emitter* emit = GetEmitter(); GenTree* addr = tree->Addr(); - if (addr->IsIconHandle(GTF_ICON_TLS_HDL, GTF_ICON_TLS_HDL_CCTOR)) + if (addr->IsIconHandle(GTF_ICON_TLS_HDL)) { noway_assert(EA_ATTR(genTypeSize(targetType)) == EA_PTRSIZE); #if TARGET_64BIT diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index 04a6b0287a68ec..6cf88c87b0a237 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -9854,75 +9854,97 @@ void cTreeFlags(Compiler* comp, GenTree* tree) switch (handleKind) { + case GTF_ICON_SCOPE_HDL: + chars += printf("[ICON_SCOPE_HDL]"); break; + case GTF_ICON_CLASS_HDL: + chars += printf("[ICON_CLASS_HDL]"); break; + case GTF_ICON_METHOD_HDL: + chars += printf("[ICON_METHOD_HDL]"); break; + case GTF_ICON_FIELD_HDL: + chars += printf("[ICON_FIELD_HDL]"); break; + case GTF_ICON_STATIC_HDL: + chars += printf("[ICON_STATIC_HDL]"); break; - case GTF_ICON_STATIC_HDL_CCTOR: - chars += printf("[ICON_STATIC_HDL_CCTOR]"); - break; + case GTF_ICON_STR_HDL: + chars += printf("[ICON_STR_HDL]"); break; + case GTF_ICON_OBJ_HDL: + chars += printf("[ICON_OBJ_HDL]"); break; + case GTF_ICON_CONST_PTR: + chars += printf("[ICON_CONST_PTR]"); break; - case GTF_ICON_CONST_PTR_CCTOR: - chars += printf("[ICON_CONST_PTR_CCTOR]"); - break; + case GTF_ICON_GLOBAL_PTR: + chars += printf("[ICON_GLOBAL_PTR]"); break; - case GTF_ICON_GLOBAL_PTR_CCTOR: - chars += printf("[ICON_GLOBAL_PTR_CCTOR]"); - break; + case GTF_ICON_VARG_HDL: + chars += printf("[ICON_VARG_HDL]"); break; + case GTF_ICON_PINVKI_HDL: + chars += printf("[ICON_PINVKI_HDL]"); break; + case GTF_ICON_TOKEN_HDL: + chars += printf("[ICON_TOKEN_HDL]"); break; + case GTF_ICON_TLS_HDL: + chars += printf("[ICON_TLD_HDL]"); break; - case GTF_ICON_TLS_HDL_CCTOR: - chars += printf("[ICON_TLD_HDL_CCTOR]"); - break; + case GTF_ICON_FTN_ADDR: + chars += printf("[ICON_FTN_ADDR]"); break; + case GTF_ICON_CIDMID_HDL: + chars += printf("[ICON_CIDMID_HDL]"); break; + case GTF_ICON_BBC_PTR: + chars += printf("[ICON_BBC_PTR]"); break; + case GTF_ICON_STATIC_BOX_PTR: + chars += printf("[GTF_ICON_STATIC_BOX_PTR]"); break; - case GTF_ICON_STATIC_BOX_PTR_CCTOR: - chars += printf("[GTF_ICON_STATIC_BOX_PTR_CCTOR]"); - break; + case GTF_ICON_STATIC_ADDR_PTR: + chars += printf("[GTF_ICON_STATIC_ADDR_PTR]"); break; + default: assert(!"a forgotten handle flag"); break; diff --git a/src/coreclr/jit/emit.cpp b/src/coreclr/jit/emit.cpp index ec1d03c68a4d20..97de8d503606c9 100644 --- a/src/coreclr/jit/emit.cpp +++ b/src/coreclr/jit/emit.cpp @@ -4396,7 +4396,7 @@ void emitter::emitDispCommentForHandle(size_t handle, size_t cookie, GenTreeFlag return; } - if ((flag == GTF_ICON_STATIC_HDL) || (flag == GTF_ICON_STATIC_HDL_CCTOR) || (flag == GTF_ICON_STATIC_BOX_PTR)) + if ((flag == GTF_ICON_STATIC_HDL) || (flag == GTF_ICON_STATIC_BOX_PTR)) { const char* fieldName = emitComp->eeGetFieldName(reinterpret_cast(cookie), true, buffer, sizeof(buffer)); @@ -4433,11 +4433,11 @@ void emitter::emitDispCommentForHandle(size_t handle, size_t cookie, GenTreeFlag { str = emitComp->eeGetClassName(reinterpret_cast(handle)); } - else if ((flag == GTF_ICON_CONST_PTR) || (flag == GTF_ICON_CONST_PTR_CCTOR)) + else if (flag == GTF_ICON_CONST_PTR) { str = "const ptr"; } - else if ((flag == GTF_ICON_GLOBAL_PTR) || (flag == GTF_ICON_GLOBAL_PTR_CCTOR)) + else if (flag == GTF_ICON_GLOBAL_PTR) { str = "global ptr"; } @@ -4445,7 +4445,7 @@ void emitter::emitDispCommentForHandle(size_t handle, size_t cookie, GenTreeFlag { str = emitComp->eeGetFieldName(reinterpret_cast(handle), true, buffer, sizeof(buffer)); } - else if ((flag == GTF_ICON_STATIC_HDL) || (flag == GTF_ICON_STATIC_HDL_CCTOR)) + else if (flag == GTF_ICON_STATIC_HDL) { str = "static handle"; } diff --git a/src/coreclr/jit/emitarm64.cpp b/src/coreclr/jit/emitarm64.cpp index 8d63533cec7c29..657f9645de04c4 100644 --- a/src/coreclr/jit/emitarm64.cpp +++ b/src/coreclr/jit/emitarm64.cpp @@ -13804,8 +13804,7 @@ void emitter::emitInsLoadStoreOp(instruction ins, emitAttr attr, regNumber dataR if (addr->isContained()) { - assert(addr->OperIs(GT_CLS_VAR_ADDR, GT_LCL_ADDR, GT_LEA) || - (addr->IsIconHandle(GTF_ICON_TLS_HDL, GTF_ICON_TLS_HDL_CCTOR))); + assert(addr->OperIs(GT_CLS_VAR_ADDR, GT_LCL_ADDR, GT_LEA) || (addr->IsIconHandle(GTF_ICON_TLS_HDL))); int offset = 0; DWORD lsl = 0; @@ -13928,7 +13927,7 @@ void emitter::emitInsLoadStoreOp(instruction ins, emitAttr attr, regNumber dataR emitIns_R_S(ins, attr, dataReg, lclNum, offset); } } - else if (addr->IsIconHandle(GTF_ICON_TLS_HDL, GTF_ICON_TLS_HDL_CCTOR)) + else if (addr->IsIconHandle(GTF_ICON_TLS_HDL)) { // On Arm64, TEB is in r18, so load from the r18 as base. emitIns_R_R_I(ins, attr, dataReg, REG_R18, addr->AsIntCon()->IconValue()); diff --git a/src/coreclr/jit/fgdiagnostic.cpp b/src/coreclr/jit/fgdiagnostic.cpp index ae06bfbdc90181..d1b1adb3b0d376 100644 --- a/src/coreclr/jit/fgdiagnostic.cpp +++ b/src/coreclr/jit/fgdiagnostic.cpp @@ -3106,11 +3106,9 @@ void Compiler::fgDebugCheckFlags(GenTree* tree) GenTreeFlags handleKind = op1->GetIconHandleFlag(); // Some of these aren't handles to invariant data... - if ((handleKind == GTF_ICON_STATIC_HDL) || // Pointer to a mutable class Static variable - (handleKind == GTF_ICON_STATIC_HDL_CCTOR) || // Pointer to a mutable class Static variable - (handleKind == GTF_ICON_BBC_PTR) || // Pointer to a mutable basic block count value - (handleKind == GTF_ICON_GLOBAL_PTR) || // Pointer to mutable data from the VM state - (handleKind == GTF_ICON_GLOBAL_PTR_CCTOR)) // Pointer to mutable data from the VM state + if ((handleKind == GTF_ICON_STATIC_HDL) || // Pointer to a mutable class Static variable + (handleKind == GTF_ICON_BBC_PTR) || // Pointer to a mutable basic block count value + (handleKind == GTF_ICON_GLOBAL_PTR)) // Pointer to mutable data from the VM state { // For statics, we expect the GTF_GLOB_REF to be set. However, we currently // fail to set it in a number of situations, and so this check is disabled. diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 20007e1a87b3e2..454bdccb53c91e 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -7203,11 +7203,9 @@ GenTree* Compiler::gtNewIndOfIconHandleNode(var_types indType, size_t addr, GenT if (isInvariant) { - assert(iconFlags != GTF_ICON_STATIC_HDL); // Pointer to a mutable class Static variable - assert(iconFlags != GTF_ICON_STATIC_HDL_CCTOR); // Pointer to a mutable class Static variable - assert(iconFlags != GTF_ICON_BBC_PTR); // Pointer to a mutable basic block count value - assert(iconFlags != GTF_ICON_GLOBAL_PTR); // Pointer to mutable data from the VM state - assert(iconFlags != GTF_ICON_GLOBAL_PTR_CCTOR); // Pointer to mutable data from the VM state + assert(iconFlags != GTF_ICON_STATIC_HDL); // Pointer to a mutable class Static variable + assert(iconFlags != GTF_ICON_BBC_PTR); // Pointer to a mutable basic block count value + assert(iconFlags != GTF_ICON_GLOBAL_PTR); // Pointer to mutable data from the VM state // This indirection also is invariant. indirFlags |= GTF_IND_INVARIANT; @@ -10764,9 +10762,19 @@ void Compiler::gtDispNode(GenTree* tree, IndentStack* indentStack, _In_ _In_opt_ case GT_CNS_INT: if (tree->IsIconHandle()) { - printf("H"); - --msgLength; - break; + if ((tree->gtFlags & GTF_ICON_INITCLASS) != 0) + { + printf("I"); // Static Field handle with INITCLASS requirement + --msgLength; + break; + } + else + { + // Some other handle + printf("H"); + --msgLength; + break; + } } goto DASH; @@ -11448,7 +11456,6 @@ void Compiler::gtDispConst(GenTree* tree) printf(" field"); break; case GTF_ICON_STATIC_HDL: - case GTF_ICON_STATIC_HDL_CCTOR: printf(" static"); break; case GTF_ICON_OBJ_HDL: @@ -11456,11 +11463,9 @@ void Compiler::gtDispConst(GenTree* tree) unreached(); // These cases are handled above break; case GTF_ICON_CONST_PTR: - case GTF_ICON_CONST_PTR_CCTOR: printf(" const ptr"); break; case GTF_ICON_GLOBAL_PTR: - case GTF_ICON_GLOBAL_PTR_CCTOR: printf(" global ptr"); break; case GTF_ICON_VARG_HDL: @@ -11473,7 +11478,6 @@ void Compiler::gtDispConst(GenTree* tree) printf(" token"); break; case GTF_ICON_TLS_HDL: - case GTF_ICON_TLS_HDL_CCTOR: printf(" tls"); break; case GTF_ICON_FTN_ADDR: @@ -17778,7 +17782,7 @@ bool GenTree::IsFieldAddr(Compiler* comp, GenTree** pBaseAddr, FieldSeq** pFldSe return false; } } - else if (IsIconHandle(GTF_ICON_STATIC_HDL, GTF_ICON_STATIC_HDL_CCTOR)) + else if (IsIconHandle(GTF_ICON_STATIC_HDL)) { fldSeq = AsIntCon()->gtFieldSeq; offset = AsIntCon()->IconValue(); @@ -18156,8 +18160,7 @@ CORINFO_CLASS_HANDLE Compiler::gtGetClassHandle(GenTree* tree, bool* pIsExact, b } } } - else if (base->IsIconHandle(GTF_ICON_CONST_PTR, GTF_ICON_CONST_PTR_CCTOR, GTF_ICON_STATIC_HDL, - GTF_ICON_STATIC_HDL_CCTOR)) + 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; diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 055c92fce24641..9c7cc95947617a 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -518,34 +518,36 @@ enum GenTreeFlags : unsigned int GTF_ARR_ADDR_NONNULL = 0x80000000, // GT_ARR_ADDR -- this array's address is not null - GTF_ICON_HDL_MASK = 0xFF000000, // Bits used by handle types below - GTF_ICON_SCOPE_HDL = 0x01000000, // GT_CNS_INT -- constant is a scope handle - GTF_ICON_CLASS_HDL = 0x02000000, // GT_CNS_INT -- constant is a class handle - GTF_ICON_METHOD_HDL = 0x03000000, // GT_CNS_INT -- constant is a method handle - GTF_ICON_FIELD_HDL = 0x04000000, // GT_CNS_INT -- constant is a field handle - GTF_ICON_STATIC_HDL = 0x05000000, // GT_CNS_INT -- constant is a handle to static data - GTF_ICON_STATIC_HDL_CCTOR = 0x06000000, // GT_CNS_INT -- constant is a handle to cctor dependent static data - GTF_ICON_STR_HDL = 0x07000000, // GT_CNS_INT -- constant is a pinned handle pointing to a string object - GTF_ICON_OBJ_HDL = 0x08000000, // GT_CNS_INT -- constant is an object handle (e.g. frozen string or Type object) - GTF_ICON_CONST_PTR = 0x09000000, // GT_CNS_INT -- constant is a pointer to immutable data, (e.g. IAT_PPVALUE) - GTF_ICON_CONST_PTR_CCTOR = 0x0A000000, // GT_CNS_INT -- constant is a pointer to cctor dependent immutable data - GTF_ICON_GLOBAL_PTR = 0x0B000000, // GT_CNS_INT -- constant is a pointer to mutable data (e.g. from the VM state) - GTF_ICON_GLOBAL_PTR_CCTOR = 0x18000000, // GT_CNS_INT -- constant is a pointer to cctor dependent mutable data (e.g. from the VM state) - GTF_ICON_VARG_HDL = 0x0C000000, // GT_CNS_INT -- constant is a var arg cookie handle - GTF_ICON_PINVKI_HDL = 0x0D000000, // GT_CNS_INT -- constant is a pinvoke calli handle - GTF_ICON_TOKEN_HDL = 0x0E000000, // GT_CNS_INT -- constant is a token handle (other than class, method or field) - GTF_ICON_TLS_HDL = 0x0F000000, // GT_CNS_INT -- constant is a TLS ref with offset - GTF_ICON_TLS_HDL_CCTOR = 0x10000000, // GT_CNS_INT -- constant is a cctor dependent TLS ref with offset - GTF_ICON_FTN_ADDR = 0x11000000, // GT_CNS_INT -- constant is a function address - GTF_ICON_CIDMID_HDL = 0x12000000, // GT_CNS_INT -- constant is a class ID or a module ID - GTF_ICON_BBC_PTR = 0x13000000, // GT_CNS_INT -- constant is a basic block count pointer - GTF_ICON_STATIC_BOX_PTR = 0x14000000, // GT_CNS_INT -- constant is an address of the box for a STATIC_IN_HEAP field - GTF_ICON_STATIC_BOX_PTR_CCTOR = 0x15000000, // GT_CNS_INT -- constant is an address of the box for a cctor dependent STATIC_IN_HEAP field - GTF_ICON_FIELD_SEQ = 0x16000000, // GT_CNS_INT -- constant is a FieldSeq* (used only as VNHandle) - GTF_ICON_STATIC_ADDR_PTR = 0x17000000, // GT_CNS_INT -- constant is a pointer to a static base address + GTF_ICON_HDL_MASK = 0xFF000000, // Bits used by handle types below + GTF_ICON_SCOPE_HDL = 0x01000000, // GT_CNS_INT -- constant is a scope handle + GTF_ICON_CLASS_HDL = 0x02000000, // GT_CNS_INT -- constant is a class handle + GTF_ICON_METHOD_HDL = 0x03000000, // GT_CNS_INT -- constant is a method handle + GTF_ICON_FIELD_HDL = 0x04000000, // GT_CNS_INT -- constant is a field handle + GTF_ICON_STATIC_HDL = 0x05000000, // GT_CNS_INT -- constant is a handle to static data + GTF_ICON_STR_HDL = 0x06000000, // GT_CNS_INT -- constant is a pinned handle pointing to a string object + GTF_ICON_OBJ_HDL = 0x12000000, // GT_CNS_INT -- constant is an object handle (e.g. frozen string or Type object) + GTF_ICON_CONST_PTR = 0x07000000, // GT_CNS_INT -- constant is a pointer to immutable data, (e.g. IAT_PPVALUE) + GTF_ICON_GLOBAL_PTR = 0x08000000, // GT_CNS_INT -- constant is a pointer to mutable data (e.g. from the VM state) + GTF_ICON_VARG_HDL = 0x09000000, // GT_CNS_INT -- constant is a var arg cookie handle + GTF_ICON_PINVKI_HDL = 0x0A000000, // GT_CNS_INT -- constant is a pinvoke calli handle + GTF_ICON_TOKEN_HDL = 0x0B000000, // GT_CNS_INT -- constant is a token handle (other than class, method or field) + GTF_ICON_TLS_HDL = 0x0C000000, // GT_CNS_INT -- constant is a TLS ref with offset + GTF_ICON_FTN_ADDR = 0x0D000000, // GT_CNS_INT -- constant is a function address + GTF_ICON_CIDMID_HDL = 0x0E000000, // GT_CNS_INT -- constant is a class ID or a module ID + GTF_ICON_BBC_PTR = 0x0F000000, // GT_CNS_INT -- constant is a basic block count pointer + GTF_ICON_STATIC_BOX_PTR = 0x10000000, // GT_CNS_INT -- constant is an address of the box for a STATIC_IN_HEAP field + GTF_ICON_FIELD_SEQ = 0x11000000, // <--------> -- constant is a FieldSeq* (used only as VNHandle) + GTF_ICON_STATIC_ADDR_PTR = 0x13000000, // GT_CNS_INT -- constant is a pointer to a static base address // GTF_ICON_REUSE_REG_VAL = 0x00800000 // GT_CNS_INT -- GTF_REUSE_REG_VAL, defined above GTF_ICON_SIMD_COUNT = 0x00200000, // GT_CNS_INT -- constant is Vector.Count + GTF_ICON_INITCLASS = 0x00100000, // GT_CNS_INT -- Constant is used to access a static that requires preceding + // class/static init helper. In some cases, the constant is + // the address of the static field itself, and in other cases + // there's an extra layer of indirection and it is the address + // of the cell that the runtime will fill in with the address + // of the static field; in both of those cases, the constant + // is what gets flagged. GTF_OVERFLOW = 0x10000000, // Supported for: GT_ADD, GT_SUB, GT_MUL and GT_CAST. // Requires an overflow check. Use gtOverflow(Ex)() to check this flag. diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 52d7d70000a1bc..ad16d27762867d 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -4331,22 +4331,25 @@ GenTree* Compiler::impImportStaticFieldAccess(CORINFO_RESOLVED_TOKEN* pResolvedT assert(pFieldInfo->fieldLookup.accessType == IAT_VALUE); size_t fldAddr = reinterpret_cast(pFieldInfo->fieldLookup.addr); GenTreeFlags handleKind; - bool cctorDependent = (pFieldInfo->fieldFlags & CORINFO_FLG_FIELD_INITCLASS) != 0; if (isBoxedStatic) { - handleKind = cctorDependent ? GTF_ICON_STATIC_BOX_PTR_CCTOR : GTF_ICON_STATIC_BOX_PTR; + handleKind = GTF_ICON_STATIC_BOX_PTR; } else if (isStaticReadOnlyInitedRef) { - handleKind = cctorDependent ? GTF_ICON_CONST_PTR_CCTOR : GTF_ICON_CONST_PTR; + handleKind = GTF_ICON_CONST_PTR; } else { - handleKind = cctorDependent ? GTF_ICON_STATIC_HDL_CCTOR : GTF_ICON_STATIC_HDL; + handleKind = GTF_ICON_STATIC_HDL; } isHoistable = true; op1 = gtNewIconHandleNode(fldAddr, handleKind, innerFldSeq); INDEBUG(op1->AsIntCon()->gtTargetHandle = reinterpret_cast(pResolvedToken->hField)); + if (pFieldInfo->fieldFlags & CORINFO_FLG_FIELD_INITCLASS) + { + op1->gtFlags |= GTF_ICON_INITCLASS; + } break; } } diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index f85b2e09dec429..ffe382e1952ee4 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -5345,9 +5345,14 @@ GenTree* Compiler::fgMorphExpandTlsFieldAddr(GenTree* tree) #define WIN32_TLS_SLOTS (0x2C) // Offset from fs:[0] where the pointer to the slots resides // Mark this ICON as a TLS_HDL, codegen will use FS:[cns] - GenTreeFlags handleType = ((tree->gtFlags & GTF_FLD_INITCLASS) != 0) ? GTF_ICON_TLS_HDL_CCTOR : GTF_ICON_TLS_HDL; - GenTree* tlsRef = gtNewIconHandleNode(WIN32_TLS_SLOTS, handleType); - tree->gtFlags &= ~GTF_FLD_INITCLASS; + GenTree* tlsRef = gtNewIconHandleNode(WIN32_TLS_SLOTS, GTF_ICON_TLS_HDL); + + // Translate GTF_FLD_INITCLASS to GTF_ICON_INITCLASS + if ((tree->gtFlags & GTF_FLD_INITCLASS) != 0) + { + tree->gtFlags &= ~GTF_FLD_INITCLASS; + tlsRef->gtFlags |= GTF_ICON_INITCLASS; + } tlsRef = gtNewIndir(TYP_I_IMPL, tlsRef, GTF_IND_NONFAULTING | GTF_IND_INVARIANT); diff --git a/src/coreclr/jit/optcse.cpp b/src/coreclr/jit/optcse.cpp index b598a795f4889c..9f8e2ee023491a 100644 --- a/src/coreclr/jit/optcse.cpp +++ b/src/coreclr/jit/optcse.cpp @@ -766,8 +766,8 @@ bool Compiler::optValnumCSE_Locate() { if (!enableConstCSE && // Unconditionally allow these constant handles to be CSE'd - !tree->IsIconHandle(GTF_ICON_STATIC_HDL, GTF_ICON_STATIC_HDL_CCTOR, GTF_ICON_CLASS_HDL, - GTF_ICON_STR_HDL, GTF_ICON_OBJ_HDL)) + !tree->IsIconHandle(GTF_ICON_STATIC_HDL) && !tree->IsIconHandle(GTF_ICON_CLASS_HDL) && + !tree->IsIconHandle(GTF_ICON_STR_HDL) && !tree->IsIconHandle(GTF_ICON_OBJ_HDL)) { continue; } diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp index fd94757f8a88b1..e154aba816f469 100644 --- a/src/coreclr/jit/optimizer.cpp +++ b/src/coreclr/jit/optimizer.cpp @@ -7543,9 +7543,7 @@ void Compiler::optHoistLoopBlocks(unsigned loopNum, ArrayStack* blo // isCctorDependent and isAddressWhoseDereferenceWouldBeCctorDependent, but we don't for // simplicity/throughput; the constant itself would be considered non-hoistable anyway, since // optIsCSEcandidate returns false for constants. - bool treeIsCctorDependent = tree->OperIs(GT_CNS_INT) && - tree->IsIconHandle(GTF_ICON_STATIC_HDL_CCTOR, GTF_ICON_CONST_PTR_CCTOR, - GTF_ICON_TLS_HDL_CCTOR, GTF_ICON_STATIC_BOX_PTR_CCTOR); + bool treeIsCctorDependent = tree->OperIs(GT_CNS_INT) && ((tree->gtFlags & GTF_ICON_INITCLASS) != 0); bool treeIsInvariant = true; bool treeHasHoistableChildren = false; int childCount; diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 3091755714611a..3d258bdb8263ea 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -5896,16 +5896,10 @@ GenTreeFlags ValueNumStore::GetFoldedArithOpResultHandleFlags(ValueNum vn) case GTF_ICON_STATIC_BOX_PTR: case GTF_ICON_STATIC_ADDR_PTR: return GTF_ICON_CONST_PTR; - case GTF_ICON_CONST_PTR_CCTOR: - case GTF_ICON_TLS_HDL_CCTOR: - return GTF_ICON_CONST_PTR_CCTOR; case GTF_ICON_STATIC_HDL: case GTF_ICON_GLOBAL_PTR: case GTF_ICON_BBC_PTR: return GTF_ICON_GLOBAL_PTR; - case GTF_ICON_STATIC_HDL_CCTOR: - case GTF_ICON_GLOBAL_PTR_CCTOR: - return GTF_ICON_GLOBAL_PTR_CCTOR; default: assert(!"Unexpected handle type"); return flags; From c6a1fcafc4aa1c47e45639808716a92e93843872 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Wed, 26 Apr 2023 14:43:09 +0200 Subject: [PATCH 3/5] Teach constant propagation to propagate GTF_ICON_INITCLASS This flag represents that dereferences off of the handle are dependent on a cctor and cannot be hoisted out unless all cctors also are. It must be propagated together with the constant for correctness. --- src/coreclr/jit/assertionprop.cpp | 67 +++++++++++++++---------------- src/coreclr/jit/compiler.h | 36 +++++++++++------ src/coreclr/jit/valuenum.cpp | 39 ++++++++++-------- src/coreclr/jit/valuenum.h | 2 +- 4 files changed, 79 insertions(+), 65 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index d7369b3adcce26..1d089b1cf4b5ea 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -699,12 +699,12 @@ void Compiler::optPrintAssertion(AssertionDsc* curAssertion, AssertionIndex asse if (curAssertion->op1.kind == O1K_EXACT_TYPE) { printf("Exact Type MT(%08X)", dspPtr(curAssertion->op2.u1.iconVal)); - assert(curAssertion->op2.HasIconFlag()); + assert(curAssertion->op2.IsHandle()); } else if (curAssertion->op1.kind == O1K_SUBTYPE) { printf("MT(%08X)", dspPtr(curAssertion->op2.u1.iconVal)); - assert(curAssertion->op2.HasIconFlag()); + assert(curAssertion->op2.IsHandle()); } else if ((curAssertion->op1.kind == O1K_BOUND_OPER_BND) || (curAssertion->op1.kind == O1K_BOUND_LOOP_BND) || @@ -741,7 +741,7 @@ void Compiler::optPrintAssertion(AssertionDsc* curAssertion, AssertionIndex asse } else { - if (curAssertion->op2.HasIconFlag()) + if (curAssertion->op2.IsHandle()) { printf("[%08p]", dspPtr(curAssertion->op2.u1.iconVal)); } @@ -1058,7 +1058,7 @@ AssertionIndex Compiler::optCreateAssertion(GenTree* op1, assertion.op2.kind = O2K_CONST_INT; assertion.op2.vn = ValueNumStore::VNForNull(); assertion.op2.u1.iconVal = 0; - assertion.op2.SetIconFlag(GTF_EMPTY); + assertion.op2.SetIconFlags(GTF_EMPTY); } // // Are we making an assertion about a local variable? @@ -1112,7 +1112,7 @@ AssertionIndex Compiler::optCreateAssertion(GenTree* op1, assertion.op1.lcl.ssaNum = op1->AsLclVarCommon()->GetSsaNum(); assertion.op2.u1.iconVal = op2->AsIntCon()->gtIconVal; assertion.op2.vn = optConservativeNormalVN(op2); - assertion.op2.SetIconFlag(op2->GetIconHandleFlag()); + assertion.op2.SetIconFlags(op2->gtFlags & AssertionDsc::PropagatedIconHandleFlags); // // Ok everything has been set and the assertion looks good @@ -1201,7 +1201,8 @@ AssertionIndex Compiler::optCreateAssertion(GenTree* op1, #endif // TARGET_ARM assertion.op2.u1.iconVal = iconVal; - assertion.op2.SetIconFlag(op2->GetIconHandleFlag(), op2->AsIntCon()->gtFieldSeq); + assertion.op2.SetIconFlags(op2->gtFlags & AssertionDsc::PropagatedIconHandleFlags, + op2->AsIntCon()->gtFieldSeq); } else if (op2->gtOper == GT_CNS_LNG) { @@ -1363,10 +1364,7 @@ AssertionIndex Compiler::optCreateAssertion(GenTree* op1, assertion.op2.kind = O2K_IND_CNS_INT; assertion.op2.u1.iconVal = cnsValue; assertion.op2.vn = optConservativeNormalVN(op2->AsOp()->gtOp1); - - /* iconFlags should only contain bits in GTF_ICON_HDL_MASK */ - assert((iconFlags & ~GTF_ICON_HDL_MASK) == 0); - assertion.op2.SetIconFlag(iconFlags); + assertion.op2.SetIconFlags(iconFlags); } // JIT case else if (optIsTreeKnownIntValue(!optLocalAssertionProp, op2, &cnsValue, &iconFlags)) @@ -1375,10 +1373,7 @@ AssertionIndex Compiler::optCreateAssertion(GenTree* op1, assertion.op2.kind = O2K_CONST_INT; assertion.op2.u1.iconVal = cnsValue; assertion.op2.vn = optConservativeNormalVN(op2); - - /* iconFlags should only contain bits in GTF_ICON_HDL_MASK */ - assert((iconFlags & ~GTF_ICON_HDL_MASK) == 0); - assertion.op2.SetIconFlag(iconFlags); + assertion.op2.SetIconFlags(iconFlags); } else { @@ -1447,7 +1442,7 @@ bool Compiler::optIsTreeKnownIntValue(bool vnBased, GenTree* tree, ssize_t* pCon if (tree->OperGet() == GT_CNS_INT) { *pConstant = tree->AsIntCon()->IconValue(); - *pFlags = tree->GetIconHandleFlag(); + *pFlags = tree->gtFlags & AssertionDsc::PropagatedIconHandleFlags; return true; } #ifdef TARGET_64BIT @@ -1456,7 +1451,7 @@ bool Compiler::optIsTreeKnownIntValue(bool vnBased, GenTree* tree, ssize_t* pCon else if (tree->OperGet() == GT_CNS_LNG) { *pConstant = tree->AsLngCon()->gtLconVal; - *pFlags = tree->GetIconHandleFlag(); + *pFlags = tree->gtFlags & AssertionDsc::PropagatedIconHandleFlags; return true; } #endif @@ -1676,12 +1671,11 @@ void Compiler::optDebugCheckAssertion(AssertionDsc* assertion) case O2K_IND_CNS_INT: case O2K_CONST_INT: { - // The only flags that can be set are those in the GTF_ICON_HDL_MASK. switch (assertion->op1.kind) { case O1K_EXACT_TYPE: case O1K_SUBTYPE: - assert(assertion->op2.HasIconFlag()); + assert(assertion->op2.IsHandle()); break; case O1K_LCLVAR: assert((lvaGetDesc(assertion->op1.lcl.lclNum)->lvType != TYP_REF) || @@ -1700,7 +1694,7 @@ void Compiler::optDebugCheckAssertion(AssertionDsc* assertion) { // All handles should be represented by O2K_CONST_INT, // so no handle bits should be set here. - assert(!assertion->op2.HasIconFlag()); + assert(!assertion->op2.IsHandle()); } break; @@ -1912,7 +1906,7 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree) dsc.op2.kind = O2K_CONST_INT; dsc.op2.vn = vnStore->VNZeroForType(op2->TypeGet()); dsc.op2.u1.iconVal = 0; - dsc.op2.SetIconFlag(GTF_EMPTY); + dsc.op2.SetIconFlags(GTF_EMPTY); AssertionIndex index = optAddAssertion(&dsc); optCreateComplementaryAssertion(index, nullptr, nullptr); return index; @@ -1929,7 +1923,7 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree) dsc.op2.kind = O2K_CONST_INT; dsc.op2.vn = vnStore->VNZeroForType(op2->TypeGet()); dsc.op2.u1.iconVal = 0; - dsc.op2.SetIconFlag(GTF_EMPTY); + dsc.op2.SetIconFlags(GTF_EMPTY); AssertionIndex index = optAddAssertion(&dsc); optCreateComplementaryAssertion(index, nullptr, nullptr); return index; @@ -1946,7 +1940,7 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree) dsc.op2.kind = O2K_CONST_INT; dsc.op2.vn = vnStore->VNZeroForType(op2->TypeGet()); dsc.op2.u1.iconVal = 0; - dsc.op2.SetIconFlag(GTF_EMPTY); + dsc.op2.SetIconFlags(GTF_EMPTY); AssertionIndex index = optAddAssertion(&dsc); optCreateComplementaryAssertion(index, nullptr, nullptr); return index; @@ -1963,7 +1957,7 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree) dsc.op2.kind = O2K_CONST_INT; dsc.op2.vn = vnStore->VNZeroForType(TYP_INT); dsc.op2.u1.iconVal = 0; - dsc.op2.SetIconFlag(GTF_EMPTY); + dsc.op2.SetIconFlags(GTF_EMPTY); AssertionIndex index = optAddAssertion(&dsc); optCreateComplementaryAssertion(index, nullptr, nullptr); return index; @@ -2006,7 +2000,7 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree) dsc.op2.kind = O2K_CONST_INT; dsc.op2.vn = vnStore->VNZeroForType(op2->TypeGet()); dsc.op2.u1.iconVal = 0; - dsc.op2.SetIconFlag(GTF_EMPTY); + dsc.op2.SetIconFlags(GTF_EMPTY); AssertionIndex index = optAddAssertion(&dsc); optCreateComplementaryAssertion(index, nullptr, nullptr); return index; @@ -2023,7 +2017,7 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree) dsc.op2.kind = O2K_CONST_INT; dsc.op2.vn = vnStore->VNZeroForType(TYP_INT); dsc.op2.u1.iconVal = 0; - dsc.op2.SetIconFlag(GTF_EMPTY); + dsc.op2.SetIconFlags(GTF_EMPTY); AssertionIndex index = optAddAssertion(&dsc); optCreateComplementaryAssertion(index, nullptr, nullptr); return index; @@ -2037,7 +2031,7 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree) dsc.op2.kind = O2K_CONST_INT; dsc.op2.vn = vnStore->VNZeroForType(TYP_INT); dsc.op2.u1.iconVal = 0; - dsc.op2.SetIconFlag(GTF_EMPTY); + dsc.op2.SetIconFlags(GTF_EMPTY); AssertionIndex index = optAddAssertion(&dsc); optCreateComplementaryAssertion(index, nullptr, nullptr); return index; @@ -2133,7 +2127,7 @@ AssertionInfo Compiler::optAssertionGenJtrue(GenTree* tree) dsc.op2.vn = vnStore->VNConservativeNormalValue(op2->gtVNPair); dsc.op2.kind = O2K_CONST_INT; dsc.op2.u1.iconVal = 0; - dsc.op2.SetIconFlag(GTF_EMPTY); + dsc.op2.SetIconFlags(GTF_EMPTY); // when con is not zero, create an assertion on the arr.Length == con edge // when con is zero, create an assertion on the arr.Length != 0 edge @@ -2952,11 +2946,13 @@ GenTree* Compiler::optConstantAssertionProp(AssertionDsc* curAssertion, break; case O2K_CONST_INT: - + { + GenTreeFlags iconFlags = curAssertion->op2.GetIconFlags(); + GenTreeFlags iconHandle = iconFlags & GTF_ICON_HDL_MASK; // Don't propagate handles if we need to report relocs. - if (opts.compReloc && curAssertion->op2.HasIconFlag() && curAssertion->op2.u1.iconVal != 0) + if (opts.compReloc && (iconHandle != 0) && curAssertion->op2.u1.iconVal != 0) { - if (curAssertion->op2.GetIconFlag() == GTF_ICON_STATIC_HDL) + if (iconHandle == GTF_ICON_STATIC_HDL) { propagateType = true; } @@ -2976,11 +2972,10 @@ GenTree* Compiler::optConstantAssertionProp(AssertionDsc* curAssertion, assert(!varTypeIsSmall(tree) || (curAssertion->op2.u1.iconVal == optCastConstantSmall(curAssertion->op2.u1.iconVal, tree->TypeGet()))); - if (curAssertion->op2.HasIconFlag()) + if (iconHandle != 0) { // Here we have to allocate a new 'large' node to replace the old one - newTree = gtNewIconHandleNode(curAssertion->op2.u1.iconVal, curAssertion->op2.GetIconFlag(), - curAssertion->op2.u1.fieldSeq); + newTree = gtNewIconHandleNode(curAssertion->op2.u1.iconVal, iconFlags, curAssertion->op2.u1.fieldSeq); // Make sure we don't retype const gc handles to TYP_I_IMPL // Although, it's possible for e.g. GTF_ICON_STATIC_HDL @@ -3003,8 +2998,10 @@ GenTree* Compiler::optConstantAssertionProp(AssertionDsc* curAssertion, { assert(varTypeIsIntegralOrI(tree)); newTree->BashToConst(curAssertion->op2.u1.iconVal, genActualType(tree)); + newTree->gtFlags |= iconFlags; } break; + } default: return nullptr; @@ -3884,7 +3881,7 @@ GenTree* Compiler::optAssertionPropGlobal_RelOp(ASSERT_VALARG_TP assertions, Gen if (vnStore->IsVNHandle(vnCns)) { - op1->gtFlags |= (vnStore->GetHandleFlags(vnCns) & GTF_ICON_HDL_MASK); + op1->gtFlags |= (vnStore->GetHandleFlags(vnCns) & AssertionDsc::PropagatedIconHandleFlags); } } else if (op1->TypeGet() == TYP_LONG) @@ -3893,7 +3890,7 @@ GenTree* Compiler::optAssertionPropGlobal_RelOp(ASSERT_VALARG_TP assertions, Gen if (vnStore->IsVNHandle(vnCns)) { - op1->gtFlags |= (vnStore->GetHandleFlags(vnCns) & GTF_ICON_HDL_MASK); + op1->gtFlags |= (vnStore->GetHandleFlags(vnCns) & AssertionDsc::PropagatedIconHandleFlags); } } else if (op1->TypeGet() == TYP_DOUBLE) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index d05f9d1ef246f5..01f2114e943a96 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -7318,30 +7318,41 @@ class Compiler IntegralRange u2; }; - bool HasIconFlag() - { - assert(m_encodedIconFlags <= 0xFF); - return m_encodedIconFlags != 0; - } - GenTreeFlags GetIconFlag() + GenTreeFlags GetIconFlags() { // number of trailing zeros in GTF_ICON_HDL_MASK const uint16_t iconMaskTzc = 24; static_assert_no_msg((0xFF000000 == GTF_ICON_HDL_MASK) && (GTF_ICON_HDL_MASK >> iconMaskTzc) == 0xFF); - GenTreeFlags flags = (GenTreeFlags)(m_encodedIconFlags << iconMaskTzc); - assert((flags & ~GTF_ICON_HDL_MASK) == 0); + GenTreeFlags flags = (GenTreeFlags)((m_encodedIconFlags & 0xFF) << iconMaskTzc); + if ((m_encodedIconFlags & 0x100) != 0) + { + flags |= GTF_ICON_INITCLASS; + } + return flags; } - void SetIconFlag(GenTreeFlags flags, FieldSeq* fieldSeq = nullptr) + void SetIconFlags(GenTreeFlags flags, FieldSeq* fieldSeq = nullptr) { const uint16_t iconMaskTzc = 24; - assert((flags & ~GTF_ICON_HDL_MASK) == 0); + assert((flags & ~PropagatedIconHandleFlags) == 0); m_encodedIconFlags = flags >> iconMaskTzc; - u1.fieldSeq = fieldSeq; + if ((flags & GTF_ICON_INITCLASS) != 0) + { + m_encodedIconFlags |= 0x100; + } + + u1.fieldSeq = fieldSeq; + } + + bool IsHandle() + { + return (GetIconFlags() & GTF_ICON_HDL_MASK) != 0; } } op2; + static const GenTreeFlags PropagatedIconHandleFlags = GTF_ICON_HDL_MASK | GTF_ICON_INITCLASS; + bool IsCheckedBoundArithBound() { return ((assertionKind == OAK_EQUAL || assertionKind == OAK_NOT_EQUAL) && op1.kind == O1K_BOUND_OPER_BND); @@ -7448,7 +7459,8 @@ class Compiler { case O2K_IND_CNS_INT: case O2K_CONST_INT: - return ((op2.u1.iconVal == that->op2.u1.iconVal) && (op2.GetIconFlag() == that->op2.GetIconFlag())); + return ((op2.u1.iconVal == that->op2.u1.iconVal) && + (op2.GetIconFlags() == that->op2.GetIconFlags())); case O2K_CONST_LONG: return (op2.lconVal == that->op2.lconVal); diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 3d258bdb8263ea..902f4bb1381214 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -2016,7 +2016,7 @@ void ValueNumStore::GetCastOperFromVN(ValueNum vn, var_types* pCastToType, bool* ValueNum ValueNumStore::VNForHandle(ssize_t cnsVal, GenTreeFlags handleFlags) { - assert((handleFlags & ~GTF_ICON_HDL_MASK) == 0); + assert((handleFlags & ~(GTF_ICON_HDL_MASK | GTF_ICON_INITCLASS)) == 0); ValueNum res; VNHandle handle; @@ -2406,7 +2406,7 @@ ValueNum ValueNumStore::VNForFunc(var_types typ, VNFunc func, ValueNum arg0VN) if ((resultVN == NoVN) && GetVNFunc(addressVN, &funcApp) && (funcApp.m_func == VNF_InvariantNonNullLoad)) { ValueNum fieldSeqVN = VNNormalValue(funcApp.m_args[0]); - if (IsVNHandle(fieldSeqVN) && (GetHandleFlags(fieldSeqVN) == GTF_ICON_FIELD_SEQ)) + if (IsVNHandle(fieldSeqVN) && ((GetHandleFlags(fieldSeqVN) & GTF_ICON_HDL_MASK) == GTF_ICON_FIELD_SEQ)) { FieldSeq* fieldSeq = FieldSeqVNToFieldSeq(fieldSeqVN); if (fieldSeq != nullptr) @@ -4151,8 +4151,8 @@ ValueNum ValueNumStore::VNEvalFoldTypeCompare(var_types type, VNFunc func, Value return NoVN; } - assert(GetHandleFlags(handle0) == GTF_ICON_CLASS_HDL); - assert(GetHandleFlags(handle1) == GTF_ICON_CLASS_HDL); + assert((GetHandleFlags(handle0) & GTF_ICON_HDL_MASK) == GTF_ICON_CLASS_HDL); + assert((GetHandleFlags(handle1) & GTF_ICON_HDL_MASK) == GTF_ICON_CLASS_HDL); const ssize_t handleVal0 = ConstantValue(handle0); const ssize_t handleVal1 = ConstantValue(handle1); @@ -5296,7 +5296,7 @@ ValueNum ValueNumStore::VNForFieldSeq(FieldSeq* fieldSeq) // FieldSeq* ValueNumStore::FieldSeqVNToFieldSeq(ValueNum vn) { - assert(IsVNHandle(vn) && (GetHandleFlags(vn) == GTF_ICON_FIELD_SEQ)); + assert(IsVNHandle(vn) && ((GetHandleFlags(vn) & GTF_ICON_HDL_MASK) == GTF_ICON_FIELD_SEQ)); return reinterpret_cast(ConstantValue(vn)); } @@ -5875,10 +5875,10 @@ GenTreeFlags ValueNumStore::GetHandleFlags(ValueNum vn) GenTreeFlags ValueNumStore::GetFoldedArithOpResultHandleFlags(ValueNum vn) { - GenTreeFlags flags = GetHandleFlags(vn); - assert((flags & GTF_ICON_HDL_MASK) == flags); + GenTreeFlags flags = GetHandleFlags(vn); + GenTreeFlags resultFlags = flags & ~GTF_ICON_HDL_MASK; - switch (flags) + switch (flags & GTF_ICON_HDL_MASK) { case GTF_ICON_SCOPE_HDL: case GTF_ICON_CLASS_HDL: @@ -5895,15 +5895,19 @@ GenTreeFlags ValueNumStore::GetFoldedArithOpResultHandleFlags(ValueNum vn) case GTF_ICON_TLS_HDL: case GTF_ICON_STATIC_BOX_PTR: case GTF_ICON_STATIC_ADDR_PTR: - return GTF_ICON_CONST_PTR; + resultFlags |= GTF_ICON_CONST_PTR; + break; case GTF_ICON_STATIC_HDL: case GTF_ICON_GLOBAL_PTR: case GTF_ICON_BBC_PTR: - return GTF_ICON_GLOBAL_PTR; + resultFlags |= GTF_ICON_GLOBAL_PTR; + break; default: assert(!"Unexpected handle type"); return flags; } + + return resultFlags; } bool ValueNumStore::IsVNHandle(ValueNum vn) @@ -5919,7 +5923,7 @@ bool ValueNumStore::IsVNHandle(ValueNum vn) bool ValueNumStore::IsVNObjHandle(ValueNum vn) { - return IsVNHandle(vn) && GetHandleFlags(vn) == GTF_ICON_OBJ_HDL; + return IsVNHandle(vn) && ((GetHandleFlags(vn) & GTF_ICON_HDL_MASK) == GTF_ICON_OBJ_HDL); } //------------------------------------------------------------------------ @@ -8379,7 +8383,7 @@ void ValueNumStore::vnDump(Compiler* comp, ValueNum vn, bool isPtr) { printf("NoVN"); } - else if (IsVNHandle(vn) && (GetHandleFlags(vn) == GTF_ICON_FIELD_SEQ)) + else if (IsVNHandle(vn) && ((GetHandleFlags(vn) & GTF_ICON_HDL_MASK) == GTF_ICON_FIELD_SEQ)) { comp->gtDispFieldSeq(FieldSeqVNToFieldSeq(vn), 0); printf(" "); @@ -9940,9 +9944,9 @@ void Compiler::fgValueNumberTreeConst(GenTree* tree) if (tree->IsIconHandle()) { const GenTreeIntCon* cns = tree->AsIntCon(); - const GenTreeFlags handleFlags = tree->GetIconHandleFlag(); + const GenTreeFlags handleFlags = tree->gtFlags & AssertionDsc::PropagatedIconHandleFlags; tree->gtVNPair.SetBoth(vnStore->VNForHandle(cns->IconValue(), handleFlags)); - if (handleFlags == GTF_ICON_CLASS_HDL) + if ((handleFlags & GTF_ICON_HDL_MASK) == GTF_ICON_CLASS_HDL) { vnStore->AddToEmbeddedHandleMap(cns->IconValue(), cns->gtCompileTimeHandle); } @@ -10034,8 +10038,8 @@ void Compiler::fgValueNumberTreeConst(GenTree* tree) else { assert(doesMethodHaveFrozenObjects()); - tree->gtVNPair.SetBoth( - vnStore->VNForHandle(ssize_t(tree->AsIntConCommon()->IconValue()), tree->GetIconHandleFlag())); + tree->gtVNPair.SetBoth(vnStore->VNForHandle(ssize_t(tree->AsIntConCommon()->IconValue()), + tree->gtFlags & AssertionDsc::PropagatedIconHandleFlags)); } break; @@ -10051,7 +10055,8 @@ void Compiler::fgValueNumberTreeConst(GenTree* tree) if (tree->IsIconHandle()) { tree->gtVNPair.SetBoth( - vnStore->VNForHandle(ssize_t(tree->AsIntConCommon()->IconValue()), tree->GetIconHandleFlag())); + vnStore->VNForHandle(ssize_t(tree->AsIntConCommon()->IconValue()), + tree->gtFlags & AssertionDsc::PropagatedIconHandleFlags)); } else { diff --git a/src/coreclr/jit/valuenum.h b/src/coreclr/jit/valuenum.h index c4f9a2918c0a35..65dacbddd0ee3c 100644 --- a/src/coreclr/jit/valuenum.h +++ b/src/coreclr/jit/valuenum.h @@ -985,7 +985,7 @@ class ValueNumStore // If "vn" is checked bound arith, then populate the "info" fields for cmpOp, cmpOper. void GetCompareCheckedBoundArithInfo(ValueNum vn, CompareCheckedBoundArithInfo* info); - // Returns the flags on the current handle. GTF_ICON_SCOPE_HDL for example. + // Returns the flags on the current handle. GTF_ICON_SCOPE_HDL for example. Can also have GTF_ICON_INITCLASS. GenTreeFlags GetHandleFlags(ValueNum vn); // Returns true iff the VN represents a handle constant. From e16e6260405adbdff4c3a0d2a2205b79006dd80c Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Wed, 26 Apr 2023 15:15:15 +0200 Subject: [PATCH 4/5] Revert "Teach constant propagation to propagate GTF_ICON_INITCLASS" This reverts commit c6a1fcafc4aa1c47e45639808716a92e93843872. --- src/coreclr/jit/assertionprop.cpp | 67 ++++++++++++++++--------------- src/coreclr/jit/compiler.h | 36 ++++++----------- src/coreclr/jit/valuenum.cpp | 39 ++++++++---------- src/coreclr/jit/valuenum.h | 2 +- 4 files changed, 65 insertions(+), 79 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 1d089b1cf4b5ea..d7369b3adcce26 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -699,12 +699,12 @@ void Compiler::optPrintAssertion(AssertionDsc* curAssertion, AssertionIndex asse if (curAssertion->op1.kind == O1K_EXACT_TYPE) { printf("Exact Type MT(%08X)", dspPtr(curAssertion->op2.u1.iconVal)); - assert(curAssertion->op2.IsHandle()); + assert(curAssertion->op2.HasIconFlag()); } else if (curAssertion->op1.kind == O1K_SUBTYPE) { printf("MT(%08X)", dspPtr(curAssertion->op2.u1.iconVal)); - assert(curAssertion->op2.IsHandle()); + assert(curAssertion->op2.HasIconFlag()); } else if ((curAssertion->op1.kind == O1K_BOUND_OPER_BND) || (curAssertion->op1.kind == O1K_BOUND_LOOP_BND) || @@ -741,7 +741,7 @@ void Compiler::optPrintAssertion(AssertionDsc* curAssertion, AssertionIndex asse } else { - if (curAssertion->op2.IsHandle()) + if (curAssertion->op2.HasIconFlag()) { printf("[%08p]", dspPtr(curAssertion->op2.u1.iconVal)); } @@ -1058,7 +1058,7 @@ AssertionIndex Compiler::optCreateAssertion(GenTree* op1, assertion.op2.kind = O2K_CONST_INT; assertion.op2.vn = ValueNumStore::VNForNull(); assertion.op2.u1.iconVal = 0; - assertion.op2.SetIconFlags(GTF_EMPTY); + assertion.op2.SetIconFlag(GTF_EMPTY); } // // Are we making an assertion about a local variable? @@ -1112,7 +1112,7 @@ AssertionIndex Compiler::optCreateAssertion(GenTree* op1, assertion.op1.lcl.ssaNum = op1->AsLclVarCommon()->GetSsaNum(); assertion.op2.u1.iconVal = op2->AsIntCon()->gtIconVal; assertion.op2.vn = optConservativeNormalVN(op2); - assertion.op2.SetIconFlags(op2->gtFlags & AssertionDsc::PropagatedIconHandleFlags); + assertion.op2.SetIconFlag(op2->GetIconHandleFlag()); // // Ok everything has been set and the assertion looks good @@ -1201,8 +1201,7 @@ AssertionIndex Compiler::optCreateAssertion(GenTree* op1, #endif // TARGET_ARM assertion.op2.u1.iconVal = iconVal; - assertion.op2.SetIconFlags(op2->gtFlags & AssertionDsc::PropagatedIconHandleFlags, - op2->AsIntCon()->gtFieldSeq); + assertion.op2.SetIconFlag(op2->GetIconHandleFlag(), op2->AsIntCon()->gtFieldSeq); } else if (op2->gtOper == GT_CNS_LNG) { @@ -1364,7 +1363,10 @@ AssertionIndex Compiler::optCreateAssertion(GenTree* op1, assertion.op2.kind = O2K_IND_CNS_INT; assertion.op2.u1.iconVal = cnsValue; assertion.op2.vn = optConservativeNormalVN(op2->AsOp()->gtOp1); - assertion.op2.SetIconFlags(iconFlags); + + /* iconFlags should only contain bits in GTF_ICON_HDL_MASK */ + assert((iconFlags & ~GTF_ICON_HDL_MASK) == 0); + assertion.op2.SetIconFlag(iconFlags); } // JIT case else if (optIsTreeKnownIntValue(!optLocalAssertionProp, op2, &cnsValue, &iconFlags)) @@ -1373,7 +1375,10 @@ AssertionIndex Compiler::optCreateAssertion(GenTree* op1, assertion.op2.kind = O2K_CONST_INT; assertion.op2.u1.iconVal = cnsValue; assertion.op2.vn = optConservativeNormalVN(op2); - assertion.op2.SetIconFlags(iconFlags); + + /* iconFlags should only contain bits in GTF_ICON_HDL_MASK */ + assert((iconFlags & ~GTF_ICON_HDL_MASK) == 0); + assertion.op2.SetIconFlag(iconFlags); } else { @@ -1442,7 +1447,7 @@ bool Compiler::optIsTreeKnownIntValue(bool vnBased, GenTree* tree, ssize_t* pCon if (tree->OperGet() == GT_CNS_INT) { *pConstant = tree->AsIntCon()->IconValue(); - *pFlags = tree->gtFlags & AssertionDsc::PropagatedIconHandleFlags; + *pFlags = tree->GetIconHandleFlag(); return true; } #ifdef TARGET_64BIT @@ -1451,7 +1456,7 @@ bool Compiler::optIsTreeKnownIntValue(bool vnBased, GenTree* tree, ssize_t* pCon else if (tree->OperGet() == GT_CNS_LNG) { *pConstant = tree->AsLngCon()->gtLconVal; - *pFlags = tree->gtFlags & AssertionDsc::PropagatedIconHandleFlags; + *pFlags = tree->GetIconHandleFlag(); return true; } #endif @@ -1671,11 +1676,12 @@ void Compiler::optDebugCheckAssertion(AssertionDsc* assertion) case O2K_IND_CNS_INT: case O2K_CONST_INT: { + // The only flags that can be set are those in the GTF_ICON_HDL_MASK. switch (assertion->op1.kind) { case O1K_EXACT_TYPE: case O1K_SUBTYPE: - assert(assertion->op2.IsHandle()); + assert(assertion->op2.HasIconFlag()); break; case O1K_LCLVAR: assert((lvaGetDesc(assertion->op1.lcl.lclNum)->lvType != TYP_REF) || @@ -1694,7 +1700,7 @@ void Compiler::optDebugCheckAssertion(AssertionDsc* assertion) { // All handles should be represented by O2K_CONST_INT, // so no handle bits should be set here. - assert(!assertion->op2.IsHandle()); + assert(!assertion->op2.HasIconFlag()); } break; @@ -1906,7 +1912,7 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree) dsc.op2.kind = O2K_CONST_INT; dsc.op2.vn = vnStore->VNZeroForType(op2->TypeGet()); dsc.op2.u1.iconVal = 0; - dsc.op2.SetIconFlags(GTF_EMPTY); + dsc.op2.SetIconFlag(GTF_EMPTY); AssertionIndex index = optAddAssertion(&dsc); optCreateComplementaryAssertion(index, nullptr, nullptr); return index; @@ -1923,7 +1929,7 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree) dsc.op2.kind = O2K_CONST_INT; dsc.op2.vn = vnStore->VNZeroForType(op2->TypeGet()); dsc.op2.u1.iconVal = 0; - dsc.op2.SetIconFlags(GTF_EMPTY); + dsc.op2.SetIconFlag(GTF_EMPTY); AssertionIndex index = optAddAssertion(&dsc); optCreateComplementaryAssertion(index, nullptr, nullptr); return index; @@ -1940,7 +1946,7 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree) dsc.op2.kind = O2K_CONST_INT; dsc.op2.vn = vnStore->VNZeroForType(op2->TypeGet()); dsc.op2.u1.iconVal = 0; - dsc.op2.SetIconFlags(GTF_EMPTY); + dsc.op2.SetIconFlag(GTF_EMPTY); AssertionIndex index = optAddAssertion(&dsc); optCreateComplementaryAssertion(index, nullptr, nullptr); return index; @@ -1957,7 +1963,7 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree) dsc.op2.kind = O2K_CONST_INT; dsc.op2.vn = vnStore->VNZeroForType(TYP_INT); dsc.op2.u1.iconVal = 0; - dsc.op2.SetIconFlags(GTF_EMPTY); + dsc.op2.SetIconFlag(GTF_EMPTY); AssertionIndex index = optAddAssertion(&dsc); optCreateComplementaryAssertion(index, nullptr, nullptr); return index; @@ -2000,7 +2006,7 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree) dsc.op2.kind = O2K_CONST_INT; dsc.op2.vn = vnStore->VNZeroForType(op2->TypeGet()); dsc.op2.u1.iconVal = 0; - dsc.op2.SetIconFlags(GTF_EMPTY); + dsc.op2.SetIconFlag(GTF_EMPTY); AssertionIndex index = optAddAssertion(&dsc); optCreateComplementaryAssertion(index, nullptr, nullptr); return index; @@ -2017,7 +2023,7 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree) dsc.op2.kind = O2K_CONST_INT; dsc.op2.vn = vnStore->VNZeroForType(TYP_INT); dsc.op2.u1.iconVal = 0; - dsc.op2.SetIconFlags(GTF_EMPTY); + dsc.op2.SetIconFlag(GTF_EMPTY); AssertionIndex index = optAddAssertion(&dsc); optCreateComplementaryAssertion(index, nullptr, nullptr); return index; @@ -2031,7 +2037,7 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree) dsc.op2.kind = O2K_CONST_INT; dsc.op2.vn = vnStore->VNZeroForType(TYP_INT); dsc.op2.u1.iconVal = 0; - dsc.op2.SetIconFlags(GTF_EMPTY); + dsc.op2.SetIconFlag(GTF_EMPTY); AssertionIndex index = optAddAssertion(&dsc); optCreateComplementaryAssertion(index, nullptr, nullptr); return index; @@ -2127,7 +2133,7 @@ AssertionInfo Compiler::optAssertionGenJtrue(GenTree* tree) dsc.op2.vn = vnStore->VNConservativeNormalValue(op2->gtVNPair); dsc.op2.kind = O2K_CONST_INT; dsc.op2.u1.iconVal = 0; - dsc.op2.SetIconFlags(GTF_EMPTY); + dsc.op2.SetIconFlag(GTF_EMPTY); // when con is not zero, create an assertion on the arr.Length == con edge // when con is zero, create an assertion on the arr.Length != 0 edge @@ -2946,13 +2952,11 @@ GenTree* Compiler::optConstantAssertionProp(AssertionDsc* curAssertion, break; case O2K_CONST_INT: - { - GenTreeFlags iconFlags = curAssertion->op2.GetIconFlags(); - GenTreeFlags iconHandle = iconFlags & GTF_ICON_HDL_MASK; + // Don't propagate handles if we need to report relocs. - if (opts.compReloc && (iconHandle != 0) && curAssertion->op2.u1.iconVal != 0) + if (opts.compReloc && curAssertion->op2.HasIconFlag() && curAssertion->op2.u1.iconVal != 0) { - if (iconHandle == GTF_ICON_STATIC_HDL) + if (curAssertion->op2.GetIconFlag() == GTF_ICON_STATIC_HDL) { propagateType = true; } @@ -2972,10 +2976,11 @@ GenTree* Compiler::optConstantAssertionProp(AssertionDsc* curAssertion, assert(!varTypeIsSmall(tree) || (curAssertion->op2.u1.iconVal == optCastConstantSmall(curAssertion->op2.u1.iconVal, tree->TypeGet()))); - if (iconHandle != 0) + if (curAssertion->op2.HasIconFlag()) { // Here we have to allocate a new 'large' node to replace the old one - newTree = gtNewIconHandleNode(curAssertion->op2.u1.iconVal, iconFlags, curAssertion->op2.u1.fieldSeq); + newTree = gtNewIconHandleNode(curAssertion->op2.u1.iconVal, curAssertion->op2.GetIconFlag(), + curAssertion->op2.u1.fieldSeq); // Make sure we don't retype const gc handles to TYP_I_IMPL // Although, it's possible for e.g. GTF_ICON_STATIC_HDL @@ -2998,10 +3003,8 @@ GenTree* Compiler::optConstantAssertionProp(AssertionDsc* curAssertion, { assert(varTypeIsIntegralOrI(tree)); newTree->BashToConst(curAssertion->op2.u1.iconVal, genActualType(tree)); - newTree->gtFlags |= iconFlags; } break; - } default: return nullptr; @@ -3881,7 +3884,7 @@ GenTree* Compiler::optAssertionPropGlobal_RelOp(ASSERT_VALARG_TP assertions, Gen if (vnStore->IsVNHandle(vnCns)) { - op1->gtFlags |= (vnStore->GetHandleFlags(vnCns) & AssertionDsc::PropagatedIconHandleFlags); + op1->gtFlags |= (vnStore->GetHandleFlags(vnCns) & GTF_ICON_HDL_MASK); } } else if (op1->TypeGet() == TYP_LONG) @@ -3890,7 +3893,7 @@ GenTree* Compiler::optAssertionPropGlobal_RelOp(ASSERT_VALARG_TP assertions, Gen if (vnStore->IsVNHandle(vnCns)) { - op1->gtFlags |= (vnStore->GetHandleFlags(vnCns) & AssertionDsc::PropagatedIconHandleFlags); + op1->gtFlags |= (vnStore->GetHandleFlags(vnCns) & GTF_ICON_HDL_MASK); } } else if (op1->TypeGet() == TYP_DOUBLE) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 01f2114e943a96..d05f9d1ef246f5 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -7318,41 +7318,30 @@ class Compiler IntegralRange u2; }; - GenTreeFlags GetIconFlags() + bool HasIconFlag() + { + assert(m_encodedIconFlags <= 0xFF); + return m_encodedIconFlags != 0; + } + GenTreeFlags GetIconFlag() { // number of trailing zeros in GTF_ICON_HDL_MASK const uint16_t iconMaskTzc = 24; static_assert_no_msg((0xFF000000 == GTF_ICON_HDL_MASK) && (GTF_ICON_HDL_MASK >> iconMaskTzc) == 0xFF); - GenTreeFlags flags = (GenTreeFlags)((m_encodedIconFlags & 0xFF) << iconMaskTzc); - if ((m_encodedIconFlags & 0x100) != 0) - { - flags |= GTF_ICON_INITCLASS; - } - + GenTreeFlags flags = (GenTreeFlags)(m_encodedIconFlags << iconMaskTzc); + assert((flags & ~GTF_ICON_HDL_MASK) == 0); return flags; } - void SetIconFlags(GenTreeFlags flags, FieldSeq* fieldSeq = nullptr) + void SetIconFlag(GenTreeFlags flags, FieldSeq* fieldSeq = nullptr) { const uint16_t iconMaskTzc = 24; - assert((flags & ~PropagatedIconHandleFlags) == 0); + assert((flags & ~GTF_ICON_HDL_MASK) == 0); m_encodedIconFlags = flags >> iconMaskTzc; - if ((flags & GTF_ICON_INITCLASS) != 0) - { - m_encodedIconFlags |= 0x100; - } - - u1.fieldSeq = fieldSeq; - } - - bool IsHandle() - { - return (GetIconFlags() & GTF_ICON_HDL_MASK) != 0; + u1.fieldSeq = fieldSeq; } } op2; - static const GenTreeFlags PropagatedIconHandleFlags = GTF_ICON_HDL_MASK | GTF_ICON_INITCLASS; - bool IsCheckedBoundArithBound() { return ((assertionKind == OAK_EQUAL || assertionKind == OAK_NOT_EQUAL) && op1.kind == O1K_BOUND_OPER_BND); @@ -7459,8 +7448,7 @@ class Compiler { case O2K_IND_CNS_INT: case O2K_CONST_INT: - return ((op2.u1.iconVal == that->op2.u1.iconVal) && - (op2.GetIconFlags() == that->op2.GetIconFlags())); + return ((op2.u1.iconVal == that->op2.u1.iconVal) && (op2.GetIconFlag() == that->op2.GetIconFlag())); case O2K_CONST_LONG: return (op2.lconVal == that->op2.lconVal); diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 902f4bb1381214..3d258bdb8263ea 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -2016,7 +2016,7 @@ void ValueNumStore::GetCastOperFromVN(ValueNum vn, var_types* pCastToType, bool* ValueNum ValueNumStore::VNForHandle(ssize_t cnsVal, GenTreeFlags handleFlags) { - assert((handleFlags & ~(GTF_ICON_HDL_MASK | GTF_ICON_INITCLASS)) == 0); + assert((handleFlags & ~GTF_ICON_HDL_MASK) == 0); ValueNum res; VNHandle handle; @@ -2406,7 +2406,7 @@ ValueNum ValueNumStore::VNForFunc(var_types typ, VNFunc func, ValueNum arg0VN) if ((resultVN == NoVN) && GetVNFunc(addressVN, &funcApp) && (funcApp.m_func == VNF_InvariantNonNullLoad)) { ValueNum fieldSeqVN = VNNormalValue(funcApp.m_args[0]); - if (IsVNHandle(fieldSeqVN) && ((GetHandleFlags(fieldSeqVN) & GTF_ICON_HDL_MASK) == GTF_ICON_FIELD_SEQ)) + if (IsVNHandle(fieldSeqVN) && (GetHandleFlags(fieldSeqVN) == GTF_ICON_FIELD_SEQ)) { FieldSeq* fieldSeq = FieldSeqVNToFieldSeq(fieldSeqVN); if (fieldSeq != nullptr) @@ -4151,8 +4151,8 @@ ValueNum ValueNumStore::VNEvalFoldTypeCompare(var_types type, VNFunc func, Value return NoVN; } - assert((GetHandleFlags(handle0) & GTF_ICON_HDL_MASK) == GTF_ICON_CLASS_HDL); - assert((GetHandleFlags(handle1) & GTF_ICON_HDL_MASK) == GTF_ICON_CLASS_HDL); + assert(GetHandleFlags(handle0) == GTF_ICON_CLASS_HDL); + assert(GetHandleFlags(handle1) == GTF_ICON_CLASS_HDL); const ssize_t handleVal0 = ConstantValue(handle0); const ssize_t handleVal1 = ConstantValue(handle1); @@ -5296,7 +5296,7 @@ ValueNum ValueNumStore::VNForFieldSeq(FieldSeq* fieldSeq) // FieldSeq* ValueNumStore::FieldSeqVNToFieldSeq(ValueNum vn) { - assert(IsVNHandle(vn) && ((GetHandleFlags(vn) & GTF_ICON_HDL_MASK) == GTF_ICON_FIELD_SEQ)); + assert(IsVNHandle(vn) && (GetHandleFlags(vn) == GTF_ICON_FIELD_SEQ)); return reinterpret_cast(ConstantValue(vn)); } @@ -5875,10 +5875,10 @@ GenTreeFlags ValueNumStore::GetHandleFlags(ValueNum vn) GenTreeFlags ValueNumStore::GetFoldedArithOpResultHandleFlags(ValueNum vn) { - GenTreeFlags flags = GetHandleFlags(vn); - GenTreeFlags resultFlags = flags & ~GTF_ICON_HDL_MASK; + GenTreeFlags flags = GetHandleFlags(vn); + assert((flags & GTF_ICON_HDL_MASK) == flags); - switch (flags & GTF_ICON_HDL_MASK) + switch (flags) { case GTF_ICON_SCOPE_HDL: case GTF_ICON_CLASS_HDL: @@ -5895,19 +5895,15 @@ GenTreeFlags ValueNumStore::GetFoldedArithOpResultHandleFlags(ValueNum vn) case GTF_ICON_TLS_HDL: case GTF_ICON_STATIC_BOX_PTR: case GTF_ICON_STATIC_ADDR_PTR: - resultFlags |= GTF_ICON_CONST_PTR; - break; + return GTF_ICON_CONST_PTR; case GTF_ICON_STATIC_HDL: case GTF_ICON_GLOBAL_PTR: case GTF_ICON_BBC_PTR: - resultFlags |= GTF_ICON_GLOBAL_PTR; - break; + return GTF_ICON_GLOBAL_PTR; default: assert(!"Unexpected handle type"); return flags; } - - return resultFlags; } bool ValueNumStore::IsVNHandle(ValueNum vn) @@ -5923,7 +5919,7 @@ bool ValueNumStore::IsVNHandle(ValueNum vn) bool ValueNumStore::IsVNObjHandle(ValueNum vn) { - return IsVNHandle(vn) && ((GetHandleFlags(vn) & GTF_ICON_HDL_MASK) == GTF_ICON_OBJ_HDL); + return IsVNHandle(vn) && GetHandleFlags(vn) == GTF_ICON_OBJ_HDL; } //------------------------------------------------------------------------ @@ -8383,7 +8379,7 @@ void ValueNumStore::vnDump(Compiler* comp, ValueNum vn, bool isPtr) { printf("NoVN"); } - else if (IsVNHandle(vn) && ((GetHandleFlags(vn) & GTF_ICON_HDL_MASK) == GTF_ICON_FIELD_SEQ)) + else if (IsVNHandle(vn) && (GetHandleFlags(vn) == GTF_ICON_FIELD_SEQ)) { comp->gtDispFieldSeq(FieldSeqVNToFieldSeq(vn), 0); printf(" "); @@ -9944,9 +9940,9 @@ void Compiler::fgValueNumberTreeConst(GenTree* tree) if (tree->IsIconHandle()) { const GenTreeIntCon* cns = tree->AsIntCon(); - const GenTreeFlags handleFlags = tree->gtFlags & AssertionDsc::PropagatedIconHandleFlags; + const GenTreeFlags handleFlags = tree->GetIconHandleFlag(); tree->gtVNPair.SetBoth(vnStore->VNForHandle(cns->IconValue(), handleFlags)); - if ((handleFlags & GTF_ICON_HDL_MASK) == GTF_ICON_CLASS_HDL) + if (handleFlags == GTF_ICON_CLASS_HDL) { vnStore->AddToEmbeddedHandleMap(cns->IconValue(), cns->gtCompileTimeHandle); } @@ -10038,8 +10034,8 @@ void Compiler::fgValueNumberTreeConst(GenTree* tree) else { assert(doesMethodHaveFrozenObjects()); - tree->gtVNPair.SetBoth(vnStore->VNForHandle(ssize_t(tree->AsIntConCommon()->IconValue()), - tree->gtFlags & AssertionDsc::PropagatedIconHandleFlags)); + tree->gtVNPair.SetBoth( + vnStore->VNForHandle(ssize_t(tree->AsIntConCommon()->IconValue()), tree->GetIconHandleFlag())); } break; @@ -10055,8 +10051,7 @@ void Compiler::fgValueNumberTreeConst(GenTree* tree) if (tree->IsIconHandle()) { tree->gtVNPair.SetBoth( - vnStore->VNForHandle(ssize_t(tree->AsIntConCommon()->IconValue()), - tree->gtFlags & AssertionDsc::PropagatedIconHandleFlags)); + vnStore->VNForHandle(ssize_t(tree->AsIntConCommon()->IconValue()), tree->GetIconHandleFlag())); } else { diff --git a/src/coreclr/jit/valuenum.h b/src/coreclr/jit/valuenum.h index 65dacbddd0ee3c..c4f9a2918c0a35 100644 --- a/src/coreclr/jit/valuenum.h +++ b/src/coreclr/jit/valuenum.h @@ -985,7 +985,7 @@ class ValueNumStore // If "vn" is checked bound arith, then populate the "info" fields for cmpOp, cmpOper. void GetCompareCheckedBoundArithInfo(ValueNum vn, CompareCheckedBoundArithInfo* info); - // Returns the flags on the current handle. GTF_ICON_SCOPE_HDL for example. Can also have GTF_ICON_INITCLASS. + // Returns the flags on the current handle. GTF_ICON_SCOPE_HDL for example. GenTreeFlags GetHandleFlags(ValueNum vn); // Returns true iff the VN represents a handle constant. From 6566ee186f5585b206e2a1476212b2a27caad3a2 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Wed, 26 Apr 2023 15:18:04 +0200 Subject: [PATCH 5/5] JIT: Change GTF_ICON_INITCLASS -> GTF_IND_INITCLASS The JIT has a flag GTF_ICON_INITCLASS that represents that accesses off that address are cctor dependent. Hoisting uses this to avoid hoisting cctor dependent indirections unless all cctors are also hoisted. However, local constant prop/VN-based constant prop do not handle this flag, so we could run into cases where addresses with GTF_ICON_INITCLASS were propagated and then subsequently hoisted incorrectly. This change moves the flag to an OperIsIndir() flag instead of being a flag on the constant. After some digging, I found that the original reason the flag was not an indir flag was simply that there were no more indir flags available, but we do have available flags today. This fix is much simpler than the alternatives which would be to teach VN/local copy prop to propagate this GTF_ICON_INITCLASS flag. Also remove GTF_FLD_INITCLASS which is never set. --- src/coreclr/jit/compiler.cpp | 4 ++++ src/coreclr/jit/gentree.cpp | 22 +++++++++------------- src/coreclr/jit/gentree.h | 11 ++--------- src/coreclr/jit/importer.cpp | 18 ++++++++++-------- src/coreclr/jit/morph.cpp | 7 ------- src/coreclr/jit/optimizer.cpp | 8 +------- 6 files changed, 26 insertions(+), 44 deletions(-) diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index 6cf88c87b0a237..53357d0ecb7404 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -9766,6 +9766,10 @@ void cTreeFlags(Compiler* comp, GenTree* tree) { chars += printf("[IND_NONNULL]"); } + if (tree->gtFlags & GTF_IND_INITCLASS) + { + chars += printf("[IND_INITCLASS]"); + } break; case GT_MUL: diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 454bdccb53c91e..cc83afe48a430a 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -10594,6 +10594,12 @@ void Compiler::gtDispNode(GenTree* tree, IndentStack* indentStack, _In_ _In_opt_ --msgLength; break; } + if (tree->gtFlags & GTF_IND_INITCLASS) + { + printf("I"); + --msgLength; + break; + } if (tree->gtFlags & GTF_IND_INVARIANT) { printf("#"); @@ -10762,19 +10768,9 @@ void Compiler::gtDispNode(GenTree* tree, IndentStack* indentStack, _In_ _In_opt_ case GT_CNS_INT: if (tree->IsIconHandle()) { - if ((tree->gtFlags & GTF_ICON_INITCLASS) != 0) - { - printf("I"); // Static Field handle with INITCLASS requirement - --msgLength; - break; - } - else - { - // Some other handle - printf("H"); - --msgLength; - break; - } + printf("H"); + --msgLength; + break; } goto DASH; diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 9c7cc95947617a..187f8f1999e111 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -476,7 +476,6 @@ enum GenTreeFlags : unsigned int GTF_FLD_TLS = 0x80000000, // GT_FIELD_ADDR -- field address is a Windows x86 TLS reference GTF_FLD_VOLATILE = 0x40000000, // GT_FIELD -- same as GTF_IND_VOLATILE - GTF_FLD_INITCLASS = 0x20000000, // GT_FIELD/GT_FIELD_ADDR -- field access requires preceding class/static init helper GTF_FLD_TGT_HEAP = 0x10000000, // GT_FIELD -- same as GTF_IND_TGT_HEAP GTF_INX_RNGCHK = 0x80000000, // GT_INDEX_ADDR -- this array address should be range-checked @@ -494,9 +493,10 @@ enum GenTreeFlags : unsigned int GTF_IND_UNALIGNED = 0x02000000, // OperIsIndir() -- the load or store is unaligned (we assume worst case alignment of 1 byte) GTF_IND_INVARIANT = 0x01000000, // GT_IND -- the target is invariant (a prejit indirection) GTF_IND_NONNULL = 0x00400000, // GT_IND -- the indirection never returns null (zero) + GTF_IND_INITCLASS = 0x00200000, // OperIsIndir() -- the indirection requires preceding static cctor GTF_IND_FLAGS = GTF_IND_VOLATILE | GTF_IND_NONFAULTING | GTF_IND_UNALIGNED | GTF_IND_INVARIANT | - GTF_IND_NONNULL | GTF_IND_TGT_NOT_HEAP | GTF_IND_TGT_HEAP, + GTF_IND_NONNULL | GTF_IND_TGT_NOT_HEAP | GTF_IND_TGT_HEAP | GTF_IND_INITCLASS, GTF_ADDRMODE_NO_CSE = 0x80000000, // GT_ADD/GT_MUL/GT_LSH -- Do not CSE this node only, forms complex // addressing mode @@ -541,13 +541,6 @@ enum GenTreeFlags : unsigned int // GTF_ICON_REUSE_REG_VAL = 0x00800000 // GT_CNS_INT -- GTF_REUSE_REG_VAL, defined above GTF_ICON_SIMD_COUNT = 0x00200000, // GT_CNS_INT -- constant is Vector.Count - GTF_ICON_INITCLASS = 0x00100000, // GT_CNS_INT -- Constant is used to access a static that requires preceding - // class/static init helper. In some cases, the constant is - // the address of the static field itself, and in other cases - // there's an extra layer of indirection and it is the address - // of the cell that the runtime will fill in with the address - // of the static field; in both of those cases, the constant - // is what gets flagged. GTF_OVERFLOW = 0x10000000, // Supported for: GT_ADD, GT_SUB, GT_MUL and GT_CAST. // Requires an overflow check. Use gtOverflow(Ex)() to check this flag. diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index ad16d27762867d..e7d03fbc9c2369 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -4167,10 +4167,11 @@ GenTree* Compiler::impImportStaticFieldAccess(CORINFO_RESOLVED_TOKEN* pResolvedT outerFldSeq = nullptr; } - bool isHoistable = false; - bool isStaticReadOnlyInitedRef = false; - unsigned typeIndex = 0; - GenTree* op1; + bool isHoistable = false; + bool isStaticReadOnlyInitedRef = false; + GenTreeFlags indirFlags = GTF_EMPTY; + unsigned typeIndex = 0; + GenTree* op1; switch (pFieldInfo->fieldAccessor) { case CORINFO_FIELD_STATIC_GENERICS_STATIC_HELPER: @@ -4348,7 +4349,7 @@ GenTree* Compiler::impImportStaticFieldAccess(CORINFO_RESOLVED_TOKEN* pResolvedT INDEBUG(op1->AsIntCon()->gtTargetHandle = reinterpret_cast(pResolvedToken->hField)); if (pFieldInfo->fieldFlags & CORINFO_FLG_FIELD_INITCLASS) { - op1->gtFlags |= GTF_ICON_INITCLASS; + indirFlags |= GTF_IND_INITCLASS; } break; } @@ -4356,8 +4357,9 @@ GenTree* Compiler::impImportStaticFieldAccess(CORINFO_RESOLVED_TOKEN* pResolvedT if (isBoxedStatic) { - op1 = gtNewIndir(TYP_REF, op1, GTF_IND_INVARIANT | GTF_IND_NONFAULTING | GTF_IND_NONNULL); - op1 = gtNewOperNode(GT_ADD, TYP_BYREF, op1, gtNewIconNode(TARGET_POINTER_SIZE, outerFldSeq)); + op1 = gtNewIndir(TYP_REF, op1, GTF_IND_INVARIANT | GTF_IND_NONFAULTING | GTF_IND_NONNULL | indirFlags); + indirFlags = GTF_EMPTY; + op1 = gtNewOperNode(GT_ADD, TYP_BYREF, op1, gtNewIconNode(TARGET_POINTER_SIZE, outerFldSeq)); } if (!(access & CORINFO_ACCESS_ADDRESS)) @@ -4366,7 +4368,7 @@ GenTree* Compiler::impImportStaticFieldAccess(CORINFO_RESOLVED_TOKEN* pResolvedT lclTyp = TypeHandleToVarType(pFieldInfo->fieldType, pFieldInfo->structType, &layout); // TODO-CQ: mark the indirections non-faulting. - op1 = (lclTyp == TYP_STRUCT) ? gtNewBlkIndir(layout, op1) : gtNewIndir(lclTyp, op1); + op1 = (lclTyp == TYP_STRUCT) ? gtNewBlkIndir(layout, op1, indirFlags) : gtNewIndir(lclTyp, op1, indirFlags); if (isStaticReadOnlyInitedRef) { diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index ffe382e1952ee4..8cd03b6e0d69c9 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -5347,13 +5347,6 @@ GenTree* Compiler::fgMorphExpandTlsFieldAddr(GenTree* tree) // Mark this ICON as a TLS_HDL, codegen will use FS:[cns] GenTree* tlsRef = gtNewIconHandleNode(WIN32_TLS_SLOTS, GTF_ICON_TLS_HDL); - // Translate GTF_FLD_INITCLASS to GTF_ICON_INITCLASS - if ((tree->gtFlags & GTF_FLD_INITCLASS) != 0) - { - tree->gtFlags &= ~GTF_FLD_INITCLASS; - tlsRef->gtFlags |= GTF_ICON_INITCLASS; - } - tlsRef = gtNewIndir(TYP_I_IMPL, tlsRef, GTF_IND_NONFAULTING | GTF_IND_INVARIANT); if (dllRef != nullptr) diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp index e154aba816f469..1ee468aef9eec9 100644 --- a/src/coreclr/jit/optimizer.cpp +++ b/src/coreclr/jit/optimizer.cpp @@ -7537,13 +7537,7 @@ void Compiler::optHoistLoopBlocks(unsigned loopNum, ArrayStack* blo return fgWalkResult::WALK_CONTINUE; } - // Initclass CLS_VARs and IconHandles are the base cases of cctor dependent trees. - // In the IconHandle case, it's of course the dereference, rather than the constant itself, that is - // truly dependent on the cctor. So a more precise approach would be to separately propagate - // isCctorDependent and isAddressWhoseDereferenceWouldBeCctorDependent, but we don't for - // simplicity/throughput; the constant itself would be considered non-hoistable anyway, since - // optIsCSEcandidate returns false for constants. - bool treeIsCctorDependent = tree->OperIs(GT_CNS_INT) && ((tree->gtFlags & GTF_ICON_INITCLASS) != 0); + bool treeIsCctorDependent = tree->OperIsIndir() && ((tree->gtFlags & GTF_IND_INITCLASS) != 0); bool treeIsInvariant = true; bool treeHasHoistableChildren = false; int childCount;