diff --git a/src/coreclr/jit/copyprop.cpp b/src/coreclr/jit/copyprop.cpp index 11bce9e358e3d3..97bf549b5877b5 100644 --- a/src/coreclr/jit/copyprop.cpp +++ b/src/coreclr/jit/copyprop.cpp @@ -161,11 +161,23 @@ bool Compiler::optCopyProp( assert((tree->gtFlags & GTF_VAR_DEF) == 0); assert(tree->GetLclNum() == lclNum); - bool madeChanges = false; - LclVarDsc* varDsc = lvaGetDesc(lclNum); - ValueNum lclDefVN = varDsc->GetPerSsaData(tree->GetSsaNum())->m_vnPair.GetConservative(); + bool madeChanges = false; + LclVarDsc* const varDsc = lvaGetDesc(lclNum); + LclSsaVarDsc* const varSsaDsc = varDsc->GetPerSsaData(tree->GetSsaNum()); + GenTree* const varDefTree = varSsaDsc->GetDefNode(); + BasicBlock* const varDefBlock = varSsaDsc->GetBlock(); + ValueNum lclDefVN = varSsaDsc->m_vnPair.GetConservative(); assert(lclDefVN != ValueNumStore::NoVN); + bool const varDefTreeIsPhiDef = (varDefTree != nullptr) && varDefTree->IsPhiDefn(); + bool varDefTreeIsPhiDefAtCycleEntry = false; + + if (varDefTreeIsPhiDef) + { + FlowGraphNaturalLoop* const loop = m_blockToLoop->GetLoop(varDefBlock); + varDefTreeIsPhiDefAtCycleEntry = (loop != nullptr) && (loop->GetHeader() == varDefBlock); + } + for (LclNumToLiveDefsMap::Node* const iter : LclNumToLiveDefsMap::KeyValueIteration(curSsaName)) { unsigned newLclNum = iter->GetKey(); @@ -190,7 +202,129 @@ bool Compiler::optCopyProp( if (newLclDefVN != lclDefVN) { - continue; + // If the definition is a phi def, and is defined in a block that has a backedge, + // we may be able to prove equivalence with some other phi def in that block, despite + // the fact that the VNs do not match. + // + // This happens because VN is one-pass and doesn't know the VNs for backedge PhiArgs, + // so the VNs for phi defs for loop entry blocks is always a novel opaque VN. But + // we can query those backedge VNs after VN is done, and if all Phi Arg VNs match, then + // the two Phi Defs are equivalent and could have had the same VN. + // + bool foundEquivalentPhi = false; + + // Is the new local def from the same block? + // + if (varDefTreeIsPhiDefAtCycleEntry && (varDefBlock == newLclSsaDef->GetBlock())) + { + GenTreeLclVarCommon* const otherTree = newLclSsaDef->GetDefNode(); + + // Is it also a phi definition? Do the types match? + // + if ((otherTree != nullptr) && otherTree->IsPhiDefn() && (otherTree->TypeGet() == tree->TypeGet())) + { + // Are all the Phi Args VN equivalent? + // + GenTreePhi* const treePhi = varDefTree->AsLclVar()->Data()->AsPhi(); + GenTreePhi* const otherPhi = otherTree->AsLclVar()->Data()->AsPhi(); + GenTreePhi::UseIterator treeIter = treePhi->Uses().begin(); + GenTreePhi::UseIterator treeEnd = treePhi->Uses().end(); + GenTreePhi::UseIterator otherIter = otherPhi->Uses().begin(); + GenTreePhi::UseIterator otherEnd = otherPhi->Uses().end(); + + bool phiArgsAreEquivalent = true; + + for (; (treeIter != treeEnd) && (otherIter != otherEnd); ++treeIter, ++otherIter) + { + GenTreePhiArg* const treePhiArg = treeIter->GetNode()->AsPhiArg(); + GenTreePhiArg* const otherPhiArg = otherIter->GetNode()->AsPhiArg(); + + assert(treePhiArg->gtPredBB == otherPhiArg->gtPredBB); + + ValueNum treePhiArgVN = treePhiArg->gtVNPair.GetConservative(); + ValueNum otherPhiArgVN = otherPhiArg->gtVNPair.GetConservative(); + + const bool phiArgIsFromDfsBackedge = m_dfsTree->IsAncestor(varDefBlock, treePhiArg->gtPredBB); + + if ((treePhiArgVN == ValueNumStore::NoVN) && phiArgIsFromDfsBackedge) + { + // Consult SSA defs + // + LclSsaVarDsc* const treePhiArgSsaDef = + lvaGetDesc(treePhiArg)->GetPerSsaData(treePhiArg->GetSsaNum()); + ValueNum treePhiArgSsaDefVN = treePhiArgSsaDef->m_vnPair.GetConservative(); + + // Opportunisitcally update the PhiArgVNs, both to short-circuit future checks here + // in copy prop, and help later phases that also scan phi arg lists (eg RBO). + // (consider doing this as a VN post pass?) + // + if (treePhiArgSsaDefVN != treePhiArgVN) + { + JITDUMP("Updating backedge phi arg [%06u] vn to " FMT_VN "\n", dspTreeID(treePhiArg), + treePhiArgSsaDefVN); + treePhiArg->SetVNs(treePhiArgSsaDef->m_vnPair); + } + + treePhiArgVN = treePhiArgSsaDefVN; + } + + if ((otherPhiArgVN == ValueNumStore::NoVN) && phiArgIsFromDfsBackedge) + { + LclSsaVarDsc* const otherPhiArgSsaDef = + lvaGetDesc(otherPhiArg)->GetPerSsaData(otherPhiArg->GetSsaNum()); + ValueNum otherPhiArgSsaDefVN = otherPhiArgSsaDef->m_vnPair.GetConservative(); + + // Opportunisitcally update the PhiArgVNs, both to short-circuit future checks here + // in copy prop, and help later phases that also scan phi arg lists (eg RBO). + // (consider doing this as a VN post pass?) + // + if (otherPhiArgSsaDefVN != otherPhiArgVN) + { + JITDUMP("Updating backedge phi arg [%06u] vn to " FMT_VN "\n", dspTreeID(otherPhiArg), + otherPhiArgSsaDefVN); + otherPhiArg->SetVNs(otherPhiArgSsaDef->m_vnPair); + } + + otherPhiArgVN = otherPhiArgSsaDefVN; + } + + // If the updated VNs differ, the phis are not equivalent + // + if (treePhiArgVN != otherPhiArgVN) + { + phiArgsAreEquivalent = false; + break; + } + + // If we failed to find meaningful VNs, the phis are not equivalent + // + if (treePhiArgVN == ValueNumStore::NoVN) + { + phiArgsAreEquivalent = false; + break; + } + } + + // If we didn't verify all phi args we have failed to prove equivalence + // + phiArgsAreEquivalent &= (treeIter == treeEnd); + phiArgsAreEquivalent &= (otherIter == otherEnd); + + if (phiArgsAreEquivalent) + { + JITDUMP("Found equivalent phi defs: [%06u] and [%06u]\n", dspTreeID(treePhi), + dspTreeID(otherPhi)); + foundEquivalentPhi = true; + } + } + } + + // VNs differed, and we didn't find phis equivalent, move on to the next candidate. + // + if (!foundEquivalentPhi) + { + continue; + } } // It may not be profitable to propagate a 'doNotEnregister' lclVar to an existing use of an @@ -257,8 +391,27 @@ bool Compiler::optCopyProp( unsigned newSsaNum = newLclVarDsc->GetSsaNumForSsaDef(newLclSsaDef); assert(newSsaNum != SsaConfig::RESERVED_SSA_NUM); - tree->AsLclVarCommon()->SetLclNum(newLclNum); - tree->AsLclVarCommon()->SetSsaNum(newSsaNum); + tree->SetLclNum(newLclNum); + tree->SetSsaNum(newSsaNum); + + // Update VN to match, and propagate up through any enclosing commas. + // (we could in principle try updating through other parents, but + // we lack VN's context for memory, so can't get them all). + // + if (newLclDefVN != lclDefVN) + { + tree->SetVNs(newLclSsaDef->m_vnPair); + GenTree* parent = tree->gtGetParent(nullptr); + + while ((parent != nullptr) && parent->OperIs(GT_COMMA)) + { + JITDUMP(" Updating COMMA parent VN [%06u]\n", dspTreeID(parent)); + ValueNumPair op1Xvnp = vnStore->VNPExceptionSet(parent->AsOp()->gtOp1->gtVNPair); + parent->SetVNs(vnStore->VNPWithExc(parent->AsOp()->gtOp2->gtVNPair, op1Xvnp)); + parent = tree->gtGetParent(nullptr); + } + } + gtUpdateSideEffects(stmt, tree); newLclSsaDef->AddUse(block); @@ -334,12 +487,6 @@ void Compiler::optCopyPropPushDef(GenTree* defNode, GenTreeLclVarCommon* lclNode else if (lclNode->HasSsaName()) { unsigned ssaNum = lclNode->GetSsaNum(); - if ((defNode != nullptr) && defNode->IsPhiDefn()) - { - // TODO-CQ: design better heuristics for propagation and remove this. - ssaNum = SsaConfig::RESERVED_SSA_NUM; - } - pushDef(lclNum, ssaNum); } }