diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 5a16ac8a58bf8..422944fed8e7c 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -6731,6 +6731,12 @@ class Compiler CALLINT_ALL, // kills everything (normal method call) }; + enum class FieldKindForVN + { + SimpleStatic, + WithBaseAddr + }; + public: // A "LoopDsc" describes a ("natural") loop. We (currently) require the body of a loop to be a contiguous (in // bbNext order) sequence of basic blocks. (At times, we may require the blocks in a loop to be "properly numbered" @@ -6785,9 +6791,9 @@ class Compiler int lpLoopVarFPCount; // The register count for the FP LclVars that are read/written inside this loop int lpVarInOutFPCount; // The register count for the FP LclVars that are alive inside or across this loop - typedef JitHashTable, bool> FieldHandleSet; - FieldHandleSet* lpFieldsModified; // This has entries (mappings to "true") for all static field and object - // instance fields modified + typedef JitHashTable, FieldKindForVN> + FieldHandleSet; + FieldHandleSet* lpFieldsModified; // This has entries for all static field and object instance fields modified // in the loop. typedef JitHashTable, bool> ClassHandleSet; @@ -6798,7 +6804,7 @@ class Compiler // Adds the variable liveness information for 'blk' to 'this' LoopDsc void AddVariableLiveness(Compiler* comp, BasicBlock* blk); - inline void AddModifiedField(Compiler* comp, CORINFO_FIELD_HANDLE fldHnd); + inline void AddModifiedField(Compiler* comp, CORINFO_FIELD_HANDLE fldHnd, FieldKindForVN fieldKind); // This doesn't *always* take a class handle -- it can also take primitive types, encoded as class handles // (shifted left, with a low-order bit set to distinguish.) // Use the {Encode/Decode}ElemType methods to construct/destruct these. @@ -7045,7 +7051,7 @@ class Compiler void AddVariableLivenessAllContainingLoops(unsigned lnum, BasicBlock* blk); // Adds "fldHnd" to the set of modified fields of "lnum" and any parent loops. - void AddModifiedFieldAllContainingLoops(unsigned lnum, CORINFO_FIELD_HANDLE fldHnd); + void AddModifiedFieldAllContainingLoops(unsigned lnum, CORINFO_FIELD_HANDLE fldHnd, FieldKindForVN fieldKind); // Adds "elemType" to the set of modified array element types of "lnum" and any parent loops. void AddModifiedElemTypeAllContainingLoops(unsigned lnum, CORINFO_CLASS_HANDLE elemType); diff --git a/src/coreclr/jit/compiler.hpp b/src/coreclr/jit/compiler.hpp index b75828d4cc68a..937d144d70ead 100644 --- a/src/coreclr/jit/compiler.hpp +++ b/src/coreclr/jit/compiler.hpp @@ -3346,14 +3346,14 @@ inline void Compiler::optAssertionRemove(AssertionIndex index) } } -inline void Compiler::LoopDsc::AddModifiedField(Compiler* comp, CORINFO_FIELD_HANDLE fldHnd) +inline void Compiler::LoopDsc::AddModifiedField(Compiler* comp, CORINFO_FIELD_HANDLE fldHnd, FieldKindForVN fieldKind) { if (lpFieldsModified == nullptr) { lpFieldsModified = new (comp->getAllocatorLoopHoist()) Compiler::LoopDsc::FieldHandleSet(comp->getAllocatorLoopHoist()); } - lpFieldsModified->Set(fldHnd, true, FieldHandleSet::Overwrite); + lpFieldsModified->Set(fldHnd, fieldKind, FieldHandleSet::Overwrite); } inline void Compiler::LoopDsc::AddModifiedElemType(Compiler* comp, CORINFO_CLASS_HANDLE structHnd) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 1132aaf58f250..d157508b0dd98 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -15930,26 +15930,26 @@ bool GenTreeIntConCommon::AddrNeedsReloc(Compiler* comp) // this: ADD(CONST FldSeq, baseAddr) // // Arguments: -// comp - the Compiler object -// pObj - [out] parameter for instance fields, the object reference -// pStatic - [out] parameter for static fields, the base address -// pFldSeq - [out] parameter for the field sequence +// comp - the Compiler object +// pBaseAddr - [out] parameter for "the base address" +// pFldSeq - [out] parameter for the field sequence // // Return Value: // If "this" matches patterns denoted above, and the FldSeq found is "full", // i. e. starts with a class field or a static field, and includes all the // struct fields that this tree represents the address of, this method will -// return "true" and set either "pObj" or "pStatic" to some value, which must -// be used by the caller as the key into the "first field map" to obtain the -// actual value for the field. +// return "true" and set either "pBaseAddr" to some value, which must be used +// by the caller as the key into the "first field map" to obtain the actual +// value for the field. For instance fields, "base address" will be the object +// reference, for statics - the address to which the field offset with the +// field sequence is added, see "impImportStaticFieldAccess" and "fgMorphField". // -bool GenTree::IsFieldAddr(Compiler* comp, GenTree** pObj, GenTree** pStatic, FieldSeqNode** pFldSeq) +bool GenTree::IsFieldAddr(Compiler* comp, GenTree** pBaseAddr, FieldSeqNode** pFldSeq) { assert(TypeIs(TYP_I_IMPL, TYP_BYREF, TYP_REF)); - *pObj = nullptr; - *pStatic = nullptr; - *pFldSeq = FieldSeqStore::NotAField(); + *pBaseAddr = nullptr; + *pFldSeq = FieldSeqStore::NotAField(); GenTree* baseAddr = nullptr; FieldSeqNode* fldSeq = nullptr; @@ -16006,8 +16006,8 @@ bool GenTree::IsFieldAddr(Compiler* comp, GenTree** pObj, GenTree** pStatic, Fie // 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. - *pStatic = baseAddr; - *pFldSeq = fldSeq; + *pBaseAddr = baseAddr; + *pFldSeq = fldSeq; return true; } @@ -16015,8 +16015,8 @@ bool GenTree::IsFieldAddr(Compiler* comp, GenTree** pObj, GenTree** pStatic, Fie { assert(!comp->eeIsValueClass(comp->info.compCompHnd->getFieldClass(fldSeq->GetFieldHandle()))); - *pObj = baseAddr; - *pFldSeq = fldSeq; + *pBaseAddr = baseAddr; + *pFldSeq = fldSeq; return true; } diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index fd8dd90c2ae70..9f98334b4f92b 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -1945,7 +1945,7 @@ struct GenTree // where Y is an arbitrary tree, and X is a lclVar. unsigned IsLclVarUpdateTree(GenTree** otherTree, genTreeOps* updateOper); - bool IsFieldAddr(Compiler* comp, GenTree** pObj, GenTree** pStatic, FieldSeqNode** pFldSeq); + bool IsFieldAddr(Compiler* comp, GenTree** pBaseAddr, FieldSeqNode** pFldSeq); // Requires "this" to be the address of an array (the child of a GT_IND labeled with GTF_IND_ARR_INDEX). // Sets "pArr" to the node representing the array (either an array object pointer, or perhaps a byref to the some diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp index ec309793e9376..23af2de57ccf4 100644 --- a/src/coreclr/jit/optimizer.cpp +++ b/src/coreclr/jit/optimizer.cpp @@ -7850,24 +7850,16 @@ bool Compiler::optComputeLoopSideEffectsOfBlock(BasicBlock* blk) } else { - // We are only interested in IsFieldAddr()'s fldSeq out parameter. - // - GenTree* obj = nullptr; // unused - GenTree* staticOffset = nullptr; // unused - FieldSeqNode* fldSeq = nullptr; - - if (arg->IsFieldAddr(this, &obj, &staticOffset, &fldSeq) && - (fldSeq != FieldSeqStore::NotAField())) + GenTree* baseAddr = nullptr; + FieldSeqNode* fldSeq = nullptr; + if (arg->IsFieldAddr(this, &baseAddr, &fldSeq)) { - // Get the first (object) field from field seq. GcHeap[field] will yield the "field map". - assert(fldSeq != nullptr); - if (fldSeq->IsFirstElemFieldSeq()) - { - fldSeq = fldSeq->m_next; - assert(fldSeq != nullptr); - } + assert((fldSeq != nullptr) && (fldSeq != FieldSeqStore::NotAField()) && + !fldSeq->IsPseudoField()); - AddModifiedFieldAllContainingLoops(mostNestedLoop, fldSeq->m_fieldHnd); + FieldKindForVN fieldKind = + (baseAddr != nullptr) ? FieldKindForVN::WithBaseAddr : FieldKindForVN::SimpleStatic; + AddModifiedFieldAllContainingLoops(mostNestedLoop, fldSeq->GetFieldHandle(), fieldKind); // Conservatively assume byrefs may alias this object. memoryHavoc |= memoryKindSet(ByrefExposed); } @@ -7893,7 +7885,8 @@ bool Compiler::optComputeLoopSideEffectsOfBlock(BasicBlock* blk) } else if (lhs->OperGet() == GT_CLS_VAR) { - AddModifiedFieldAllContainingLoops(mostNestedLoop, lhs->AsClsVar()->gtClsVarHnd); + AddModifiedFieldAllContainingLoops(mostNestedLoop, lhs->AsClsVar()->gtClsVarHnd, + FieldKindForVN::SimpleStatic); // Conservatively assume byrefs may alias this static field memoryHavoc |= memoryKindSet(ByrefExposed); } @@ -8073,12 +8066,12 @@ void Compiler::AddVariableLivenessAllContainingLoops(unsigned lnum, BasicBlock* } // Adds "fldHnd" to the set of modified fields of "lnum" and any parent loops. -void Compiler::AddModifiedFieldAllContainingLoops(unsigned lnum, CORINFO_FIELD_HANDLE fldHnd) +void Compiler::AddModifiedFieldAllContainingLoops(unsigned lnum, CORINFO_FIELD_HANDLE fldHnd, FieldKindForVN fieldKind) { assert(0 <= lnum && lnum < optLoopCount); while (lnum != BasicBlock::NOT_IN_LOOP) { - optLoopTable[lnum].AddModifiedField(this, fldHnd); + optLoopTable[lnum].AddModifiedField(this, fldHnd, fieldKind); lnum = optLoopTable[lnum].lpParent; } } diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index d6939cf905393..cd27ed421bdf7 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -7128,8 +7128,9 @@ ValueNum Compiler::fgMemoryVNForLoopSideEffects(MemoryKind memoryKind, for (Compiler::LoopDsc::FieldHandleSet::KeyIterator ki = fieldsMod->Begin(); !ki.Equal(fieldsMod->End()); ++ki) { - CORINFO_FIELD_HANDLE fldHnd = ki.Get(); - ValueNum fldHndVN = vnStore->VNForHandle(ssize_t(fldHnd), GTF_ICON_FIELD_HDL); + CORINFO_FIELD_HANDLE fldHnd = ki.Get(); + FieldKindForVN fieldKind = ki.GetValue(); + ValueNum fldHndVN = vnStore->VNForHandle(ssize_t(fldHnd), GTF_ICON_FIELD_HDL); #ifdef DEBUG if (verbose) @@ -7140,9 +7141,9 @@ ValueNum Compiler::fgMemoryVNForLoopSideEffects(MemoryKind memoryKind, } #endif // DEBUG - // Instance field maps get a placeholder type - they do not represent "singular" - // values. Static field maps, on the other hand, do, and so must be given proper types. - var_types fldMapType = eeIsFieldStatic(fldHnd) ? eeGetFieldType(fldHnd) : TYP_MEM; + // Instance fields and "complex" statics select "first field maps" + // with a placeholder type. "Simple" statics select their own types. + var_types fldMapType = (fieldKind == FieldKindForVN::WithBaseAddr) ? TYP_MEM : eeGetFieldType(fldHnd); newMemoryVN = vnStore->VNForMapStore(newMemoryVN, fldHndVN, vnStore->VNForExpr(entryBlock, fldMapType)); } @@ -7735,9 +7736,8 @@ void Compiler::fgValueNumberAssignment(GenTreeOp* tree) // Was the argument of the GT_IND the address of a local, handled above? if (!wasLocal) { - GenTree* obj = nullptr; - GenTree* staticOffset = nullptr; - FieldSeqNode* fldSeq = nullptr; + GenTree* baseAddr = nullptr; + FieldSeqNode* fldSeq = nullptr; // Is the LHS an array index expression? if (argIsVNFunc && funcApp.m_func == VNF_PtrToArrElem) @@ -7796,7 +7796,7 @@ void Compiler::fgValueNumberAssignment(GenTreeOp* tree) rhsVNPair.GetLiberal(), lhs->TypeGet()); recordGcHeapStore(tree, heapVN DEBUGARG("ArrIndexAssign (case 2)")); } - else if (arg->IsFieldAddr(this, &obj, &staticOffset, &fldSeq)) + else if (arg->IsFieldAddr(this, &baseAddr, &fldSeq)) { assert((fldSeq != nullptr) && (fldSeq != FieldSeqStore::NotAField()) && !fldSeq->IsPseudoField()); @@ -7807,11 +7807,10 @@ void Compiler::fgValueNumberAssignment(GenTreeOp* tree) // We will check that the final field in the sequence matches 'indType'. var_types indType = lhs->TypeGet(); - // when (obj != nullptr) we have an instance field, otherwise a static field - // when (staticOffset != nullptr) it represents a offset into a static or the call to - // Shared Static Base - if ((obj != nullptr) || (staticOffset != nullptr)) + if (baseAddr != nullptr) { + // Instance field / "complex" static: heap[field][baseAddr][struct fields...] = storeVal. + var_types firstFieldType; ValueNum firstFieldSelectorVN = vnStore->VNForFieldSelector(fldSeq->GetFieldHandle(), &firstFieldType); @@ -7821,15 +7820,7 @@ void Compiler::fgValueNumberAssignment(GenTreeOp* tree) ValueNum fldMapVN = vnStore->VNForMapSelect(VNK_Liberal, TYP_MEM, fgCurMemoryVN[GcHeap], firstFieldSelectorVN); - ValueNum firstFieldValueSelectorVN = ValueNumStore::NoVN; - if (obj != nullptr) - { - firstFieldValueSelectorVN = vnStore->VNLiberalNormalValue(obj->gtVNPair); - } - else // (staticOffset != nullptr) - { - firstFieldValueSelectorVN = vnStore->VNLiberalNormalValue(staticOffset->gtVNPair); - } + ValueNum firstFieldValueSelectorVN = vnStore->VNLiberalNormalValue(baseAddr->gtVNPair); ValueNum newFirstFieldValueVN = ValueNumStore::NoVN; // Optimization: avoid traversting the maps for the value of the first field if @@ -7840,12 +7831,12 @@ void Compiler::fgValueNumberAssignment(GenTreeOp* tree) } else { - // Construct the ValueNumber for fldMap[obj/offset]. This (struct) + // Construct the ValueNumber for fldMap[baseAddr]. This (struct) // map represents the specific field we're looking to store to. ValueNum firstFieldValueVN = vnStore->VNForMapSelect(VNK_Liberal, firstFieldType, fldMapVN, firstFieldValueSelectorVN); - // Construct the maps updating the rest of the fields in the sequence. + // Construct the maps updating the struct fields in the sequence. newFirstFieldValueVN = vnStore->VNApplySelectorsAssign(VNK_Liberal, firstFieldValueVN, fldSeq->m_next, storeVal, indType); } @@ -7859,7 +7850,7 @@ void Compiler::fgValueNumberAssignment(GenTreeOp* tree) } else { - // Plain static field. + // "Simple" static: heap[field][struct fields...] = storeVal. newHeapVN = vnStore->VNApplySelectorsAssign(VNK_Liberal, fgCurMemoryVN[GcHeap], fldSeq, storeVal, indType); } @@ -8771,12 +8762,11 @@ void Compiler::fgValueNumberTree(GenTree* tree) // a pointer to an object field or array element. Other cases become uses of // the current ByrefExposed value and the pointer value, so that at least we // can recognize redundant loads with no stores between them. - GenTree* addr = tree->AsIndir()->Addr(); - GenTreeLclVarCommon* lclVarTree = nullptr; - FieldSeqNode* fldSeq2 = nullptr; - GenTree* obj = nullptr; - GenTree* staticOffset = nullptr; - bool isVolatile = (tree->gtFlags & GTF_IND_VOLATILE) != 0; + GenTree* addr = tree->AsIndir()->Addr(); + GenTreeLclVarCommon* lclVarTree = nullptr; + FieldSeqNode* fldSeq = nullptr; + GenTree* baseAddr = nullptr; + bool isVolatile = (tree->gtFlags & GTF_IND_VOLATILE) != 0; // See if the addr has any exceptional part. ValueNumPair addrNvnp; @@ -8964,41 +8954,42 @@ void Compiler::fgValueNumberTree(GenTree* tree) { fgValueNumberArrIndexVal(tree, &funcApp, addrXvnp.GetLiberal()); } - else if (addr->IsFieldAddr(this, &obj, &staticOffset, &fldSeq2)) + else if (addr->IsFieldAddr(this, &baseAddr, &fldSeq)) { - assert((fldSeq2 != nullptr) && (fldSeq2 != FieldSeqStore::NotAField()) && - !fldSeq2->IsPseudoField()); + assert((fldSeq != nullptr) && (fldSeq != FieldSeqStore::NotAField()) && !fldSeq->IsPseudoField()); // The size of the ultimate value we will select, if it is of a struct type. - size_t structSize = 0; + size_t structSize = 0; + ValueNum valueVN = ValueNumStore::NoVN; - // Get the selector for the first field. - var_types firstFieldType; - ValueNum firstFieldSelectorVN = - vnStore->VNForFieldSelector(fldSeq2->GetFieldHandle(), &firstFieldType, &structSize); + if (baseAddr != nullptr) + { + // Instance field / "complex" static: heap[field][baseAddr][struct fields...]. - ValueNum fldMapVN = - vnStore->VNForMapSelect(VNK_Liberal, TYP_MEM, fgCurMemoryVN[GcHeap], firstFieldSelectorVN); + // Get the selector for the first field. + var_types firstFieldType; + ValueNum firstFieldSelectorVN = + vnStore->VNForFieldSelector(fldSeq->GetFieldHandle(), &firstFieldType, &structSize); - ValueNum firstFieldValueSelectorVN; - if (obj != nullptr) - { - firstFieldValueSelectorVN = vnStore->VNLiberalNormalValue(obj->gtVNPair); + ValueNum fldMapVN = + vnStore->VNForMapSelect(VNK_Liberal, TYP_MEM, fgCurMemoryVN[GcHeap], firstFieldSelectorVN); + + ValueNum firstFieldValueSelectorVN = vnStore->VNLiberalNormalValue(baseAddr->gtVNPair); + + // Construct the value number for fldMap[baseAddr]. + ValueNum firstFieldValueVN = + vnStore->VNForMapSelect(VNK_Liberal, firstFieldType, fldMapVN, firstFieldValueSelectorVN); + + // Finally, account for the rest of the fields in the sequence. + valueVN = + vnStore->VNApplySelectors(VNK_Liberal, firstFieldValueVN, fldSeq->m_next, &structSize); } else { - assert(staticOffset != nullptr); - firstFieldValueSelectorVN = vnStore->VNLiberalNormalValue(staticOffset->gtVNPair); + // "Simple" static: heap[static][struct fields...]. + valueVN = vnStore->VNApplySelectors(VNK_Liberal, fgCurMemoryVN[GcHeap], fldSeq, &structSize); } - // Construct the value number for fldMap[obj/offset]. - ValueNum firstFieldValueVN = - vnStore->VNForMapSelect(VNK_Liberal, firstFieldType, fldMapVN, firstFieldValueSelectorVN); - - // Finally, account for the rest of the fields in the sequence. - ValueNum valueVN = - vnStore->VNApplySelectors(VNK_Liberal, firstFieldValueVN, fldSeq2->m_next, &structSize); - valueVN = vnStore->VNApplySelectorsTypeCheck(valueVN, tree->TypeGet(), structSize); tree->gtVNPair.SetLiberal(valueVN);