diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 03c317da5c42e..f92b51c542ca3 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -3058,7 +3058,7 @@ class Compiler // Check if this tree is a typeof() bool gtIsTypeof(GenTree* tree, CORINFO_CLASS_HANDLE* handle = nullptr); - GenTree* gtCallGetDefinedRetBufLclAddr(GenTreeCall* call); + GenTreeLclVarCommon* gtCallGetDefinedRetBufLclAddr(GenTreeCall* call); //------------------------------------------------------------------------- // Functions to display the trees diff --git a/src/coreclr/jit/gcinfo.cpp b/src/coreclr/jit/gcinfo.cpp index 97fc0a78bc440..5cc69df3f3859 100644 --- a/src/coreclr/jit/gcinfo.cpp +++ b/src/coreclr/jit/gcinfo.cpp @@ -285,7 +285,7 @@ GCInfo::WriteBarrierForm GCInfo::gcIsWriteBarrierCandidate(GenTreeStoreInd* stor // GCInfo::WriteBarrierForm GCInfo::gcWriteBarrierFormFromTargetAddress(GenTree* tgtAddr) { - if (tgtAddr->IsLocalAddrExpr() != nullptr) + if (tgtAddr->OperIsLocalAddr()) { // No need for a GC barrier when writing to a local variable. return GCInfo::WBF_NoBarrier; diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 47ce69981e1d8..c4bf547c57cec 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -5485,25 +5485,6 @@ unsigned Compiler::gtSetEvalOrder(GenTree* tree) goto DONE; - case GT_COLON: - - level = gtSetEvalOrder(op1); - lvl2 = gtSetEvalOrder(op2); - - if (level < lvl2) - { - level = lvl2; - } - else if (level == lvl2) - { - level += 1; - } - - costEx = op1->GetCostEx() + op2->GetCostEx(); - costSz = op1->GetCostSz() + op2->GetCostSz(); - - goto DONE; - case GT_INDEX_ADDR: costEx = 6; // cmp reg,reg; jae throw; mov reg, [addrmode] (not taken) costSz = 9; // jump to cold section @@ -7860,15 +7841,20 @@ GenTreeObj* Compiler::gtNewObjNode(ClassLayout* layout, GenTree* addr) // An Obj is not a global reference, if it is known to be a local struct. if ((addr->gtFlags & GTF_GLOB_REF) == 0) { - GenTreeLclVarCommon* lclNode = addr->IsLocalAddrExpr(); - if (lclNode != nullptr) + // TODO-Bug: this method does not have enough information to make this determination. + // The local may end up (or already is) address-exposed. + if (addr->OperIsLocalAddr()) { objNode->gtFlags |= GTF_IND_NONFAULTING; - if (!lvaIsImplicitByRefLocal(lclNode->GetLclNum())) + if (lvaIsImplicitByRefLocal(addr->AsLclVarCommon()->GetLclNum())) { - objNode->gtFlags &= ~GTF_GLOB_REF; + objNode->gtFlags |= GTF_GLOB_REF; } } + else + { + objNode->gtFlags |= GTF_GLOB_REF; + } } return objNode; @@ -17178,8 +17164,6 @@ bool GenTree::IsPartialLclFld(Compiler* comp) //------------------------------------------------------------------------ // DefinesLocal: Does "this" define a local? // -// Recognizes "ASG" stores. Also recognizes "STORE_OBJ/BLK". -// // Arguments: // comp - the compiler instance // pLclVarTree - [out] parameter for the local representing the definition @@ -17187,30 +17171,27 @@ bool GenTree::IsPartialLclFld(Compiler* comp) // a "full" definition (overwrites the entire variable) // pOffset - optional [out] parameter for the offset, relative to the // local, at which the store is performed +// pSize - optional [out] parameter for the amount of bytes affected +// by the store // // Return Value: -// Whether "this" represents a store to a local variable. +// Whether "this" represents a store to a possibly tracked local variable +// before rationalization. // // Notes: // This function is contractually bound to recognize a superset of stores -// that "LocalAddressVisitor" recognizes, as it is used to detect which -// trees can define tracked locals. +// that "LocalAddressVisitor" recognizes and transforms, as it is used to +// detect which trees can define tracked locals. // bool GenTree::DefinesLocal( Compiler* comp, GenTreeLclVarCommon** pLclVarTree, bool* pIsEntire, ssize_t* pOffset, unsigned* pSize) { assert((pOffset == nullptr) || (*pOffset == 0)); - GenTreeLclVarCommon* lclVarTree = nullptr; - unsigned storeSize = 0; - ssize_t offset = 0; - if (OperIs(GT_ASG)) { GenTree* lhs = AsOp()->gtGetOp1(); - // Return early for the common case. - // if (lhs->OperIs(GT_LCL_VAR)) { *pLclVarTree = lhs->AsLclVarCommon(); @@ -17248,97 +17229,35 @@ bool GenTree::DefinesLocal( return true; } - - if (lhs->OperIsIndir() && lhs->AsIndir()->Addr()->DefinesLocalAddr(&lclVarTree, &offset)) - { - storeSize = lhs->AsIndir()->Size(); - } - else - { - return false; - } } else if (OperIs(GT_CALL)) { - GenTree* retBufArg = comp->gtCallGetDefinedRetBufLclAddr(AsCall()); - if (retBufArg == nullptr) + GenTreeLclVarCommon* lclAddr = comp->gtCallGetDefinedRetBufLclAddr(AsCall()); + if (lclAddr == nullptr) { return false; } - if (!retBufArg->DefinesLocalAddr(&lclVarTree, &offset)) - { - return false; - } + *pLclVarTree = lclAddr; - storeSize = comp->typGetObjLayout(AsCall()->gtRetClsHnd)->GetSize(); - } - else if (OperIs(GT_STORE_BLK, GT_STORE_OBJ) && AsBlk()->Addr()->DefinesLocalAddr(&lclVarTree, &offset)) - { - storeSize = AsBlk()->Size(); - } - else - { - return false; - } - - assert(lclVarTree != nullptr); - *pLclVarTree = lclVarTree; - - if (pIsEntire != nullptr) - { - if (offset == 0) + if ((pIsEntire != nullptr) || (pSize != nullptr)) { - unsigned lclSize = comp->lvaLclExactSize(lclVarTree->GetLclNum()); - if (comp->lvaGetDesc(lclVarTree)->lvNormalizeOnStore()) + unsigned storeSize = comp->typGetObjLayout(AsCall()->gtRetClsHnd)->GetSize(); + + if (pIsEntire != nullptr) { - // It's normalize on store, so use the full storage width -- writing to low bytes won't - // necessarily yield a normalized value. - lclSize = genTypeSize(TYP_INT); + *pIsEntire = storeSize == comp->lvaLclExactSize(lclAddr->GetLclNum()); } - *pIsEntire = storeSize == lclSize; - } - else - { - *pIsEntire = false; + if (pSize != nullptr) + { + *pSize = storeSize; + } } - } - - if (pOffset != nullptr) - { - *pOffset = offset; - } - - if (pSize != nullptr) - { - *pSize = storeSize; - } - - return true; -} - -//------------------------------------------------------------------------ -// DefinesLocalAddr: Does "this" represent a local address tree? -// -// Arguments: -// pLclVarTree - [out] parameter for the local node -// pOffset - optional [out] parameter for the offset (relative to the -// local itself) that this tree computes. The caller must -// initialize this to zero. -// -// Return Value: -// Whether "this" is a LCL_VAR|FLD_ADDR-equivalent tree. -// -bool GenTree::DefinesLocalAddr(GenTreeLclVarCommon** pLclVarTree, ssize_t* pOffset) -{ - if (OperIsLocalAddr()) - { - *pLclVarTree = AsLclVarCommon(); if (pOffset != nullptr) { - *pOffset += AsLclVarCommon()->GetLclOffs(); + *pOffset = lclAddr->GetLclOffs(); } return true; @@ -17347,14 +17266,6 @@ bool GenTree::DefinesLocalAddr(GenTreeLclVarCommon** pLclVarTree, ssize_t* pOffs return false; } -// If this tree evaluates some sum of a local address and some constants, -// return the node for the local being addressed - -const GenTreeLclVarCommon* GenTree::IsLocalAddrExpr() const -{ - return OperIsLocalAddr() ? AsLclVarCommon() : nullptr; -} - //------------------------------------------------------------------------ // IsImplicitByrefParameterValuePreMorph: // Determine if this tree represents the value of an implicit byref @@ -18252,79 +18163,73 @@ CORINFO_CLASS_HANDLE Compiler::gtGetClassHandle(GenTree* tree, bool* pIsExact, b case GT_IND: { GenTreeIndir* indir = obj->AsIndir(); + GenTree* base = indir->Base(); - if (indir->HasBase() && !indir->HasIndex()) + // indir(lcl_var_addr) --> lcl + // + // This comes up during constrained callvirt on ref types. + // + if (base->OperIs(GT_LCL_VAR_ADDR)) { - // indir(addr(lcl)) --> lcl - // - // This comes up during constrained callvirt on ref types. - - GenTree* base = indir->Base(); - GenTreeLclVarCommon* lcl = base->IsLocalAddrExpr(); + const unsigned objLcl = base->AsLclVarCommon()->GetLclNum(); + objClass = lvaTable[objLcl].lvClassHnd; + *pIsExact = lvaTable[objLcl].lvClassIsExact; + } + else if (base->OperIs(GT_INDEX_ADDR, GT_ARR_ELEM)) + { + // indir(arr_elem(...)) -> array element type - if ((lcl != nullptr) && (base->OperGet() != GT_ADD)) + if (base->OperIs(GT_INDEX_ADDR)) { - const unsigned objLcl = lcl->GetLclNum(); - objClass = lvaTable[objLcl].lvClassHnd; - *pIsExact = lvaTable[objLcl].lvClassIsExact; + objClass = gtGetArrayElementClassHandle(base->AsIndexAddr()->Arr()); } - else if (base->OperIs(GT_INDEX_ADDR, GT_ARR_ELEM)) + else { - // indir(arr_elem(...)) -> array element type - - if (base->OperIs(GT_INDEX_ADDR)) - { - objClass = gtGetArrayElementClassHandle(base->AsIndexAddr()->Arr()); - } - else - { - objClass = gtGetArrayElementClassHandle(base->AsArrElem()->gtArrObj); - } - - *pIsExact = false; - *pIsNonNull = false; + objClass = gtGetArrayElementClassHandle(base->AsArrElem()->gtArrObj); } - else if (base->OperGet() == GT_ADD) - { - // TODO-VNTypes: use "IsFieldAddr" here instead. - // This could be a static field access. - // - // See if op1 is a static field base helper call - // and if so, op2 will have the field info. - GenTree* op1 = base->AsOp()->gtOp1; - GenTree* op2 = base->AsOp()->gtOp2; + *pIsExact = false; + *pIsNonNull = false; + } + else if (base->OperGet() == GT_ADD) + { + // TODO-VNTypes: use "IsFieldAddr" here instead. + + // This could be a static field access. + // + // See if op1 is a static field base helper call + // and if so, op2 will have the field info. + GenTree* op1 = base->AsOp()->gtOp1; + GenTree* op2 = base->AsOp()->gtOp2; - if (op2->IsCnsIntOrI()) + if (op2->IsCnsIntOrI()) + { + FieldSeq* fieldSeq = op2->AsIntCon()->gtFieldSeq; + if ((fieldSeq != nullptr) && (fieldSeq->GetOffset() == op2->AsIntCon()->IconValue())) { - FieldSeq* fieldSeq = op2->AsIntCon()->gtFieldSeq; - if ((fieldSeq != nullptr) && (fieldSeq->GetOffset() == op2->AsIntCon()->IconValue())) - { - // No benefit to calling gtGetFieldClassHandle here, as - // the exact field being accessed can vary. - CORINFO_FIELD_HANDLE fieldHnd = fieldSeq->GetFieldHandle(); - CORINFO_CLASS_HANDLE fieldClass = NO_CLASS_HANDLE; - var_types fieldType = eeGetFieldType(fieldHnd, &fieldClass); + // No benefit to calling gtGetFieldClassHandle here, as + // the exact field being accessed can vary. + CORINFO_FIELD_HANDLE fieldHnd = fieldSeq->GetFieldHandle(); + CORINFO_CLASS_HANDLE fieldClass = NO_CLASS_HANDLE; + var_types fieldType = eeGetFieldType(fieldHnd, &fieldClass); - if (fieldType == TYP_REF) - { - objClass = fieldClass; - } + if (fieldType == TYP_REF) + { + objClass = fieldClass; } } } - else if (base->IsIconHandle(GTF_ICON_CONST_PTR, GTF_ICON_STATIC_HDL)) + } + 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; + if ((fldSeq != nullptr) && (fldSeq->GetOffset() == base->AsIntCon()->IconValue())) { - // Check if we have IND(ICON_HANDLE) that represents a static field - FieldSeq* fldSeq = base->AsIntCon()->gtFieldSeq; - if ((fldSeq != nullptr) && (fldSeq->GetOffset() == base->AsIntCon()->IconValue())) - { - CORINFO_FIELD_HANDLE fldHandle = base->AsIntCon()->gtFieldSeq->GetFieldHandle(); - objClass = gtGetFieldClassHandle(fldHandle, pIsExact, pIsNonNull); - } + CORINFO_FIELD_HANDLE fldHandle = base->AsIntCon()->gtFieldSeq->GetFieldHandle(); + objClass = gtGetFieldClassHandle(fldHandle, pIsExact, pIsNonNull); } } - break; } @@ -18628,7 +18533,7 @@ bool Compiler::gtIsTypeof(GenTree* tree, CORINFO_CLASS_HANDLE* handle) // Returns: // A tree representing the address of a local. // -GenTree* Compiler::gtCallGetDefinedRetBufLclAddr(GenTreeCall* call) +GenTreeLclVarCommon* Compiler::gtCallGetDefinedRetBufLclAddr(GenTreeCall* call) { if (!call->IsOptimizingRetBufAsLocal()) { @@ -18654,9 +18559,9 @@ GenTree* Compiler::gtCallGetDefinedRetBufLclAddr(GenTreeCall* call) // This may be called very late to check validity of LIR. node = node->gtSkipReloadOrCopy(); - assert(node->OperIsLocalAddr() && lvaGetDesc(node->AsLclVarCommon())->lvHiddenBufferStructArg); + assert(node->OperIsLocalAddr() && lvaGetDesc(node->AsLclVarCommon())->IsHiddenBufferStructArg()); - return node; + return node->AsLclVarCommon(); } //------------------------------------------------------------------------ @@ -25180,7 +25085,7 @@ bool GenTreeLclFld::IsOffsetMisaligned() const bool GenTree::IsInvariant() const { - return OperIsConst() || IsLocalAddrExpr(); + return OperIsConst() || OperIsLocalAddr(); } //------------------------------------------------------------------------ diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 314cba6023d10..353269f5a25ed 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -1081,7 +1081,7 @@ struct GenTree bool IsNotGcDef() const { - return IsIntegralConst(0) || IsLocalAddrExpr(); + return IsIntegralConst(0) || OperIsLocalAddr(); } // LIR flags @@ -1977,14 +1977,6 @@ struct GenTree ssize_t* pOffset = nullptr, unsigned* pSize = nullptr); - bool DefinesLocalAddr(GenTreeLclVarCommon** pLclVarTree, ssize_t* pOffset = nullptr); - - const GenTreeLclVarCommon* IsLocalAddrExpr() const; - GenTreeLclVarCommon* IsLocalAddrExpr() - { - return const_cast(static_cast(this)->IsLocalAddrExpr()); - } - GenTreeLclVarCommon* IsImplicitByrefParameterValuePreMorph(Compiler* compiler); GenTreeLclVar* IsImplicitByrefParameterValuePostMorph(Compiler* compiler, GenTree** addr); @@ -7379,26 +7371,15 @@ struct GenTreeBlk : public GenTreeIndir struct GenTreeObj : public GenTreeBlk { - void Init() - { - // By default, an OBJ is assumed to be a global reference, unless it is local. - GenTreeLclVarCommon* lcl = Addr()->IsLocalAddrExpr(); - if ((lcl == nullptr) || ((lcl->gtFlags & GTF_GLOB_EFFECT) != 0)) - { - gtFlags |= GTF_GLOB_REF; - } - noway_assert(GetLayout()->GetClassHandle() != NO_CLASS_HANDLE); - } - GenTreeObj(var_types type, GenTree* addr, ClassLayout* layout) : GenTreeBlk(GT_OBJ, type, addr, layout) { - Init(); + noway_assert(GetLayout()->GetClassHandle() != NO_CLASS_HANDLE); } GenTreeObj(var_types type, GenTree* addr, GenTree* data, ClassLayout* layout) : GenTreeBlk(GT_STORE_OBJ, type, addr, data, layout) { - Init(); + noway_assert(GetLayout()->GetClassHandle() != NO_CLASS_HANDLE); } #if DEBUGGABLE_GENTREE diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index d6e36f367391f..27169fc89b352 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -520,10 +520,9 @@ void Compiler::impAppendStmt(Statement* stmt, unsigned chkLevel, bool checkConsu assert(retBuf->TypeIs(TYP_I_IMPL, TYP_BYREF)); - GenTreeLclVarCommon* lclNode = retBuf->IsLocalAddrExpr(); - if (lclNode != nullptr) + if (retBuf->OperIs(GT_LCL_VAR_ADDR)) { - dstVarDsc = lvaGetDesc(lclNode); + dstVarDsc = lvaGetDesc(retBuf->AsLclVarCommon()); } } } @@ -2527,12 +2526,12 @@ CORINFO_CLASS_HANDLE Compiler::impGetObjectClass() /* static */ void Compiler::impBashVarAddrsToI(GenTree* tree1, GenTree* tree2) { - if (tree1->IsLocalAddrExpr() != nullptr) + if (tree1->OperIsLocalAddr()) { tree1->gtType = TYP_I_IMPL; } - if (tree2 && (tree2->IsLocalAddrExpr() != nullptr)) + if (tree2 && tree2->OperIsLocalAddr()) { tree2->gtType = TYP_I_IMPL; } @@ -6798,7 +6797,7 @@ void Compiler::impImportBlockCode(BasicBlock* block) // We had better assign it a value of the correct type assertImp( genActualType(lclTyp) == genActualType(op1->gtType) || - (genActualType(lclTyp) == TYP_I_IMPL && op1->IsLocalAddrExpr() != nullptr) || + (genActualType(lclTyp) == TYP_I_IMPL && op1->OperIsLocalAddr()) || (genActualType(lclTyp) == TYP_I_IMPL && (op1->gtType == TYP_BYREF || op1->gtType == TYP_REF)) || (genActualType(op1->gtType) == TYP_I_IMPL && lclTyp == TYP_BYREF) || (varTypeIsFloating(lclTyp) && varTypeIsFloating(op1->TypeGet())) || @@ -12780,10 +12779,6 @@ bool Compiler::impIsInvariant(const GenTree* tree) // Returns: // true if it points into a local // -// Remarks: -// This is a variant of GenTree::IsLocalAddrExpr that is more suitable for -// use during import. Unlike that function, this one handles field nodes. -// bool Compiler::impIsAddressInLocal(const GenTree* tree, GenTree** lclVarTreeOut) { const GenTree* op = tree; @@ -13491,7 +13486,7 @@ void Compiler::impInlineInitVars(InlineInfo* pInlineInfo) assert(varTypeIsIntOrI(sigType)); /* If possible bash the BYREF to an int */ - if (inlArgNode->IsLocalAddrExpr() != nullptr) + if (inlArgNode->OperIsLocalAddr()) { inlArgNode->gtType = TYP_I_IMPL; lclVarInfo[i].lclVerTypeInfo = typeInfo(varType2tiType(TYP_I_IMPL)); diff --git a/src/coreclr/jit/lclmorph.cpp b/src/coreclr/jit/lclmorph.cpp index def227e98ff27..54402124209ae 100644 --- a/src/coreclr/jit/lclmorph.cpp +++ b/src/coreclr/jit/lclmorph.cpp @@ -461,7 +461,6 @@ class LocalAddressVisitor final : public GenTreeVisitor enum class IndirTransform { - None, Nop, BitCast, NarrowCast, @@ -1143,10 +1142,6 @@ class LocalAddressVisitor final : public GenTreeVisitor switch (transform) { - case IndirTransform::None: - // TODO-ADDR: eliminate all such cases. - return; - case IndirTransform::Nop: indir->gtBashToNOP(); m_stmtModified = true; diff --git a/src/coreclr/jit/liveness.cpp b/src/coreclr/jit/liveness.cpp index 863dca578a61a..5e2f4621edd7d 100644 --- a/src/coreclr/jit/liveness.cpp +++ b/src/coreclr/jit/liveness.cpp @@ -240,18 +240,7 @@ void Compiler::fgPerNodeLocalVarLiveness(GenTree* tree) // Otherwise, we treat it as a use here. if ((tree->gtFlags & GTF_IND_ASG_LHS) == 0) { - GenTreeLclVarCommon* lclVarTree = nullptr; - GenTree* addrArg = tree->AsOp()->gtOp1->gtEffectiveVal(/*commaOnly*/ true); - if (!addrArg->DefinesLocalAddr(&lclVarTree)) - { - fgCurMemoryUse |= memoryKindSet(GcHeap, ByrefExposed); - } - else - { - // Defines a local addr - assert(lclVarTree != nullptr); - fgMarkUseDef(lclVarTree->AsLclVarCommon()); - } + fgCurMemoryUse |= memoryKindSet(GcHeap, ByrefExposed); } break; @@ -260,6 +249,14 @@ void Compiler::fgPerNodeLocalVarLiveness(GenTree* tree) unreached(); break; + case GT_ASG: + // An indirect store defines a memory location. + if (!tree->AsOp()->gtGetOp1()->OperIsLocal()) + { + fgCurMemoryDef |= memoryKindSet(GcHeap, ByrefExposed); + } + break; + // We'll assume these are use-then-defs of memory. case GT_LOCKADD: case GT_XORR: @@ -272,8 +269,11 @@ void Compiler::fgPerNodeLocalVarLiveness(GenTree* tree) fgCurMemoryHavoc |= memoryKindSet(GcHeap, ByrefExposed); break; - case GT_MEMORYBARRIER: - // Similar to any Volatile indirection, we must handle this as a definition of GcHeap/ByrefExposed + case GT_STOREIND: + case GT_STORE_OBJ: + case GT_STORE_BLK: + case GT_STORE_DYN_BLK: + case GT_MEMORYBARRIER: // Similar to Volatile indirections, we must handle this as a memory def. fgCurMemoryDef |= memoryKindSet(GcHeap, ByrefExposed); break; @@ -344,34 +344,10 @@ void Compiler::fgPerNodeLocalVarLiveness(GenTree* tree) } } } - break; } default: - - // Determine what memory locations it defines. - if (tree->OperIs(GT_ASG) || tree->OperIsBlkOp()) - { - GenTreeLclVarCommon* dummyLclVarTree = nullptr; - if (tree->DefinesLocal(this, &dummyLclVarTree)) - { - if (lvaVarAddrExposed(dummyLclVarTree->GetLclNum())) - { - fgCurMemoryDef |= memoryKindSet(ByrefExposed); - - // We've found a store that modifies ByrefExposed - // memory but not GcHeap memory, so track their - // states separately. - byrefStatesMatchGcHeapStates = false; - } - } - else - { - // If it doesn't define a local, then it might update GcHeap/ByrefExposed. - fgCurMemoryDef |= memoryKindSet(GcHeap, ByrefExposed); - } - } break; } } @@ -2298,293 +2274,208 @@ bool Compiler::fgRemoveDeadStore(GenTree** pTree, assert(!compRationalIRForm); // Vars should have already been checked for address exposure by this point. - assert(!varDsc->lvIsStructField || !lvaTable[varDsc->lvParentLcl].IsAddressExposed()); assert(!varDsc->IsAddressExposed()); GenTree* asgNode = nullptr; - GenTree* rhsNode = nullptr; - GenTree* addrNode = nullptr; GenTree* const tree = *pTree; GenTree* nextNode = tree->gtNext; - *pStoreRemoved = false; - - // First, characterize the lclVarTree and see if we are taking its address. - if (tree->OperIsLocalStore()) - { - rhsNode = tree->AsOp()->gtOp1; - asgNode = tree; - } - else if (tree->OperIsLocal()) - { - if (nextNode == nullptr) - { - return false; - } - if (nextNode->OperGet() == GT_ADDR) - { - addrNode = nextNode; - nextNode = nextNode->gtNext; - } - } - else + // We can have two types of assignments: ASG(LCL_VAR/FLD, ...), in which case the assignment must have + // been reversed, so it is [RHS, LCL_VAR/FLD, ASG] in linear order, or we have a call, in which case we + // bail (we most likely cannot remove the call anyway). + if (tree->OperIs(GT_LCL_VAR, GT_LCL_FLD)) { - assert(tree->OperIsLocalAddr()); - addrNode = tree; - } + assert((tree->gtFlags & GTF_VAR_DEF) != 0); + assert(nextNode != nullptr); - // Next, find the assignment (i.e. if we didn't have a LocalStore) - if (asgNode == nullptr) - { - if (addrNode == nullptr) + if (nextNode->OperIs(GT_ASG) && (tree == nextNode->gtGetOp1())) { asgNode = nextNode; } - else - { - // This may be followed by GT_IND/assign or GT_STOREIND. - if (nextNode == nullptr) - { - return false; - } - if (nextNode->OperIsIndir()) - { - // This must be a non-nullcheck form of indir, or it would not be a def. - assert(nextNode->OperGet() != GT_NULLCHECK); - if (nextNode->OperIsStore()) - { - // This is a store, which takes a location and a value to be stored. - // It's 'rhsNode' is the value to be stored. - asgNode = nextNode; - if (asgNode->OperIsBlk()) - { - rhsNode = asgNode->AsBlk()->Data(); - } - else - { - // This is a non-block store. - rhsNode = asgNode->gtGetOp2(); - } - } - else - { - // This is a non-store indirection, and the assignment will com after it. - asgNode = nextNode->gtNext; - } - } - } } + *pStoreRemoved = false; + if (asgNode == nullptr) { return false; } - if (asgNode->OperIs(GT_ASG)) - { - rhsNode = asgNode->gtGetOp2(); - } - else if (rhsNode == nullptr) - { - return false; - } + GenTree* rhsNode = asgNode->gtGetOp2(); - // Do not remove if this local variable represents - // a promoted struct field of an address exposed local. - if (varDsc->lvIsStructField && lvaTable[varDsc->lvParentLcl].IsAddressExposed()) - { - return false; - } + // We are now committed to removing the store. + *pStoreRemoved = true; - // Do not remove if the address of the variable has been exposed. - if (varDsc->IsAddressExposed()) + // Check for side effects on the RHS. + GenTree* sideEffList = nullptr; + if (rhsNode->gtFlags & GTF_SIDE_EFFECT) { - return false; +#ifdef DEBUG + if (verbose) + { + printf(FMT_BB " - Dead assignment has side effects...\n", compCurBB->bbNum); + gtDispTree(asgNode); + printf("\n"); + } +#endif // DEBUG + + gtExtractSideEffList(rhsNode, &sideEffList); } - if (asgNode->gtFlags & GTF_ASG) + // Test for interior statement + if (asgNode->gtNext == nullptr) { - noway_assert(rhsNode); - noway_assert(tree->gtFlags & GTF_VAR_DEF); + // This is a "NORMAL" statement with the assignment node hanging from the statement. - assert(asgNode->OperIs(GT_ASG)); + noway_assert(compCurStmt->GetRootNode() == asgNode); + JITDUMP("top level assign\n"); - // We are now committed to removing the store. - *pStoreRemoved = true; - - // Check for side effects - GenTree* sideEffList = nullptr; - if (rhsNode->gtFlags & GTF_SIDE_EFFECT) + if (sideEffList != nullptr) { + noway_assert(sideEffList->gtFlags & GTF_SIDE_EFFECT); #ifdef DEBUG if (verbose) { - printf(FMT_BB " - Dead assignment has side effects...\n", compCurBB->bbNum); - gtDispTree(asgNode); + printf("Extracted side effects list...\n"); + gtDispTree(sideEffList); printf("\n"); } #endif // DEBUG - // Extract the side effects - gtExtractSideEffList(rhsNode, &sideEffList); - } - - // Test for interior statement - - if (asgNode->gtNext == nullptr) - { - // This is a "NORMAL" statement with the assignment node hanging from the statement. - noway_assert(compCurStmt->GetRootNode() == asgNode); - JITDUMP("top level assign\n"); + // Replace the assignment statement with the list of side effects - if (sideEffList != nullptr) - { - noway_assert(sideEffList->gtFlags & GTF_SIDE_EFFECT); + *pTree = sideEffList; + compCurStmt->SetRootNode(sideEffList); #ifdef DEBUG - if (verbose) - { - printf("Extracted side effects list...\n"); - gtDispTree(sideEffList); - printf("\n"); - } -#endif // DEBUG - - // Replace the assignment statement with the list of side effects - - *pTree = sideEffList; - compCurStmt->SetRootNode(sideEffList); -#ifdef DEBUG - *treeModf = true; + *treeModf = true; #endif // DEBUG - // Update ordering, costs, FP levels, etc. - gtSetStmtInfo(compCurStmt); + // Update ordering, costs, FP levels, etc. + gtSetStmtInfo(compCurStmt); - // Re-link the nodes for this statement - fgSetStmtSeq(compCurStmt); + // Re-link the nodes for this statement + fgSetStmtSeq(compCurStmt); - // Since the whole statement gets replaced it is safe to - // re-thread and update order. No need to compute costs again. - *pStmtInfoDirty = false; + // Since the whole statement gets replaced it is safe to + // re-thread and update order. No need to compute costs again. + *pStmtInfoDirty = false; - // Compute the live set for the new statement - *doAgain = true; - return false; - } - else - { - JITDUMP("removing stmt with no side effects\n"); + // Compute the live set for the new statement + *doAgain = true; + return false; + } + else + { + JITDUMP("removing stmt with no side effects\n"); - // No side effects - remove the whole statement from the block->bbStmtList. - fgRemoveStmt(compCurBB, compCurStmt); + // No side effects - remove the whole statement from the block->bbStmtList. + fgRemoveStmt(compCurBB, compCurStmt); - // Since we removed it do not process the rest (i.e. RHS) of the statement - // variables in the RHS will not be marked as live, so we get the benefit of - // propagating dead variables up the chain - return true; - } + // Since we removed it do not process the rest (i.e. RHS) of the statement + // variables in the RHS will not be marked as live, so we get the benefit of + // propagating dead variables up the chain + return true; + } + } + else + { + // This is an INTERIOR STATEMENT with a dead assignment - remove it + // TODO-Cleanup: I'm not sure this assert is valuable; we've already determined this when + // we computed that it was dead. + if (varDsc->lvTracked) + { + noway_assert(!VarSetOps::IsMember(this, life, varDsc->lvVarIndex)); } else { - // This is an INTERIOR STATEMENT with a dead assignment - remove it - // TODO-Cleanup: I'm not sure this assert is valuable; we've already determined this when - // we computed that it was dead. - if (varDsc->lvTracked) - { - noway_assert(!VarSetOps::IsMember(this, life, varDsc->lvVarIndex)); - } - else + for (unsigned i = 0; i < varDsc->lvFieldCnt; ++i) { - for (unsigned i = 0; i < varDsc->lvFieldCnt; ++i) + unsigned fieldVarNum = varDsc->lvFieldLclStart + i; { - unsigned fieldVarNum = varDsc->lvFieldLclStart + i; - { - LclVarDsc* fieldVarDsc = lvaGetDesc(fieldVarNum); - noway_assert(fieldVarDsc->lvTracked && - !VarSetOps::IsMember(this, life, fieldVarDsc->lvVarIndex)); - } + LclVarDsc* fieldVarDsc = lvaGetDesc(fieldVarNum); + noway_assert(fieldVarDsc->lvTracked && !VarSetOps::IsMember(this, life, fieldVarDsc->lvVarIndex)); } } + } - if (sideEffList != nullptr) + if (sideEffList != nullptr) + { + noway_assert(sideEffList->gtFlags & GTF_SIDE_EFFECT); +#ifdef DEBUG + if (verbose) + { + printf("Extracted side effects list from condition...\n"); + gtDispTree(sideEffList); + printf("\n"); + } +#endif // DEBUG + if (sideEffList->gtOper == asgNode->gtOper) { - noway_assert(sideEffList->gtFlags & GTF_SIDE_EFFECT); #ifdef DEBUG - if (verbose) - { - printf("Extracted side effects list from condition...\n"); - gtDispTree(sideEffList); - printf("\n"); - } + *treeModf = true; #endif // DEBUG - if (sideEffList->gtOper == asgNode->gtOper) - { + asgNode->AsOp()->gtOp1 = sideEffList->AsOp()->gtOp1; + asgNode->AsOp()->gtOp2 = sideEffList->AsOp()->gtOp2; + asgNode->gtType = sideEffList->gtType; + } + else + { #ifdef DEBUG - *treeModf = true; + *treeModf = true; #endif // DEBUG + // Change the node to a GT_COMMA holding the side effect list + asgNode->gtBashToNOP(); + + asgNode->ChangeOper(GT_COMMA); + asgNode->gtFlags |= sideEffList->gtFlags & GTF_ALL_EFFECT; + + if (sideEffList->gtOper == GT_COMMA) + { asgNode->AsOp()->gtOp1 = sideEffList->AsOp()->gtOp1; asgNode->AsOp()->gtOp2 = sideEffList->AsOp()->gtOp2; - asgNode->gtType = sideEffList->gtType; } else { -#ifdef DEBUG - *treeModf = true; -#endif // DEBUG - // Change the node to a GT_COMMA holding the side effect list - asgNode->gtBashToNOP(); - - asgNode->ChangeOper(GT_COMMA); - asgNode->gtFlags |= sideEffList->gtFlags & GTF_ALL_EFFECT; - - if (sideEffList->gtOper == GT_COMMA) - { - asgNode->AsOp()->gtOp1 = sideEffList->AsOp()->gtOp1; - asgNode->AsOp()->gtOp2 = sideEffList->AsOp()->gtOp2; - } - else - { - asgNode->AsOp()->gtOp1 = sideEffList; - asgNode->AsOp()->gtOp2 = gtNewNothingNode(); - } + asgNode->AsOp()->gtOp1 = sideEffList; + asgNode->AsOp()->gtOp2 = gtNewNothingNode(); } } - else - { + } + else + { #ifdef DEBUG - if (verbose) - { - printf("\nRemoving tree "); - printTreeID(asgNode); - printf(" in " FMT_BB " as useless\n", compCurBB->bbNum); - gtDispTree(asgNode); - printf("\n"); - } + if (verbose) + { + printf("\nRemoving tree "); + printTreeID(asgNode); + printf(" in " FMT_BB " as useless\n", compCurBB->bbNum); + gtDispTree(asgNode); + printf("\n"); + } #endif // DEBUG - // No side effects - Change the assignment to a GT_NOP node - asgNode->gtBashToNOP(); + // No side effects - Change the assignment to a GT_NOP node + asgNode->gtBashToNOP(); #ifdef DEBUG - *treeModf = true; + *treeModf = true; #endif // DEBUG - } + } - // Re-link the nodes for this statement - Do not update ordering! + // Re-link the nodes for this statement - Do not update ordering! - // Do not update costs by calling gtSetStmtInfo. fgSetStmtSeq modifies - // the tree threading based on the new costs. Removing nodes could - // cause a subtree to get evaluated first (earlier second) during the - // liveness walk. Instead just set a flag that costs are dirty and - // caller has to call gtSetStmtInfo. - *pStmtInfoDirty = true; + // Do not update costs by calling gtSetStmtInfo. fgSetStmtSeq modifies + // the tree threading based on the new costs. Removing nodes could + // cause a subtree to get evaluated first (earlier second) during the + // liveness walk. Instead just set a flag that costs are dirty and + // caller has to call gtSetStmtInfo. + *pStmtInfoDirty = true; - fgSetStmtSeq(compCurStmt); + fgSetStmtSeq(compCurStmt); - // Continue analysis from this node + // Continue analysis from this node - *pTree = asgNode; + *pTree = asgNode; - return false; - } + return false; } return false; diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 72f57d2bdc173..ffcb0c41af116 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -2226,10 +2226,9 @@ void CallArgs::AddFinalArgsAndDetermineABIInfo(Compiler* comp, GenTreeCall* call assert(arg.GetEarlyNode() != nullptr); GenTree* argx = arg.GetEarlyNode(); - // Change the node to TYP_I_IMPL so we don't report GC info - // NOTE: We deferred this from the importer because of the inliner. - - if (argx->IsLocalAddrExpr() != nullptr) + // TODO-Cleanup: this is duplicative with the code in args morphing, however, also kicks in for + // "non-standard" (return buffer on ARM64) arguments. Fix args morphing and delete this code. + if (argx->OperIsLocalAddr()) { argx->gtType = TYP_I_IMPL; } @@ -3178,9 +3177,9 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* call) } assert(arg.AbiInfo.ByteSize > 0); - // For pointers to locals we can skip reporting GC info and also skip - // zero initialization. - if (argx->IsLocalAddrExpr() != nullptr) + // For pointers to locals we can skip reporting GC info and also skip zero initialization. + // NOTE: We deferred this from the importer because of the inliner. + if (argx->OperIsLocalAddr()) { argx->gtType = TYP_I_IMPL; } @@ -5048,8 +5047,7 @@ GenTree* Compiler::fgMorphField(GenTree* tree, MorphAddrContext* mac) if (tree->OperIs(GT_FIELD)) { - noway_assert(((objRef != nullptr) && (objRef->IsLocalAddrExpr() != nullptr)) || - ((tree->gtFlags & GTF_GLOB_REF) != 0)); + noway_assert(((objRef != nullptr) && objRef->OperIsLocalAddr()) || ((tree->gtFlags & GTF_GLOB_REF) != 0)); } if (fieldNode->IsInstance()) @@ -9829,6 +9827,26 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA tree->gtFlags |= (GTF_IND_INVARIANT | GTF_IND_NONFAULTING | GTF_IND_NONNULL); } + if (!tree->AsIndir()->IsVolatile() && !tree->TypeIs(TYP_STRUCT) && op1->OperIsLocalAddr() && + !optValnumCSE_phase) + { + unsigned loadSize = tree->AsIndir()->Size(); + unsigned offset = op1->AsLclVarCommon()->GetLclOffs(); + unsigned loadExtent = offset + loadSize; + unsigned lclSize = lvaLclExactSize(op1->AsLclVarCommon()->GetLclNum()); + + if ((loadExtent <= lclSize) && (loadExtent < UINT16_MAX)) + { + op1->ChangeType(tree->TypeGet()); + op1->SetOper(GT_LCL_FLD); + op1->AsLclFld()->SetLclOffs(offset); + op1->SetVNsFromNode(tree); + op1->AddAllEffectsFlags(tree->gtFlags & GTF_GLOB_REF); + + return op1; + } + } + #ifdef TARGET_ARM GenTree* effOp1 = op1->gtEffectiveVal(true); // Check for a misalignment floating point indirection. diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp index 2baaa459d9ae1..5e48188912546 100644 --- a/src/coreclr/jit/optimizer.cpp +++ b/src/coreclr/jit/optimizer.cpp @@ -8621,8 +8621,6 @@ bool Compiler::optComputeLoopSideEffectsOfBlock(BasicBlock* blk) { GenTree* lhs = tree->gtGetOp1(); - assert(!lhs->OperIs(GT_COMMA)); - if (lhs->OperIs(GT_IND)) { GenTree* arg = lhs->AsOp()->gtOp1->gtEffectiveVal(/*commaOnly*/ true); @@ -8693,20 +8691,12 @@ bool Compiler::optComputeLoopSideEffectsOfBlock(BasicBlock* blk) } else if (lhs->OperIsBlk()) { - GenTreeLclVarCommon* lclVarTree; - bool isEntire; - if (!tree->DefinesLocal(this, &lclVarTree, &isEntire)) - { - // For now, assume arbitrary side effects on GcHeap/ByrefExposed... - memoryHavoc |= memoryKindSet(GcHeap, ByrefExposed); - } - else if (lvaVarAddrExposed(lclVarTree->GetLclNum())) - { - memoryHavoc |= memoryKindSet(ByrefExposed); - } + // For now, assume arbitrary side effects on GcHeap/ByrefExposed... + // TODO-CQ: delete this pessimization by folding into the above. + memoryHavoc |= memoryKindSet(GcHeap, ByrefExposed); } - // Otherwise, must be local lhs form. I should assert that. - else if (lhs->OperIsLocal()) + // Otherwise, must be local lhs form. + else { GenTreeLclVarCommon* lhsLcl = lhs->AsLclVarCommon(); ValueNum rhsVN = tree->AsOp()->gtOp2->gtVNPair.GetLiberal(); diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 1c9e7460285ed..0a588abae2be4 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -9903,10 +9903,6 @@ void Compiler::fgValueNumberAssignment(GenTreeOp* tree) assert(fldSeq != nullptr); fgValueNumberFieldStore(tree, baseAddr, fldSeq, offset, storeSize, rhsVNPair.GetLiberal()); } - else if (addr->DefinesLocalAddr(&lclVarTree, &offset)) - { - fgValueNumberLocalStore(tree, lclVarTree, offset, storeSize, rhsVNPair); - } else { assert(!tree->DefinesLocal(this, &lclVarTree)); @@ -10460,11 +10456,10 @@ 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; - FieldSeq* fldSeq = nullptr; - GenTree* baseAddr = nullptr; - bool isVolatile = (tree->gtFlags & GTF_IND_VOLATILE) != 0; + GenTree* addr = tree->AsIndir()->Addr(); + FieldSeq* fldSeq = nullptr; + GenTree* baseAddr = nullptr; + bool isVolatile = (tree->gtFlags & GTF_IND_VOLATILE) != 0; // See if the addr has any exceptional part. ValueNumPair addrNvnp; @@ -10561,13 +10556,6 @@ void Compiler::fgValueNumberTree(GenTree* tree) { tree->gtVNPair.SetBoth(vnStore->VNForExpr(compCurBB, loadType)); } - else if (addr->DefinesLocalAddr(&lclVarTree, &offset) && lclVarTree->HasSsaName()) - { - ValueNumPair lclVNPair = lvaGetDesc(lclVarTree)->GetPerSsaData(lclVarTree->GetSsaNum())->m_vnPair; - unsigned lclSize = lvaLclExactSize(lclVarTree->GetLclNum()); - - tree->gtVNPair = vnStore->VNPairForLoad(lclVNPair, lclSize, tree->TypeGet(), offset, loadSize); - } else if (tree->OperIs(GT_IND, GT_BLK, GT_OBJ) && fgValueNumberConstLoad(tree->AsIndir())) { // VN is assigned inside fgValueNumberConstLoad