diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 8ab406b2a52bc1..80670b20835cac 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -7362,7 +7362,7 @@ class Compiler LclNumToLiveDefsMap* curSsaName); void optBlockCopyPropPopStacks(BasicBlock* block, LclNumToLiveDefsMap* curSsaName); bool optBlockCopyProp(BasicBlock* block, LclNumToLiveDefsMap* curSsaName); - void optCopyPropPushDef(GenTree* defNode, GenTreeLclVarCommon* lclNode, LclNumToLiveDefsMap* curSsaName); + void optCopyPropPushDef(GenTreeLclVarCommon* lclNode, LclNumToLiveDefsMap* curSsaName); int optCopyProp_LclVarScore(const LclVarDsc* lclVarDsc, const LclVarDsc* copyVarDsc, bool preferOp2); PhaseStatus optVnCopyProp(); INDEBUG(void optDumpCopyPropStack(LclNumToLiveDefsMap* curSsaName)); diff --git a/src/coreclr/jit/compiler.hpp b/src/coreclr/jit/compiler.hpp index 0f9b1de014ad49..10e0f7b124f537 100644 --- a/src/coreclr/jit/compiler.hpp +++ b/src/coreclr/jit/compiler.hpp @@ -4703,6 +4703,105 @@ GenTree::VisitResult GenTree::VisitOperands(TVisitor visitor) } } +//------------------------------------------------------------------------ +// VisitLocalDefs: Visit locals being defined by this node. +// +// Arguments: +// comp - the compiler instance +// visitor - Functor of type GenTree::VisitResult(LocalDef) +// +// Return Value: +// VisitResult::Abort if the functor aborted; otherwise VisitResult::Continue. +// +// Notes: +// This function is contractually bound to recognize a superset of stores +// that "LocalAddressVisitor" recognizes and transforms, as it is used to +// detect which trees can define tracked locals. +// +template +GenTree::VisitResult GenTree::VisitLocalDefs(Compiler* comp, TVisitor visitor) +{ + if (OperIs(GT_STORE_LCL_VAR)) + { + unsigned size = comp->lvaLclExactSize(AsLclVarCommon()->GetLclNum()); + return visitor(LocalDef(AsLclVarCommon(), /* isEntire */ true, 0, size)); + } + if (OperIs(GT_STORE_LCL_FLD)) + { + GenTreeLclFld* fld = AsLclFld(); + return visitor(LocalDef(fld, !fld->IsPartialLclFld(comp), fld->GetLclOffs(), fld->GetSize())); + } + if (OperIs(GT_CALL)) + { + GenTreeLclVarCommon* lclAddr = comp->gtCallGetDefinedRetBufLclAddr(AsCall()); + if (lclAddr != nullptr) + { + unsigned storeSize = comp->typGetObjLayout(AsCall()->gtRetClsHnd)->GetSize(); + + bool isEntire = storeSize == comp->lvaLclExactSize(lclAddr->GetLclNum()); + + return visitor(LocalDef(lclAddr, isEntire, lclAddr->GetLclOffs(), storeSize)); + } + } + + return VisitResult::Continue; +} + +//------------------------------------------------------------------------ +// VisitLocalDefNodes: Visit GenTreeLclVarCommon nodes representing definitions in the specified node. +// +// Arguments: +// comp - the compiler instance +// visitor - Functor of type GenTree::VisitResult(GenTreeLclVarCommon*) +// +// Return Value: +// VisitResult::Abort if the functor aborted; otherwise VisitResult::Continue. +// +// Notes: +// This function is contractually bound to recognize a superset of stores +// that "LocalAddressVisitor" recognizes and transforms, as it is used to +// detect which trees can define tracked locals. +// +template +GenTree::VisitResult GenTree::VisitLocalDefNodes(Compiler* comp, TVisitor visitor) +{ + if (OperIs(GT_STORE_LCL_VAR)) + { + return visitor(AsLclVarCommon()); + } + if (OperIs(GT_STORE_LCL_FLD)) + { + return visitor(AsLclFld()); + } + if (OperIs(GT_CALL)) + { + GenTreeLclVarCommon* lclAddr = comp->gtCallGetDefinedRetBufLclAddr(AsCall()); + if (lclAddr != nullptr) + { + return visitor(lclAddr); + } + } + + return VisitResult::Continue; +} + +//------------------------------------------------------------------------ +// HasAnyLocalDefs: +// Check if a tree is considered as defining any locals. +// +// Arguments: +// comp - the compiler instance +// +// Return Value: +// True if it is. +// +inline bool GenTree::HasAnyLocalDefs(Compiler* comp) +{ + return VisitLocalDefNodes(comp, [](GenTreeLclVarCommon* lcl) { + return GenTree::VisitResult::Abort; + }) == GenTree::VisitResult::Abort; +} + /***************************************************************************** * operator new * diff --git a/src/coreclr/jit/copyprop.cpp b/src/coreclr/jit/copyprop.cpp index 93ff8bd2676127..22ebf6a8791346 100644 --- a/src/coreclr/jit/copyprop.cpp +++ b/src/coreclr/jit/copyprop.cpp @@ -46,24 +46,31 @@ void Compiler::optBlockCopyPropPopStacks(BasicBlock* block, LclNumToLiveDefsMap* { for (GenTree* const tree : stmt->TreeList()) { - GenTreeLclVarCommon* lclDefNode = nullptr; - if (tree->OperIsSsaDef() && tree->DefinesLocal(this, &lclDefNode)) + if (!tree->OperIsSsaDef()) { - if (lclDefNode->HasCompositeSsaName()) + continue; + } + + auto visitDef = [=](GenTreeLclVarCommon* lcl) { + if (lcl->HasCompositeSsaName()) { - LclVarDsc* varDsc = lvaGetDesc(lclDefNode); + LclVarDsc* varDsc = lvaGetDesc(lcl); assert(varDsc->lvPromoted); for (unsigned index = 0; index < varDsc->lvFieldCnt; index++) { - popDef(varDsc->lvFieldLclStart + index, lclDefNode->GetSsaNum(this, index)); + popDef(varDsc->lvFieldLclStart + index, lcl->GetSsaNum(this, index)); } } else { - popDef(lclDefNode->GetLclNum(), lclDefNode->GetSsaNum()); + popDef(lcl->GetLclNum(), lcl->GetSsaNum()); } - } + + return GenTree::VisitResult::Continue; + }; + + tree->VisitLocalDefNodes(this, visitDef); } } } @@ -304,11 +311,10 @@ bool Compiler::optCopyProp( // optCopyPropPushDef: Push the new live SSA def on the stack for "lclNode". // // Arguments: -// defNode - The definition node for this def (store/GT_CALL) (will be "nullptr" for "use" defs) // lclNode - The local tree representing "the def" // curSsaName - The map of local numbers to stacks of their defs // -void Compiler::optCopyPropPushDef(GenTree* defNode, GenTreeLclVarCommon* lclNode, LclNumToLiveDefsMap* curSsaName) +void Compiler::optCopyPropPushDef(GenTreeLclVarCommon* lclNode, LclNumToLiveDefsMap* curSsaName) { unsigned lclNum = lclNode->GetLclNum(); @@ -400,10 +406,14 @@ bool Compiler::optBlockCopyProp(BasicBlock* block, LclNumToLiveDefsMap* curSsaNa { treeLifeUpdater.UpdateLife(tree); - GenTreeLclVarCommon* lclDefNode = nullptr; - if (tree->OperIsSsaDef() && tree->DefinesLocal(this, &lclDefNode)) + if (tree->OperIsSsaDef()) { - optCopyPropPushDef(tree, lclDefNode, curSsaName); + auto visitDef = [=](GenTreeLclVarCommon* lcl) { + optCopyPropPushDef(lcl, curSsaName); + return GenTree::VisitResult::Continue; + }; + + tree->VisitLocalDefNodes(this, visitDef); } else if (tree->OperIs(GT_LCL_VAR, GT_LCL_FLD) && tree->AsLclVarCommon()->HasSsaName()) { @@ -413,7 +423,7 @@ bool Compiler::optBlockCopyProp(BasicBlock* block, LclNumToLiveDefsMap* curSsaNa // live definition. Since they are always live, we'll do it only once. if ((lvaGetDesc(lclNum)->lvIsParam || (lclNum == info.compThisArg)) && !curSsaName->Lookup(lclNum)) { - optCopyPropPushDef(nullptr, tree->AsLclVarCommon(), curSsaName); + optCopyPropPushDef(tree->AsLclVarCommon(), curSsaName); } // TODO-Review: EH successor/predecessor iteration seems broken. diff --git a/src/coreclr/jit/fgdiagnostic.cpp b/src/coreclr/jit/fgdiagnostic.cpp index 50145f04b32b81..29f3b356c0ca2e 100644 --- a/src/coreclr/jit/fgdiagnostic.cpp +++ b/src/coreclr/jit/fgdiagnostic.cpp @@ -4326,61 +4326,58 @@ class SsaCheckVisitor : public GenTreeVisitor void ProcessDefs(GenTree* tree) { - GenTreeLclVarCommon* lclNode; - bool isFullDef = false; - ssize_t offset = 0; - unsigned storeSize = 0; - bool definesLocal = tree->DefinesLocal(m_compiler, &lclNode, &isFullDef, &offset, &storeSize); - - if (!definesLocal) - { - return; - } - - const bool isUse = (lclNode->gtFlags & GTF_VAR_USEASG) != 0; - unsigned const lclNum = lclNode->GetLclNum(); - LclVarDsc* const varDsc = m_compiler->lvaGetDesc(lclNum); + auto visitDef = [=](const LocalDef& def) { + const bool isUse = (def.Def->gtFlags & GTF_VAR_USEASG) != 0; + unsigned const lclNum = def.Def->GetLclNum(); + LclVarDsc* const varDsc = m_compiler->lvaGetDesc(lclNum); - assert(!(isFullDef && isUse)); + assert(!(def.IsEntire && isUse)); - if (lclNode->HasCompositeSsaName()) - { - for (unsigned index = 0; index < varDsc->lvFieldCnt; index++) + if (def.Def->HasCompositeSsaName()) { - unsigned const fieldLclNum = varDsc->lvFieldLclStart + index; - LclVarDsc* const fieldVarDsc = m_compiler->lvaGetDesc(fieldLclNum); - unsigned const fieldSsaNum = lclNode->GetSsaNum(m_compiler, index); - - ssize_t fieldStoreOffset; - unsigned fieldStoreSize; - if (m_compiler->gtStoreDefinesField(fieldVarDsc, offset, storeSize, &fieldStoreOffset, &fieldStoreSize)) + for (unsigned index = 0; index < varDsc->lvFieldCnt; index++) { - ProcessDef(lclNode, fieldLclNum, fieldSsaNum); - - if (!ValueNumStore::LoadStoreIsEntire(genTypeSize(fieldVarDsc), fieldStoreOffset, fieldStoreSize)) + unsigned const fieldLclNum = varDsc->lvFieldLclStart + index; + LclVarDsc* const fieldVarDsc = m_compiler->lvaGetDesc(fieldLclNum); + unsigned const fieldSsaNum = def.Def->GetSsaNum(m_compiler, index); + + ssize_t fieldStoreOffset; + unsigned fieldStoreSize; + if (m_compiler->gtStoreDefinesField(fieldVarDsc, def.Offset, def.Size, &fieldStoreOffset, + &fieldStoreSize)) { - assert(isUse); - unsigned const fieldUseSsaNum = fieldVarDsc->GetPerSsaData(fieldSsaNum)->GetUseDefSsaNum(); - ProcessUse(lclNode, fieldLclNum, fieldUseSsaNum); + ProcessDef(def.Def, fieldLclNum, fieldSsaNum); + + if (!ValueNumStore::LoadStoreIsEntire(genTypeSize(fieldVarDsc), fieldStoreOffset, + fieldStoreSize)) + { + assert(isUse); + unsigned const fieldUseSsaNum = fieldVarDsc->GetPerSsaData(fieldSsaNum)->GetUseDefSsaNum(); + ProcessUse(def.Def, fieldLclNum, fieldUseSsaNum); + } } } } - } - else - { - unsigned const ssaNum = lclNode->GetSsaNum(); - ProcessDef(lclNode, lclNum, ssaNum); - - if (isUse) + else { - unsigned useSsaNum = SsaConfig::RESERVED_SSA_NUM; - if (ssaNum != SsaConfig::RESERVED_SSA_NUM) + unsigned const ssaNum = def.Def->GetSsaNum(); + ProcessDef(def.Def, lclNum, ssaNum); + + if (isUse) { - useSsaNum = varDsc->GetPerSsaData(ssaNum)->GetUseDefSsaNum(); + unsigned useSsaNum = SsaConfig::RESERVED_SSA_NUM; + if (ssaNum != SsaConfig::RESERVED_SSA_NUM) + { + useSsaNum = varDsc->GetPerSsaData(ssaNum)->GetUseDefSsaNum(); + } + ProcessUse(def.Def, lclNum, useSsaNum); } - ProcessUse(lclNode, lclNum, useSsaNum); } - } + + return GenTree::VisitResult::Continue; + }; + + tree->VisitLocalDefs(m_compiler, visitDef); } void ProcessUse(GenTreeLclVarCommon* tree, unsigned lclNum, unsigned ssaNum) diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index 4db0f7b1ccbce1..43575b8c2c495f 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/src/coreclr/jit/flowgraph.cpp @@ -5391,11 +5391,13 @@ bool FlowGraphNaturalLoop::VisitDefs(TFunc func) return Compiler::WALK_SKIP_SUBTREES; } - GenTreeLclVarCommon* lclDef; - if (tree->DefinesLocal(m_compiler, &lclDef)) + auto visitDef = [=](GenTreeLclVarCommon* lcl) { + return m_func(lcl) ? GenTree::VisitResult::Continue : GenTree::VisitResult::Abort; + }; + + if (tree->VisitLocalDefNodes(m_compiler, visitDef) == GenTree::VisitResult::Abort) { - if (!m_func(lclDef)) - return Compiler::WALK_ABORT; + return Compiler::WALK_ABORT; } return Compiler::WALK_CONTINUE; diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 489e5d08c70808..d78d867e99c04a 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -17895,105 +17895,6 @@ bool GenTree::IsPartialLclFld(Compiler* comp) (comp->lvaGetDesc(AsLclFld())->lvExactSize() != AsLclFld()->GetSize()); } -//------------------------------------------------------------------------ -// DefinesLocal: Does "this" define a local? -// -// Arguments: -// comp - the compiler instance -// pLclVarTree - [out] parameter for the local representing the definition -// pIsEntire - optional [out] parameter for whether the store represents -// 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 possibly tracked local variable -// before rationalization. -// -// Notes: -// This function is contractually bound to recognize a superset of stores -// 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)); - - if (OperIs(GT_STORE_LCL_VAR)) - { - *pLclVarTree = AsLclVarCommon(); - if (pIsEntire != nullptr) - { - *pIsEntire = true; - } - if (pOffset != nullptr) - { - *pOffset = 0; - } - if (pSize != nullptr) - { - *pSize = comp->lvaLclExactSize(AsLclVarCommon()->GetLclNum()); - } - - return true; - } - if (OperIs(GT_STORE_LCL_FLD)) - { - *pLclVarTree = AsLclVarCommon(); - if (pIsEntire != nullptr) - { - *pIsEntire = !AsLclFld()->IsPartialLclFld(comp); - } - if (pOffset != nullptr) - { - *pOffset = AsLclFld()->GetLclOffs(); - } - if (pSize != nullptr) - { - *pSize = AsLclFld()->GetSize(); - } - - return true; - } - if (OperIs(GT_CALL)) - { - GenTreeLclVarCommon* lclAddr = comp->gtCallGetDefinedRetBufLclAddr(AsCall()); - if (lclAddr == nullptr) - { - return false; - } - - *pLclVarTree = lclAddr; - - if ((pIsEntire != nullptr) || (pSize != nullptr)) - { - unsigned storeSize = comp->typGetObjLayout(AsCall()->gtRetClsHnd)->GetSize(); - - if (pIsEntire != nullptr) - { - *pIsEntire = storeSize == comp->lvaLclExactSize(lclAddr->GetLclNum()); - } - - if (pSize != nullptr) - { - *pSize = storeSize; - } - } - - if (pOffset != nullptr) - { - *pOffset = lclAddr->GetLclOffs(); - } - - return true; - } - - return false; -} - //------------------------------------------------------------------------ // IsImplicitByrefParameterValuePreMorph: // Determine if this tree represents the value of an implicit byref diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 33690a76ef9169..c618e5b84d1027 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -648,6 +648,22 @@ inline GenTreeDebugFlags& operator &=(GenTreeDebugFlags& a, GenTreeDebugFlags b) // clang-format on +struct LocalDef +{ + GenTreeLclVarCommon* Def; + bool IsEntire; + ssize_t Offset; + unsigned Size; + + LocalDef(GenTreeLclVarCommon* def, bool isEntire, ssize_t offset, unsigned size) + : Def(def) + , IsEntire(isEntire) + , Offset(offset) + , Size(size) + { + } +}; + #ifndef HOST_64BIT #include #endif @@ -696,6 +712,12 @@ struct GenTree #include "gtstructs.h" + enum class VisitResult + { + Abort = false, + Continue = true + }; + genTreeOps gtOper; // enum subtype BYTE var_types gtType; // enum subtype BYTE @@ -2023,11 +2045,13 @@ struct GenTree // is not the same size as the type of the GT_LCL_VAR. bool IsPartialLclFld(Compiler* comp); - bool DefinesLocal(Compiler* comp, - GenTreeLclVarCommon** pLclVarTree, - bool* pIsEntire = nullptr, - ssize_t* pOffset = nullptr, - unsigned* pSize = nullptr); + template + VisitResult VisitLocalDefs(Compiler* comp, TVisitor visitor); + + template + VisitResult VisitLocalDefNodes(Compiler* comp, TVisitor visitor); + + bool HasAnyLocalDefs(Compiler* comp); GenTreeLclVarCommon* IsImplicitByrefParameterValuePreMorph(Compiler* compiler); GenTreeLclVar* IsImplicitByrefParameterValuePostMorph(Compiler* compiler, GenTree** addr, target_ssize_t* offset); @@ -2350,12 +2374,6 @@ struct GenTree // Returns a range that will produce the operands of this node in execution order. IteratorPair Operands(); - enum class VisitResult - { - Abort = false, - Continue = true - }; - // Visits each operand of this node. The operand must be either a lambda, function, or functor with the signature // `GenTree::VisitResult VisitorFunction(GenTree* operand)`. Here is a simple example: // diff --git a/src/coreclr/jit/liveness.cpp b/src/coreclr/jit/liveness.cpp index d51746731b6c8e..08ef18528c1867 100644 --- a/src/coreclr/jit/liveness.cpp +++ b/src/coreclr/jit/liveness.cpp @@ -342,11 +342,11 @@ void Compiler::fgPerNodeLocalVarLiveness(GenTree* tree) } } - GenTreeLclVarCommon* definedLcl = gtCallGetDefinedRetBufLclAddr(call); - if (definedLcl != nullptr) - { - fgMarkUseDef(definedLcl); - } + auto visitDef = [=](GenTreeLclVarCommon* lcl) { + fgMarkUseDef(lcl); + return GenTree::VisitResult::Continue; + }; + call->VisitLocalDefNodes(this, visitDef); break; } @@ -796,13 +796,11 @@ void Compiler::fgLiveVarAnalysis() // call - The call node in question. // // Returns: -// local defined by the call, if any (eg retbuf) +// partially defined local by the call, if any (eg retbuf) // GenTreeLclVarCommon* Compiler::fgComputeLifeCall(VARSET_TP& life, VARSET_VALARG_TP keepAliveVars, GenTreeCall* call) { assert(call != nullptr); - GenTreeLclVarCommon* definedLcl = nullptr; - // If this is a tail-call via helper, and we have any unmanaged p/invoke calls in // the method, then we're going to run the p/invoke epilog // So we mark the FrameRoot as used by this instruction. @@ -861,13 +859,22 @@ GenTreeLclVarCommon* Compiler::fgComputeLifeCall(VARSET_TP& life, VARSET_VALARG_ } } - definedLcl = gtCallGetDefinedRetBufLclAddr(call); - if (definedLcl != nullptr) - { - fgComputeLifeLocal(life, keepAliveVars, definedLcl); - } + GenTreeLclVarCommon* partialDef = nullptr; + + auto visitDef = [&](const LocalDef& def) { + if (!def.IsEntire) + { + assert(partialDef == nullptr); + partialDef = def.Def; + } + + fgComputeLifeLocal(life, keepAliveVars, def.Def); + return GenTree::VisitResult::Continue; + }; + + call->VisitLocalDefs(this, visitDef); - return definedLcl; + return partialDef; } //------------------------------------------------------------------------ @@ -1207,11 +1214,12 @@ void Compiler::fgComputeLife(VARSET_TP& life, if (tree->IsCall()) { - GenTreeLclVarCommon* const definedLcl = fgComputeLifeCall(life, keepAliveVars, tree->AsCall()); - if (definedLcl != nullptr) + GenTreeLclVarCommon* const partialDef = fgComputeLifeCall(life, keepAliveVars, tree->AsCall()); + if (partialDef != nullptr) { - isUse = (definedLcl->gtFlags & GTF_VAR_USEASG) != 0; - varDsc = lvaGetDesc(definedLcl); + assert((partialDef->gtFlags & GTF_VAR_USEASG) != 0); + isUse = true; + varDsc = lvaGetDesc(partialDef); } } else if (tree->OperIsNonPhiLocal()) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 3b8b03c975c45f..6c57e953302c04 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -8636,15 +8636,16 @@ void Lowering::FindInducedParameterRegisterLocals() { hasRegisterKill |= node->IsCall(); - GenTreeLclVarCommon* storeLcl; - if (node->DefinesLocal(comp, &storeLcl)) - { - storedToLocals.Emplace(storeLcl->GetLclNum(), true); - continue; - } + auto visitDefs = [&](GenTreeLclVarCommon* lcl) { + storedToLocals.Emplace(lcl->GetLclNum(), true); + return GenTree::VisitResult::Continue; + }; + + node->VisitLocalDefNodes(comp, visitDefs); if (node->OperIs(GT_LCL_ADDR)) { + // Model these as stored to, since we cannot reason about them in the same way. storedToLocals.Emplace(node->AsLclVarCommon()->GetLclNum(), true); continue; } diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 688787a6761674..02d85faa37797a 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -6824,22 +6824,22 @@ GenTree* Compiler::fgMorphLeaf(GenTree* tree) void Compiler::fgAssignSetVarDef(GenTree* tree) { - GenTreeLclVarCommon* lclVarCmnTree; - bool isEntire = false; - if (tree->DefinesLocal(this, &lclVarCmnTree, &isEntire)) - { - if (isEntire) + auto visitDef = [=](const LocalDef& def) { + if (def.IsEntire) { - lclVarCmnTree->gtFlags |= GTF_VAR_DEF; + def.Def->gtFlags |= GTF_VAR_DEF; } else { // We consider partial definitions to be modeled as uses followed by definitions. // This captures the idea that precedings defs are not necessarily made redundant // by this definition. - lclVarCmnTree->gtFlags |= (GTF_VAR_DEF | GTF_VAR_USEASG); + def.Def->gtFlags |= (GTF_VAR_DEF | GTF_VAR_USEASG); } - } + return GenTree::VisitResult::Continue; + }; + + tree->VisitLocalDefs(this, visitDef); } #ifdef FEATURE_SIMD @@ -12635,10 +12635,14 @@ void Compiler::fgMorphTreeDone(GenTree* tree, bool optAssertionPropDone DEBUGARG // Kill active assertions // - GenTreeLclVarCommon* lclVarTree = nullptr; - if ((optAssertionCount > 0) && tree->DefinesLocal(this, &lclVarTree)) + if (optAssertionCount > 0) { - fgKillDependentAssertions(lclVarTree->GetLclNum() DEBUGARG(tree)); + auto visitDef = [=](GenTreeLclVarCommon* lcl) { + fgKillDependentAssertions(lcl->GetLclNum() DEBUGARG(tree)); + return GenTree::VisitResult::Continue; + }; + + tree->VisitLocalDefNodes(this, visitDef); } // Generate assertions diff --git a/src/coreclr/jit/ssabuilder.cpp b/src/coreclr/jit/ssabuilder.cpp index e7aec078041b29..3cdd3441ac19c1 100644 --- a/src/coreclr/jit/ssabuilder.cpp +++ b/src/coreclr/jit/ssabuilder.cpp @@ -421,25 +421,21 @@ void SsaBuilder::RenameDef(GenTree* defNode, BasicBlock* block) { assert(defNode->OperIsStore() || defNode->OperIs(GT_CALL)); - GenTreeLclVarCommon* lclNode; - bool isFullDef = false; - ssize_t offset = 0; - unsigned storeSize = 0; - bool isLocal = defNode->DefinesLocal(m_pCompiler, &lclNode, &isFullDef, &offset, &storeSize); - - if (isLocal) - { + bool anyDefs = false; + auto visitDef = [&](const LocalDef& def) { + anyDefs = true; // This should have been marked as definition. - assert(((lclNode->gtFlags & GTF_VAR_DEF) != 0) && (((lclNode->gtFlags & GTF_VAR_USEASG) != 0) == !isFullDef)); + assert(((def.Def->gtFlags & GTF_VAR_DEF) != 0) && + (((def.Def->gtFlags & GTF_VAR_USEASG) != 0) == !def.IsEntire)); - unsigned lclNum = lclNode->GetLclNum(); + unsigned lclNum = def.Def->GetLclNum(); LclVarDsc* varDsc = m_pCompiler->lvaGetDesc(lclNum); if (m_pCompiler->lvaInSsa(lclNum)) { - lclNode->SetSsaNum(RenamePushDef(defNode, block, lclNum, isFullDef)); + def.Def->SetSsaNum(RenamePushDef(defNode, block, lclNum, def.IsEntire)); assert(!varDsc->IsAddressExposed()); // Cannot define SSA memory. - return; + return GenTree::VisitResult::Continue; } if (varDsc->lvPromoted) @@ -455,11 +451,11 @@ void SsaBuilder::RenameDef(GenTree* defNode, BasicBlock* block) unsigned ssaNum = SsaConfig::RESERVED_SSA_NUM; // Fast-path the common case of an "entire" store. - if (isFullDef) + if (def.IsEntire) { ssaNum = RenamePushDef(defNode, block, fieldLclNum, /* defIsFull */ true); } - else if (m_pCompiler->gtStoreDefinesField(fieldVarDsc, offset, storeSize, &fieldStoreOffset, + else if (m_pCompiler->gtStoreDefinesField(fieldVarDsc, def.Offset, def.Size, &fieldStoreOffset, &fieldStoreSize)) { ssaNum = RenamePushDef(defNode, block, fieldLclNum, @@ -469,71 +465,34 @@ void SsaBuilder::RenameDef(GenTree* defNode, BasicBlock* block) if (ssaNum != SsaConfig::RESERVED_SSA_NUM) { - lclNode->SetSsaNum(m_pCompiler, index, ssaNum); + def.Def->SetSsaNum(m_pCompiler, index, ssaNum); } } } } - } - else if (defNode->OperIs(GT_CALL)) - { - // If the current def is a call we either know the call is pure or else has arbitrary memory definitions, - // the memory effect of the call is captured by the live out state from the block and doesn't need special - // handling here. If we ever change liveness to more carefully model call effects (from interprecedural - // information) we might need to revisit this. - return; - } - // Figure out if "defNode" may make a new GC heap state (if we care for this block). - if (((block->bbMemoryHavoc & memoryKindSet(GcHeap)) == 0) && m_pCompiler->ehBlockHasExnFlowDsc(block)) - { - bool isAddrExposedLocal = isLocal && m_pCompiler->lvaVarAddrExposed(lclNode->GetLclNum()); - bool hasByrefHavoc = ((block->bbMemoryHavoc & memoryKindSet(ByrefExposed)) != 0); - if (!isLocal || (isAddrExposedLocal && !hasByrefHavoc)) + if (varDsc->IsAddressExposed()) { - // It *may* define byref memory in a non-havoc way. Make a new SSA # -- associate with this node. - unsigned ssaNum = m_pCompiler->lvMemoryPerSsaData.AllocSsaNum(m_allocator); - if (!hasByrefHavoc) - { - m_renameStack.PushMemory(ByrefExposed, block, ssaNum); - m_pCompiler->GetMemorySsaMap(ByrefExposed)->Set(defNode, ssaNum); -#ifdef DEBUG - if (m_pCompiler->verboseSsa) - { - printf("Node "); - Compiler::printTreeID(defNode); - printf(" (in try block) may define memory; ssa # = %d.\n", ssaNum); - } -#endif // DEBUG + RenamePushMemoryDef(def.Def, block); + } - // Now add this SSA # to all phis of the reachable catch blocks. - AddMemoryDefToEHSuccessorPhis(ByrefExposed, block, ssaNum); - } + return GenTree::VisitResult::Continue; + }; - if (!isLocal) - { - // Add a new def for GcHeap as well - if (m_pCompiler->byrefStatesMatchGcHeapStates) - { - // GcHeap and ByrefExposed share the same stacks, SsaMap, and phis - assert(!hasByrefHavoc); - assert(*m_pCompiler->GetMemorySsaMap(GcHeap)->LookupPointer(defNode) == ssaNum); - assert(block->bbMemorySsaPhiFunc[GcHeap] == block->bbMemorySsaPhiFunc[ByrefExposed]); - } - else - { - if (!hasByrefHavoc) - { - // Allocate a distinct defnum for the GC Heap - ssaNum = m_pCompiler->lvMemoryPerSsaData.AllocSsaNum(m_allocator); - } + defNode->VisitLocalDefs(m_pCompiler, visitDef); - m_renameStack.PushMemory(GcHeap, block, ssaNum); - m_pCompiler->GetMemorySsaMap(GcHeap)->Set(defNode, ssaNum); - AddMemoryDefToEHSuccessorPhis(GcHeap, block, ssaNum); - } - } + if (!anyDefs) + { + if (defNode->OperIs(GT_CALL)) + { + // If the current def is a call we either know the call is pure or else has arbitrary memory definitions, + // the memory effect of the call is captured by the live out state from the block and doesn't need special + // handling here. If we ever change liveness to more carefully model call effects (from interprecedural + // information) we might need to revisit this. + return; } + + RenamePushMemoryDef(defNode, block); } } @@ -583,6 +542,66 @@ unsigned SsaBuilder::RenamePushDef(GenTree* defNode, BasicBlock* block, unsigned return ssaNum; } +void SsaBuilder::RenamePushMemoryDef(GenTree* defNode, BasicBlock* block) +{ + // Figure out if "defNode" may make a new GC heap state (if we care for this block). + if (((block->bbMemoryHavoc & memoryKindSet(GcHeap)) != 0) || !m_pCompiler->ehBlockHasExnFlowDsc(block)) + { + return; + } + + bool hasByrefHavoc = ((block->bbMemoryHavoc & memoryKindSet(ByrefExposed)) != 0); + + if (defNode->OperIsAnyLocal() && hasByrefHavoc) + { + // No need to record these. + return; + } + + // It *may* define byref memory in a non-havoc way. Make a new SSA # -- associate with this node. + unsigned ssaNum = m_pCompiler->lvMemoryPerSsaData.AllocSsaNum(m_allocator); + if (!hasByrefHavoc) + { + m_renameStack.PushMemory(ByrefExposed, block, ssaNum); + m_pCompiler->GetMemorySsaMap(ByrefExposed)->Set(defNode, ssaNum); +#ifdef DEBUG + if (m_pCompiler->verboseSsa) + { + printf("Node "); + Compiler::printTreeID(defNode); + printf(" (in try block) may define memory; ssa # = %d.\n", ssaNum); + } +#endif // DEBUG + + // Now add this SSA # to all phis of the reachable catch blocks. + AddMemoryDefToEHSuccessorPhis(ByrefExposed, block, ssaNum); + } + + if (!defNode->OperIsAnyLocal()) + { + // Add a new def for GcHeap as well + if (m_pCompiler->byrefStatesMatchGcHeapStates) + { + // GcHeap and ByrefExposed share the same stacks, SsaMap, and phis + assert(!hasByrefHavoc); + assert(*m_pCompiler->GetMemorySsaMap(GcHeap)->LookupPointer(defNode) == ssaNum); + assert(block->bbMemorySsaPhiFunc[GcHeap] == block->bbMemorySsaPhiFunc[ByrefExposed]); + } + else + { + if (!hasByrefHavoc) + { + // Allocate a distinct defnum for the GC Heap + ssaNum = m_pCompiler->lvMemoryPerSsaData.AllocSsaNum(m_allocator); + } + + m_renameStack.PushMemory(GcHeap, block, ssaNum); + m_pCompiler->GetMemorySsaMap(GcHeap)->Set(defNode, ssaNum); + AddMemoryDefToEHSuccessorPhis(GcHeap, block, ssaNum); + } + } +} + //------------------------------------------------------------------------ // RenameLclUse: Rename a use of a local variable. // diff --git a/src/coreclr/jit/ssabuilder.h b/src/coreclr/jit/ssabuilder.h index fd1ea275bb99cc..70f0cb23832eb7 100644 --- a/src/coreclr/jit/ssabuilder.h +++ b/src/coreclr/jit/ssabuilder.h @@ -81,6 +81,7 @@ class SsaBuilder // Rename a local or memory definition generated by a local store/GT_CALL node. void RenameDef(GenTree* defNode, BasicBlock* block); unsigned RenamePushDef(GenTree* defNode, BasicBlock* block, unsigned lclNum, bool isFullDef); + void RenamePushMemoryDef(GenTree* defNode, BasicBlock* block); // Rename a use of a local variable. void RenameLclUse(GenTreeLclVarCommon* lclNode, BasicBlock* block); diff --git a/src/coreclr/jit/treelifeupdater.cpp b/src/coreclr/jit/treelifeupdater.cpp index a8f7408b182373..5f77e0598aa463 100644 --- a/src/coreclr/jit/treelifeupdater.cpp +++ b/src/coreclr/jit/treelifeupdater.cpp @@ -295,23 +295,21 @@ void TreeLifeUpdater::UpdateLife(GenTree* tree) } // Note that after lowering, we can see indirect uses and definitions of tracked variables. - GenTreeLclVarCommon* lclVarTree = nullptr; if (tree->OperIsNonPhiLocal()) { - lclVarTree = tree->AsLclVarCommon(); + UpdateLifeVar(tree, tree->AsLclVarCommon()); } else if (tree->OperIsIndir() && tree->AsIndir()->Addr()->OperIs(GT_LCL_ADDR)) { - lclVarTree = tree->AsIndir()->Addr()->AsLclVarCommon(); + UpdateLifeVar(tree, tree->AsIndir()->Addr()->AsLclVarCommon()); } else if (tree->IsCall()) { - lclVarTree = compiler->gtCallGetDefinedRetBufLclAddr(tree->AsCall()); - } - - if (lclVarTree != nullptr) - { - UpdateLifeVar(tree, lclVarTree); + auto visitDef = [=](GenTreeLclVarCommon* lcl) { + UpdateLifeVar(tree, lcl); + return GenTree::VisitResult::Continue; + }; + tree->VisitLocalDefNodes(compiler, visitDef); } } diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 19a34dba7f6bf6..287289e2cac6e5 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -11954,7 +11954,7 @@ void Compiler::fgValueNumberStore(GenTree* store) } else { - assert(!store->DefinesLocal(this, &lclVarTree)); + assert(!store->HasAnyLocalDefs(this)); // If it doesn't define a local, then it might update GcHeap/ByrefExposed. // For the new ByrefExposed VN, we could use an operator here like // VNF_ByrefExposedStore that carries the VNs of the pointer and RHS, then @@ -13809,18 +13809,17 @@ void Compiler::fgValueNumberCall(GenTreeCall* call) } } - // If the call generates a definition, because it uses "return buffer", then VN the local + // If the call generates any definitions, for example because it uses "return buffer", then VN the local // as well. - GenTreeLclVarCommon* lclVarTree = nullptr; - ssize_t offset = 0; - unsigned storeSize = 0; - if (call->DefinesLocal(this, &lclVarTree, /* pIsEntire */ nullptr, &offset, &storeSize)) - { + auto visitDef = [=](const LocalDef& def) { ValueNumPair storeValue; - storeValue.SetBoth(vnStore->VNForExpr(compCurBB, TYP_STRUCT)); + storeValue.SetBoth(vnStore->VNForExpr(compCurBB, lvaGetDesc(def.Def->AsLclVarCommon())->TypeGet())); - fgValueNumberLocalStore(call, lclVarTree, offset, storeSize, storeValue); - } + fgValueNumberLocalStore(call, def.Def, def.Offset, def.Size, storeValue); + return GenTree::VisitResult::Continue; + }; + + call->VisitLocalDefs(this, visitDef); } void Compiler::fgValueNumberCastHelper(GenTreeCall* call)