diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 35b9d2d829ca42..7a2909373104cc 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -15917,6 +15917,9 @@ bool GenTree::IsFieldAddr(Compiler* comp, GenTree** pBaseAddr, FieldSeqNode** pF // will effectively treat such cases ("possible" in unsafe code) as undefined behavior. if (comp->eeIsFieldStatic(fldSeq->GetFieldHandle())) { + // TODO-VNTypes: this code is out of sync w.r.t. boxed statics that are numbered with + // VNF_PtrToStatic and treated as "simple" while here we treat them as "complex". + // TODO-VNTypes: we will always return the "baseAddr" here for now, but strictly speaking, // we only need to do that if we have a shared field, to encode the logical "instantiation" // argument. In all other cases, this serves no purpose and just leads to redundant maps. diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index f07fe3ca9ed02b..4e458d5ad80323 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -6265,20 +6265,32 @@ GenTree* Compiler::fgMorphField(GenTree* tree, MorphAddrContext* mac) FieldSeqNode* fldSeq = !isBoxedStatic ? GetFieldSeqStore()->CreateSingleton(symHnd) : FieldSeqStore::NotAField(); -#ifdef TARGET_64BIT + // TODO-CQ: enable this optimization for 32 bit targets. bool isStaticReadOnlyInited = false; - bool plsSpeculative = true; - if (info.compCompHnd->getStaticFieldCurrentClass(symHnd, &plsSpeculative) != NO_CLASS_HANDLE) +#ifdef TARGET_64BIT + if (tree->TypeIs(TYP_REF) && !isBoxedStatic) { - isStaticReadOnlyInited = !plsSpeculative; + bool pIsSpeculative = true; + if (info.compCompHnd->getStaticFieldCurrentClass(symHnd, &pIsSpeculative) != NO_CLASS_HANDLE) + { + isStaticReadOnlyInited = !pIsSpeculative; + } } +#endif // TARGET_64BIT - // even if RelocTypeHint is REL32 let's still prefer IND over GT_CLS_VAR - // for static readonly fields of statically initialized classes - thus we can - // apply GTF_IND_INVARIANT flag and make it hoistable/CSE-friendly - if (isStaticReadOnlyInited || (IMAGE_REL_BASED_REL32 != eeGetRelocTypeHint(fldAddr))) + // TODO: choices made below have mostly historical reasons and + // should be unified to always use the IND(
) form. + CLANG_FORMAT_COMMENT_ANCHOR; + +#ifdef TARGET_64BIT + bool preferIndir = + isBoxedStatic || isStaticReadOnlyInited || (IMAGE_REL_BASED_REL32 != eeGetRelocTypeHint(fldAddr)); +#else // !TARGET_64BIT + bool preferIndir = isBoxedStatic; +#endif // !TARGET_64BIT + + if (preferIndir) { - // The address is not directly addressible, so force it into a constant, so we handle it properly. GenTreeFlags handleKind = GTF_EMPTY; if (isBoxedStatic) { @@ -6321,7 +6333,6 @@ GenTree* Compiler::fgMorphField(GenTree* tree, MorphAddrContext* mac) return fgMorphSmpOp(tree); } else -#endif // TARGET_64BIT { // Only volatile or classinit could be set, and they map over noway_assert((tree->gtFlags & ~(GTF_FLD_VOLATILE | GTF_FLD_INITCLASS | GTF_COMMON_MASK)) == 0); diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 704cefdd27537b..3ec66459831975 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -7802,8 +7802,24 @@ void Compiler::fgValueNumberAssignment(GenTreeOp* tree) GenTree* baseAddr = nullptr; FieldSeqNode* fldSeq = nullptr; + if (argIsVNFunc && funcApp.m_func == VNF_PtrToStatic) + { + FieldSeqNode* fldSeq = vnStore->FieldSeqVNToFieldSeq(funcApp.m_args[1]); + assert(fldSeq != nullptr); // We should never see an empty sequence here. + + if (fldSeq != FieldSeqStore::NotAField()) + { + ValueNum newHeapVN = vnStore->VNApplySelectorsAssign(VNK_Liberal, fgCurMemoryVN[GcHeap], fldSeq, + rhsVNPair.GetLiberal(), lhs->TypeGet()); + recordGcHeapStore(tree, newHeapVN DEBUGARG("static field store")); + } + else + { + fgMutateGcHeap(tree DEBUGARG("indirect store at NotAField PtrToStatic address")); + } + } // Is the LHS an array index expression? - if (argIsVNFunc && funcApp.m_func == VNF_PtrToArrElem) + else if (argIsVNFunc && funcApp.m_func == VNF_PtrToArrElem) { CORINFO_CLASS_HANDLE elemTypeEq = CORINFO_CLASS_HANDLE(vnStore->ConstantValue(funcApp.m_args[0])); @@ -8625,39 +8641,19 @@ void Compiler::fgValueNumberTree(GenTree* tree) ValueNumPair clsVarVNPair; GenTreeClsVar* clsVar = tree->AsClsVar(); FieldSeqNode* fldSeq = clsVar->gtFieldSeq; - assert(fldSeq != nullptr); // We need to have one. - CORINFO_FIELD_HANDLE fieldHnd = clsVar->gtClsVarHnd; - assert(fieldHnd != NO_FIELD_HANDLE); - ValueNum selectedStaticVar = ValueNumStore::NoVN; - - if (fldSeq == FieldSeqStore::NotAField()) - { - // This is the box for a "boxed static" - see "fgMorphField". - assert(gtIsStaticFieldPtrToBoxedStruct(clsVar->TypeGet(), fieldHnd)); - - // We will create an empty field sequence for VNF_PtrToStatic here. We will assume - // the actual sequence will get appended later, when processing the TARGET_POINTER_SIZE - // offset that is always added to this box to get to its payload. + assert((fldSeq != nullptr) && (fldSeq != FieldSeqStore::NotAField())); // We need to have one. - ValueNum fieldHndVN = vnStore->VNForHandle(ssize_t(fieldHnd), GTF_ICON_FIELD_HDL); - ValueNum fieldSeqVN = vnStore->VNForFieldSeq(nullptr); - clsVarVNPair.SetBoth(vnStore->VNForFunc(TYP_REF, VNF_PtrToStatic, fieldHndVN, fieldSeqVN)); - } - else - { - // This is a reference to heap memory. - // We model statics as indices into GcHeap (which is a subset of ByrefExposed). + // This is a reference to heap memory. + // We model statics as indices into GcHeap (which is a subset of ByrefExposed). + size_t structSize = 0; + ValueNum selectedStaticVar = + vnStore->VNApplySelectors(VNK_Liberal, fgCurMemoryVN[GcHeap], fldSeq, &structSize); + selectedStaticVar = + vnStore->VNApplySelectorsTypeCheck(selectedStaticVar, tree->TypeGet(), structSize); - size_t structSize = 0; - selectedStaticVar = - vnStore->VNApplySelectors(VNK_Liberal, fgCurMemoryVN[GcHeap], fldSeq, &structSize); - selectedStaticVar = - vnStore->VNApplySelectorsTypeCheck(selectedStaticVar, tree->TypeGet(), structSize); - - clsVarVNPair.SetLiberal(selectedStaticVar); - // The conservative interpretation always gets a new, unique VN. - clsVarVNPair.SetConservative(vnStore->VNForExpr(compCurBB, tree->TypeGet())); - } + clsVarVNPair.SetLiberal(selectedStaticVar); + // The conservative interpretation always gets a new, unique VN. + clsVarVNPair.SetConservative(vnStore->VNForExpr(compCurBB, tree->TypeGet())); // The ValueNum returned must represent the full-sized IL-Stack value // If we need to widen this value then we need to introduce a VNF_Cast here to represent @@ -8834,9 +8830,22 @@ void Compiler::fgValueNumberTree(GenTree* tree) if (!wasNewobj) { + // Indirections off of addresses for boxed statics represent bases for + // the address of the static itself. Here we will use "nullptr" for the + // field sequence and assume the actual static field will be appended to + // it later, as part of numbering the method table pointer offset addition. + if (addr->IsCnsIntOrI() && addr->IsIconHandle(GTF_ICON_STATIC_BOX_PTR)) + { + assert(addrNvnp.BothEqual() && (addrXvnp == vnStore->VNPForEmptyExcSet())); + ValueNum boxAddrVN = addrNvnp.GetLiberal(); + ValueNum fieldSeqVN = vnStore->VNForFieldSeq(nullptr); + ValueNum staticAddrVN = + vnStore->VNForFunc(tree->TypeGet(), VNF_PtrToStatic, boxAddrVN, fieldSeqVN); + tree->gtVNPair = ValueNumPair(staticAddrVN, staticAddrVN); + } // 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) + else if ((tree->gtFlags & GTF_IND_NONNULL) != 0) { assert(tree->gtFlags & GTF_IND_NONFAULTING); tree->gtVNPair = vnStore->VNPairForFunc(tree->TypeGet(), VNF_NonNullIndirect, addrNvnp); @@ -10614,8 +10623,7 @@ void Compiler::fgValueNumberAddExceptionSetForIndirection(GenTree* tree, GenTree // The normal VN for base address is used to create the NullPtrExc ValueNumPair vnpBaseNorm = vnStore->VNPNormalPair(baseVNP); - - ValueNumPair excChkSet = vnStore->VNPForEmptyExcSet(); + ValueNumPair excChkSet = vnStore->VNPForEmptyExcSet(); if (!vnStore->IsKnownNonNull(vnpBaseNorm.GetLiberal())) { diff --git a/src/coreclr/jit/valuenumfuncs.h b/src/coreclr/jit/valuenumfuncs.h index fc10c05398cf03..f82c2ff603d07e 100644 --- a/src/coreclr/jit/valuenumfuncs.h +++ b/src/coreclr/jit/valuenumfuncs.h @@ -16,7 +16,7 @@ ValueNumFuncDef(NotAField, 0, false, false, false) // Value number function for ValueNumFuncDef(PtrToLoc, 2, false, true, false) // Pointer (byref) to a local variable. Args: VN's of: 0: var num, 1: FieldSeq. ValueNumFuncDef(PtrToArrElem, 4, false, false, false) // Pointer (byref) to an array element. Args: 0: array elem type eq class var_types value, VN's of: 1: array, 2: index, 3: FieldSeq. ValueNumFuncDef(PtrToStatic, 2, false, true, false) // Pointer (byref) to a static variable (or possibly a field thereof, if the static variable is a struct). - // Args: 0: (VN of) the field handle, 1: the field sequence, of which the first element is the static itself. + // Args: 0: (VN of) the box's address if the static is "boxed", 1: the field sequence, of which the first element is the static itself. ValueNumFuncDef(Phi, 2, false, false, false) // A phi function. Only occurs as arg of PhiDef or PhiMemoryDef. Arguments are SSA numbers of var being defined. ValueNumFuncDef(PhiDef, 3, false, false, false) // Args: 0: local var # (or -1 for memory), 1: SSA #, 2: VN of definition. diff --git a/src/coreclr/jit/valuenumtype.h b/src/coreclr/jit/valuenumtype.h index 1c991e5c05a971..5ed44e280f8f7e 100644 --- a/src/coreclr/jit/valuenumtype.h +++ b/src/coreclr/jit/valuenumtype.h @@ -96,6 +96,16 @@ struct ValueNumPair m_conservative = vn; } + bool operator==(const ValueNumPair& other) const + { + return (m_liberal == other.m_liberal) && (m_conservative == other.m_conservative); + } + + bool operator!=(const ValueNumPair& other) const + { + return !(*this == other); + } + void operator=(const ValueNumPair& vn2) { m_liberal = vn2.m_liberal;