From c00fc65a25c71215fc89de14337e74d44d6e4c90 Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Thu, 2 Dec 2021 23:08:35 +0300 Subject: [PATCH] Mark static boxes as invariant This will be needed to make the upcoming changes to how we parse the field sequences for boxed statics zero-diff. It also happens to be a CQ improvement on its own. --- src/coreclr/jit/compiler.cpp | 5 +++++ src/coreclr/jit/gentree.cpp | 3 +++ src/coreclr/jit/gentree.h | 43 ++++++++++++++++++------------------ src/coreclr/jit/importer.cpp | 19 +++++++++++----- src/coreclr/jit/morph.cpp | 36 +++++++++++++++++++++--------- src/coreclr/jit/valuenum.cpp | 14 +++--------- 6 files changed, 73 insertions(+), 47 deletions(-) diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index 62496ab0c4c44..86f569f6ca301 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -9382,6 +9382,11 @@ void cTreeFlags(Compiler* comp, GenTree* tree) chars += printf("[ICON_BBC_PTR]"); break; + case GTF_ICON_STATIC_BOX_PTR: + + chars += printf("[GTF_ICON_STATIC_BOX_PTR]"); + break; + case GTF_ICON_FIELD_OFF: chars += printf("[ICON_FIELD_OFF]"); diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 09e596a774b6e..4568783c785d0 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -10454,6 +10454,9 @@ void Compiler::gtDispConst(GenTree* tree) case GTF_ICON_BBC_PTR: printf(" bbc"); break; + case GTF_ICON_STATIC_BOX_PTR: + printf(" static box ptr"); + break; default: printf(" UNKNOWN"); break; diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 553ed29837c83..7d8eaf7b6f350 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -542,27 +542,28 @@ enum GenTreeFlags : unsigned int GTF_BOX_VALUE = 0x80000000, // GT_BOX -- "box" is on a value type - GTF_ICON_HDL_MASK = 0xF0000000, // Bits used by handle types below - GTF_ICON_SCOPE_HDL = 0x10000000, // GT_CNS_INT -- constant is a scope handle - GTF_ICON_CLASS_HDL = 0x20000000, // GT_CNS_INT -- constant is a class handle - GTF_ICON_METHOD_HDL = 0x30000000, // GT_CNS_INT -- constant is a method handle - GTF_ICON_FIELD_HDL = 0x40000000, // GT_CNS_INT -- constant is a field handle - GTF_ICON_STATIC_HDL = 0x50000000, // GT_CNS_INT -- constant is a handle to static data - GTF_ICON_STR_HDL = 0x60000000, // GT_CNS_INT -- constant is a string handle - GTF_ICON_CONST_PTR = 0x70000000, // GT_CNS_INT -- constant is a pointer to immutable data, (e.g. IAT_PPVALUE) - GTF_ICON_GLOBAL_PTR = 0x80000000, // GT_CNS_INT -- constant is a pointer to mutable data (e.g. from the VM state) - GTF_ICON_VARG_HDL = 0x90000000, // GT_CNS_INT -- constant is a var arg cookie handle - GTF_ICON_PINVKI_HDL = 0xA0000000, // GT_CNS_INT -- constant is a pinvoke calli handle - GTF_ICON_TOKEN_HDL = 0xB0000000, // GT_CNS_INT -- constant is a token handle (other than class, method or field) - GTF_ICON_TLS_HDL = 0xC0000000, // GT_CNS_INT -- constant is a TLS ref with offset - GTF_ICON_FTN_ADDR = 0xD0000000, // GT_CNS_INT -- constant is a function address - GTF_ICON_CIDMID_HDL = 0xE0000000, // GT_CNS_INT -- constant is a class ID or a module ID - GTF_ICON_BBC_PTR = 0xF0000000, // GT_CNS_INT -- constant is a basic block count pointer - - GTF_ICON_FIELD_OFF = 0x08000000, // GT_CNS_INT -- constant is a field offset - GTF_ICON_SIMD_COUNT = 0x04000000, // GT_CNS_INT -- constant is Vector.Count - - GTF_ICON_INITCLASS = 0x02000000, // GT_CNS_INT -- Constant is used to access a static that requires preceding + 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 string handle + 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_REUSE_REG_VAL = 0x00800000 // GT_CNS_INT -- GTF_REUSE_REG_VAL, defined above + GTF_ICON_FIELD_OFF = 0x00400000, // GT_CNS_INT -- constant is a field offset + 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 diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index b8f52bf71aa8c..103030a9fcbdd 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -8116,15 +8116,23 @@ GenTree* Compiler::impImportStaticFieldAccess(CORINFO_RESOLVED_TOKEN* pResolvedT void** pFldAddr = nullptr; void* fldAddr = info.compCompHnd->getFieldAddress(pResolvedToken->hField, (void**)&pFldAddr); - // We should always be able to access this static's address directly - // + // We should always be able to access this static's address directly. assert(pFldAddr == nullptr); + GenTreeFlags handleKind; + if ((pFieldInfo->fieldFlags & CORINFO_FLG_FIELD_STATIC_IN_HEAP) != 0) + { + handleKind = GTF_ICON_STATIC_BOX_PTR; + } + else + { + handleKind = GTF_ICON_STATIC_HDL; + } + FieldSeqNode* fldSeq = GetFieldSeqStore()->CreateSingleton(pResolvedToken->hField); - /* Create the data member node */ - op1 = gtNewIconHandleNode(pFldAddr == nullptr ? (size_t)fldAddr : (size_t)pFldAddr, GTF_ICON_STATIC_HDL, - fldSeq); + // Create the address node. + op1 = gtNewIconHandleNode((size_t)fldAddr, handleKind, fldSeq); #ifdef DEBUG op1->AsIntCon()->gtTargetHandle = op1->AsIntCon()->gtIconVal; #endif @@ -8175,6 +8183,7 @@ GenTree* Compiler::impImportStaticFieldAccess(CORINFO_RESOLVED_TOKEN* pResolvedT if (pFieldInfo->fieldFlags & CORINFO_FLG_FIELD_STATIC_IN_HEAP) { op1 = gtNewOperNode(GT_IND, TYP_REF, op1); + op1->gtFlags |= (GTF_IND_INVARIANT | GTF_IND_NONFAULTING | GTF_IND_NONNULL); FieldSeqNode* fldSeq = GetFieldSeqStore()->CreateSingleton(FieldSeqStore::FirstElemPseudoField); diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 911dadf03d9fd..ecaa0476d6db8 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -6272,13 +6272,25 @@ GenTree* Compiler::fgMorphField(GenTree* tree, MorphAddrContext* mac) // The address is not directly addressible, so force it into a // constant, so we handle it properly - GenTree* addr = gtNewIconHandleNode((size_t)fldAddr, GTF_ICON_STATIC_HDL); - addr->gtType = TYP_I_IMPL; - FieldSeqNode* fieldSeq = - fieldMayOverlap ? FieldSeqStore::NotAField() : GetFieldSeqStore()->CreateSingleton(symHnd); - addr->AsIntCon()->gtFieldSeq = fieldSeq; - // Translate GTF_FLD_INITCLASS to GTF_ICON_INITCLASS - if ((tree->gtFlags & GTF_FLD_INITCLASS) != 0) + bool isBoxedStatic = gtIsStaticFieldPtrToBoxedStruct(tree->TypeGet(), symHnd); + GenTreeFlags handleKind = GTF_EMPTY; + if (isBoxedStatic) + { + handleKind = GTF_ICON_STATIC_BOX_PTR; + } + else if (isStaticReadOnlyInited) + { + handleKind = GTF_ICON_CONST_PTR; + } + else + { + handleKind = GTF_ICON_STATIC_HDL; + } + FieldSeqNode* fieldSeq = GetFieldSeqStore()->CreateSingleton(symHnd); + GenTree* addr = gtNewIconHandleNode((size_t)fldAddr, handleKind, fieldSeq); + + // Translate GTF_FLD_INITCLASS to GTF_ICON_INITCLASS, if we need to. + if (((tree->gtFlags & GTF_FLD_INITCLASS) != 0) && !isStaticReadOnlyInited) { tree->gtFlags &= ~GTF_FLD_INITCLASS; addr->gtFlags |= GTF_ICON_INITCLASS; @@ -6287,14 +6299,18 @@ GenTree* Compiler::fgMorphField(GenTree* tree, MorphAddrContext* mac) tree->SetOper(GT_IND); tree->AsOp()->gtOp1 = addr; - if (isStaticReadOnlyInited) + if (isBoxedStatic) + { + // The box for the static cannot be null, and is logically invariant, since it + // represents (a base for) the static's address. + tree->gtFlags |= (GTF_IND_INVARIANT | GTF_IND_NONFAULTING | GTF_IND_NONNULL); + } + else if (isStaticReadOnlyInited) { JITDUMP("Marking initialized static read-only field '%s' as invariant.\n", eeGetFieldName(symHnd)); // Static readonly field is not null at this point (see getStaticFieldCurrentClass impl). tree->gtFlags |= (GTF_IND_INVARIANT | GTF_IND_NONFAULTING | GTF_IND_NONNULL); - tree->gtFlags &= ~GTF_ICON_INITCLASS; - addr->gtFlags = GTF_ICON_CONST_PTR; } return fgMorphSmpOp(tree); diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 51927d5256c8e..9da20c770d0d3 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -8808,6 +8808,7 @@ void Compiler::fgValueNumberTree(GenTree* tree) vnStore->VNPUnpackExc(addr->gtVNPair, &addrNvnp, &addrXvnp); // Is the dereference immutable? If so, model it as referencing the read-only heap. + // TODO-VNTypes: this code needs to encode the types of the indirections. if (tree->gtFlags & GTF_IND_INVARIANT) { assert(!isVolatile); // We don't expect both volatile and invariant @@ -8830,22 +8831,13 @@ void Compiler::fgValueNumberTree(GenTree* tree) if (!wasNewobj) { - // Is this invariant indirect expected to always return a non-null value? + // TODO-VNTypes: non-null indirects should only be used for TYP_REFs. if ((tree->gtFlags & GTF_IND_NONNULL) != 0) { assert(tree->gtFlags & GTF_IND_NONFAULTING); tree->gtVNPair = vnStore->VNPairForFunc(tree->TypeGet(), VNF_NonNullIndirect, addrNvnp); - if (addr->IsCnsIntOrI()) - { - assert(addrXvnp.BothEqual() && - (addrXvnp.GetLiberal() == ValueNumStore::VNForEmptyExcSet())); - } - else - { - assert(false && "it's not expected to be hit at the moment, but can be allowed."); - // tree->gtVNPair = vnStore->VNPWithExc(tree->gtVNPair, addrXvnp); - } + tree->gtVNPair = vnStore->VNPWithExc(tree->gtVNPair, addrXvnp); } else {