diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 0f395e7dae899..cfee3b00519f5 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -7384,14 +7384,11 @@ class Compiler typedef ArrayStack GenTreePtrStack; typedef JitHashTable, GenTreePtrStack*> LclNumToGenTreePtrStack; - // Kill set to track variables with intervening definitions. - VARSET_TP optCopyPropKillSet; - // Copy propagation functions. - void optCopyProp(BasicBlock* block, Statement* stmt, GenTree* tree, LclNumToGenTreePtrStack* curSsaName); + void optCopyProp(Statement* stmt, GenTreeLclVarCommon* tree, unsigned lclNum, LclNumToGenTreePtrStack* curSsaName); void optBlockCopyPropPopStacks(BasicBlock* block, LclNumToGenTreePtrStack* curSsaName); void optBlockCopyProp(BasicBlock* block, LclNumToGenTreePtrStack* curSsaName); - unsigned optIsSsaLocal(GenTree* tree); + unsigned optIsSsaLocal(GenTreeLclVarCommon* lclNode); int optCopyProp_LclVarScore(LclVarDsc* lclVarDsc, LclVarDsc* copyVarDsc, bool preferOp2); void optVnCopyProp(); INDEBUG(void optDumpCopyPropStack(LclNumToGenTreePtrStack* curSsaName)); diff --git a/src/coreclr/jit/copyprop.cpp b/src/coreclr/jit/copyprop.cpp index 1c80436902f90..52c765a12dc41 100644 --- a/src/coreclr/jit/copyprop.cpp +++ b/src/coreclr/jit/copyprop.cpp @@ -32,17 +32,16 @@ void Compiler::optBlockCopyPropPopStacks(BasicBlock* block, LclNumToGenTreePtrSt { for (GenTree* const tree : stmt->TreeList()) { - if (!tree->IsLocal()) - { - continue; - } - const unsigned lclNum = optIsSsaLocal(tree); - if (lclNum == BAD_VAR_NUM) - { - continue; - } - if (tree->gtFlags & GTF_VAR_DEF) + GenTreeLclVarCommon* lclDefNode = nullptr; + if (tree->OperIs(GT_ASG) && tree->DefinesLocal(this, &lclDefNode)) { + const unsigned lclNum = optIsSsaLocal(lclDefNode); + + if (lclNum == BAD_VAR_NUM) + { + continue; + } + GenTreePtrStack* stack = nullptr; curSsaName->Lookup(lclNum, &stack); stack->Pop(); @@ -123,97 +122,67 @@ int Compiler::optCopyProp_LclVarScore(LclVarDsc* lclVarDsc, LclVarDsc* copyVarDs // definitions share the same value number. If so, then we can make the replacement. // // Arguments: -// block - Block the tree belongs to // stmt - Statement the tree belongs to -// tree - The tree to perform copy propagation on +// tree - The local tree to perform copy propagation on +// lclNum - The local number of said tree // curSsaName - The map from lclNum to its recently live definitions as a stack -void Compiler::optCopyProp(BasicBlock* block, Statement* stmt, GenTree* tree, LclNumToGenTreePtrStack* curSsaName) +void Compiler::optCopyProp(Statement* stmt, + GenTreeLclVarCommon* tree, + unsigned lclNum, + LclNumToGenTreePtrStack* curSsaName) { - // TODO-Review: EH successor/predecessor iteration seems broken. - if (block->bbCatchTyp == BBCT_FINALLY || block->bbCatchTyp == BBCT_FAULT) - { - return; - } - - // If not local nothing to do. - if (!tree->IsLocal()) - { - return; - } - if (tree->OperGet() == GT_PHI_ARG || tree->OperGet() == GT_LCL_FLD) - { - return; - } - - // Propagate only on uses. - if (tree->gtFlags & GTF_VAR_DEF) - { - return; - } - const unsigned lclNum = optIsSsaLocal(tree); - - // Skip non-SSA variables. - if (lclNum == BAD_VAR_NUM) - { - return; - } - + assert((lclNum != BAD_VAR_NUM) && (optIsSsaLocal(tree) == lclNum) && ((tree->gtFlags & GTF_VAR_DEF) == 0)); assert(tree->gtVNPair.GetConservative() != ValueNumStore::NoVN); for (LclNumToGenTreePtrStack::KeyIterator iter = curSsaName->Begin(); !iter.Equal(curSsaName->End()); ++iter) { unsigned newLclNum = iter.Get(); - GenTree* op = iter.GetValue()->Top(); - // Nothing to do if same. if (lclNum == newLclNum) { continue; } - // Skip variables with assignments embedded in the statement (i.e., with a comma). Because we - // are not currently updating their SSA names as live in the copy-prop pass of the stmt. - if (VarSetOps::IsMember(this, optCopyPropKillSet, lvaTable[newLclNum].lvVarIndex)) - { - continue; - } + LclVarDsc* varDsc = lvaGetDesc(lclNum); + LclVarDsc* newLclVarDsc = lvaGetDesc(newLclNum); + GenTree* newLclDefNode = iter.GetValue()->Top(); // Note that this "def" node can actually be a use (for + // parameters and other use-before-def locals). // Do not copy propagate if the old and new lclVar have different 'doNotEnregister' settings. // This is primarily to avoid copy propagating to IND(ADDR(LCL_VAR)) where the replacement lclVar // is not marked 'lvDoNotEnregister'. // However, in addition, it may not be profitable to propagate a 'doNotEnregister' lclVar to an // existing use of an enregisterable lclVar. - - if (lvaTable[lclNum].lvDoNotEnregister != lvaTable[newLclNum].lvDoNotEnregister) + if (varDsc->lvDoNotEnregister != newLclVarDsc->lvDoNotEnregister) { continue; } - if (op->gtFlags & GTF_VAR_CAST) + if ((gsShadowVarInfo != nullptr) && newLclVarDsc->lvIsParam && + (gsShadowVarInfo[newLclNum].shadowCopy == lclNum)) { continue; } - if (gsShadowVarInfo != nullptr && lvaTable[newLclNum].lvIsParam && - gsShadowVarInfo[newLclNum].shadowCopy == lclNum) - { - continue; - } - ValueNum opVN = GetUseAsgDefVNOrTreeVN(op); - if (opVN == ValueNumStore::NoVN) + + ValueNum newLclDefVN = GetUseAsgDefVNOrTreeVN(newLclDefNode); + if (newLclDefVN == ValueNumStore::NoVN) { continue; } - if (op->TypeGet() != tree->TypeGet()) + + if (newLclDefNode->TypeGet() != tree->TypeGet()) { continue; } - if (opVN != tree->gtVNPair.GetConservative()) + + if (newLclDefVN != tree->gtVNPair.GetConservative()) { continue; } - if (optCopyProp_LclVarScore(lvaGetDesc(lclNum), lvaGetDesc(newLclNum), true) <= 0) + + if (optCopyProp_LclVarScore(varDsc, newLclVarDsc, true) <= 0) { continue; } @@ -230,34 +199,25 @@ void Compiler::optCopyProp(BasicBlock* block, Statement* stmt, GenTree* tree, Lc // node x2 = phi(x0, x1) which can then be used to substitute 'c' with. But because of pruning // there would be no such phi node. To solve this we'll check if 'x' is live, before replacing // 'c' with 'x.' - if (!lvaTable[newLclNum].lvVerTypeInfo.IsThisPtr()) - { - if (lvaTable[newLclNum].IsAddressExposed()) - { - continue; - } - // We compute liveness only on tracked variables. So skip untracked locals. - if (!lvaTable[newLclNum].lvTracked) - { - continue; - } + // We compute liveness only on tracked variables. And all SSA locals are tracked. + assert(lvaGetDesc(newLclNum)->lvTracked); - // Because of this dependence on live variable analysis, CopyProp phase is immediately - // after Liveness, SSA and VN. - if (!VarSetOps::IsMember(this, compCurLife, lvaTable[newLclNum].lvVarIndex)) - { - continue; - } + // Because of this dependence on live variable analysis, CopyProp phase is immediately + // after Liveness, SSA and VN. + if ((newLclNum != info.compThisArg) && !VarSetOps::IsMember(this, compCurLife, newLclVarDsc->lvVarIndex)) + { + continue; } + unsigned newSsaNum = SsaConfig::RESERVED_SSA_NUM; - if (op->gtFlags & GTF_VAR_DEF) + if (newLclDefNode->gtFlags & GTF_VAR_DEF) { - newSsaNum = GetSsaNumForLocalVarDef(op); + newSsaNum = GetSsaNumForLocalVarDef(newLclDefNode); } else // parameters, this pointer etc. { - newSsaNum = op->AsLclVarCommon()->GetSsaNum(); + newSsaNum = newLclDefNode->AsLclVarCommon()->GetSsaNum(); } if (newSsaNum == SsaConfig::RESERVED_SSA_NUM) @@ -271,48 +231,42 @@ void Compiler::optCopyProp(BasicBlock* block, Statement* stmt, GenTree* tree, Lc JITDUMP("VN based copy assertion for "); printTreeID(tree); printf(" V%02d " FMT_VN " by ", lclNum, tree->GetVN(VNK_Conservative)); - printTreeID(op); - printf(" V%02d " FMT_VN ".\n", newLclNum, op->GetVN(VNK_Conservative)); - gtDispTree(tree, nullptr, nullptr, true); + printTreeID(newLclDefNode); + printf(" V%02d " FMT_VN ".\n", newLclNum, newLclDefNode->GetVN(VNK_Conservative)); + DISPNODE(tree); } #endif tree->AsLclVarCommon()->SetLclNum(newLclNum); tree->AsLclVarCommon()->SetSsaNum(newSsaNum); gtUpdateSideEffects(stmt, tree); + #ifdef DEBUG if (verbose) { printf("copy propagated to:\n"); - gtDispTree(tree, nullptr, nullptr, true); + DISPNODE(tree); } #endif break; } - return; } //------------------------------------------------------------------------------ // optIsSsaLocal : helper to check if the tree is a local that participates in SSA numbering. // // Arguments: -// tree - The tree to perform the check on; +// lclNode - The local tree to perform the check on; // // Returns: // - lclNum if the local is participating in SSA; // - fieldLclNum if the parent local can be replaced by its only field; // - BAD_VAR_NUM otherwise. // -unsigned Compiler::optIsSsaLocal(GenTree* tree) +unsigned Compiler::optIsSsaLocal(GenTreeLclVarCommon* lclNode) { - if (!tree->IsLocal()) - { - return BAD_VAR_NUM; - } - - GenTreeLclVarCommon* lclNode = tree->AsLclVarCommon(); - unsigned lclNum = lclNode->GetLclNum(); - LclVarDsc* varDsc = lvaGetDesc(lclNum); + unsigned lclNum = lclNode->GetLclNum(); + LclVarDsc* varDsc = lvaGetDesc(lclNum); if (!lvaInSsa(lclNum) && varDsc->CanBeReplacedWithItsField(this)) { @@ -355,68 +309,63 @@ void Compiler::optBlockCopyProp(BasicBlock* block, LclNumToGenTreePtrStack* curS VarSetOps::Assign(this, compCurLife, block->bbLiveIn); for (Statement* const stmt : block->Statements()) { - VarSetOps::ClearD(this, optCopyPropKillSet); - // Walk the tree to find if any local variable can be replaced with current live definitions. + // Simultaneously, push live definitions on the stack - that logic must be in sync with the + // SSA renaming process. for (GenTree* const tree : stmt->TreeList()) { treeLifeUpdater.UpdateLife(tree); - optCopyProp(block, stmt, tree, curSsaName); - - // TODO-Review: Merge this loop with the following loop to correctly update the - // live SSA num while also propagating copies. - // - // 1. This loop performs copy prop with currently live (on-top-of-stack) SSA num. - // 2. The subsequent loop maintains a stack for each lclNum with - // currently active SSA numbers when definitions are encountered. - // - // If there is an embedded definition using a "comma" in a stmt, then the currently - // live SSA number will get updated only in the next loop (2). However, this new - // definition is now supposed to be live (on tos). If we did not update the stacks - // using (2), copy prop (1) will use a SSA num defined outside the stmt ignoring the - // embedded update. Killing the variable is a simplification to produce 0 ASM diffs - // for an update release. - // - const unsigned lclNum = optIsSsaLocal(tree); - if ((lclNum != BAD_VAR_NUM) && (tree->gtFlags & GTF_VAR_DEF)) + GenTreeLclVarCommon* lclDefNode = nullptr; + if (tree->OperIs(GT_ASG) && tree->DefinesLocal(this, &lclDefNode)) { - VarSetOps::AddElemD(this, optCopyPropKillSet, lvaTable[lclNum].lvVarIndex); - } - } + const unsigned lclNum = optIsSsaLocal(lclDefNode); - // This logic must be in sync with SSA renaming process. - for (GenTree* const tree : stmt->TreeList()) - { - const unsigned lclNum = optIsSsaLocal(tree); - if (lclNum == BAD_VAR_NUM) - { - continue; - } + if (lclNum == BAD_VAR_NUM) + { + continue; + } - // As we encounter a definition add it to the stack as a live definition. - if (tree->gtFlags & GTF_VAR_DEF) - { GenTreePtrStack* stack; if (!curSsaName->Lookup(lclNum, &stack)) { stack = new (curSsaName->GetAllocator()) GenTreePtrStack(curSsaName->GetAllocator()); } - stack->Push(tree); + stack->Push(lclDefNode); curSsaName->Set(lclNum, stack, LclNumToGenTreePtrStack::Overwrite); } - // If we encounter first use of a param or this pointer add it as a live definition. - // Since they are always live, do it only once. - else if ((tree->gtOper == GT_LCL_VAR) && !(tree->gtFlags & GTF_VAR_USEASG) && - (lvaTable[lclNum].lvIsParam || lvaTable[lclNum].lvVerTypeInfo.IsThisPtr())) + // TODO-CQ: propagate on LCL_FLDs too. + else if (tree->OperIs(GT_LCL_VAR) && ((tree->gtFlags & GTF_VAR_DEF) == 0)) { - GenTreePtrStack* stack; - if (!curSsaName->Lookup(lclNum, &stack)) + const unsigned lclNum = optIsSsaLocal(tree->AsLclVarCommon()); + + if (lclNum == BAD_VAR_NUM) { - stack = new (curSsaName->GetAllocator()) GenTreePtrStack(curSsaName->GetAllocator()); - stack->Push(tree); - curSsaName->Set(lclNum, stack); + continue; + } + + // If we encounter first use of a param or this pointer add it as a live definition. + // Since they are always live, we'll do it only once. + if (lvaGetDesc(lclNum)->lvIsParam || (lclNum == info.compThisArg)) + { + GenTreePtrStack* stack; + if (!curSsaName->Lookup(lclNum, &stack)) + { + assert(tree->AsLclVarCommon()->GetSsaNum() == SsaConfig::FIRST_SSA_NUM); + + stack = new (curSsaName->GetAllocator()) GenTreePtrStack(curSsaName->GetAllocator()); + stack->Push(tree); + curSsaName->Set(lclNum, stack); + } } + + // TODO-Review: EH successor/predecessor iteration seems broken. + if ((block->bbCatchTyp == BBCT_FINALLY) || (block->bbCatchTyp == BBCT_FAULT)) + { + continue; + } + + optCopyProp(stmt, tree->AsLclVarCommon(), lclNum, curSsaName); } } } @@ -462,7 +411,6 @@ void Compiler::optVnCopyProp() } VarSetOps::AssignNoCopy(this, compCurLife, VarSetOps::MakeEmpty(this)); - VarSetOps::AssignNoCopy(this, optCopyPropKillSet, VarSetOps::MakeEmpty(this)); class CopyPropDomTreeVisitor : public DomTreeVisitor {