diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 2e5b463c63d8c..ba04b072c3665 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -1466,9 +1466,24 @@ AssertionIndex Compiler::optCreateAssertion(GenTree* op1, assertion.op1.vn = optConservativeNormalVN(op1); assertion.op1.lcl.ssaNum = op1->AsLclVarCommon()->GetSsaNum(); - assert((assertion.op1.lcl.ssaNum == SsaConfig::RESERVED_SSA_NUM) || - (assertion.op1.vn == vnStore->VNConservativeNormalValue( - lvaGetDesc(lclNum)->GetPerSsaData(assertion.op1.lcl.ssaNum)->m_vnPair))); +#ifdef DEBUG + + // If we're ssa based, check that the VN is reasonable. + // + if (assertion.op1.lcl.ssaNum != SsaConfig::RESERVED_SSA_NUM) + { + LclSsaVarDsc* const ssaDsc = lvaGetDesc(lclNum)->GetPerSsaData(assertion.op1.lcl.ssaNum); + + bool doesVNMatch = (assertion.op1.vn == vnStore->VNConservativeNormalValue(ssaDsc->m_vnPair)); + + if (!doesVNMatch && ssaDsc->m_updated) + { + doesVNMatch = (assertion.op1.vn == vnStore->VNConservativeNormalValue(ssaDsc->m_origVNPair)); + } + + assert(doesVNMatch); + } +#endif ssize_t cnsValue = 0; GenTreeFlags iconFlags = GTF_EMPTY; diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 743cd5c4095fd..72d410c98a443 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -312,6 +312,13 @@ class LclSsaVarDsc } ValueNumPair m_vnPair; + +#ifdef DEBUG + // True if this ssa def VN was updated + bool m_updated = false; + // Originally assigned VN + ValueNumPair m_origVNPair; +#endif }; // This class stores information associated with a memory SSA definition. @@ -5749,9 +5756,15 @@ class Compiler // Utility functions for fgValueNumber. + // Value number a block or blocks in a loop + void fgValueNumberBlocks(BasicBlock* block, BlockSet& visitedBlocks); + // Perform value-numbering for the trees in "blk". void fgValueNumberBlock(BasicBlock* blk); + // Value number a phi definition + void fgValueNumberPhiDef(GenTreeLclVar* newSsaDef, BasicBlock* block, bool isUpdate = false); + // Requires that "entryBlock" is the header block of "loop" and that "loop" is the // innermost loop of which "entryBlock" is the entry. Returns the value number that should be // assumed for the memoryKind at the start "entryBlk". diff --git a/src/coreclr/jit/copyprop.cpp b/src/coreclr/jit/copyprop.cpp index 11bce9e358e3d..f3c858bc9bcaa 100644 --- a/src/coreclr/jit/copyprop.cpp +++ b/src/coreclr/jit/copyprop.cpp @@ -161,11 +161,25 @@ 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 const lclDefVN = varSsaDsc->m_vnPair.GetConservative(); assert(lclDefVN != ValueNumStore::NoVN); + // See if this local is a candidate for phi dev equivalence checks + // + 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 +204,15 @@ bool Compiler::optCopyProp( if (newLclDefVN != lclDefVN) { - continue; + bool arePhiDefsEquivalent = + varDefTreeIsPhiDefAtCycleEntry && vnStore->AreVNsEquivalent(lclDefVN, newLclDefVN); + if (!arePhiDefsEquivalent) + { + continue; + } + + JITDUMP("orig [%06u] copy [%06u] VNs proved equivalent\n", dspTreeID(tree), + dspTreeID(newLclDef.GetDefNode())); } // It may not be profitable to propagate a 'doNotEnregister' lclVar to an existing use of an @@ -259,6 +281,24 @@ bool Compiler::optCopyProp( tree->AsLclVarCommon()->SetLclNum(newLclNum); tree->AsLclVarCommon()->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 +374,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); } } diff --git a/src/coreclr/jit/redundantbranchopts.cpp b/src/coreclr/jit/redundantbranchopts.cpp index 9e44637f99190..b12ab304c0857 100644 --- a/src/coreclr/jit/redundantbranchopts.cpp +++ b/src/coreclr/jit/redundantbranchopts.cpp @@ -1628,6 +1628,31 @@ bool Compiler::optJumpThreadCore(JumpThreadInfo& jti) // JITDUMP("Optimizing via jump threading\n"); + bool setNoCseIn = false; + + // If this is a phi-based threading, and the block we're bypassing has + // a memory phi, mark the successor blocks with BBF_NO_CSE_IN so we can + // block unsound CSE propagation. + // + if (jti.m_isPhiBased) + { + for (MemoryKind memoryKind : allMemoryKinds()) + { + if ((memoryKind == ByrefExposed) && byrefStatesMatchGcHeapStates) + { + continue; + } + + if (jti.m_block->bbMemorySsaPhiFunc[memoryKind] != nullptr) + { + JITDUMP(FMT_BB " has %s memory phi; will be marking blocks with BBF_NO_CSE_IN\n", jti.m_block->bbNum, + memoryKindNames[memoryKind]); + setNoCseIn = true; + break; + } + } + } + // Now reroute the flow from the predecessors. // If this pred is in the set that will reuse block, do nothing. // Else revise pred to branch directly to the appropriate successor of block. @@ -1638,6 +1663,11 @@ bool Compiler::optJumpThreadCore(JumpThreadInfo& jti) // if (BlockSetOps::IsMember(this, jti.m_ambiguousPreds, predBlock->bbNum)) { + if (setNoCseIn && !jti.m_block->HasFlag(BBF_NO_CSE_IN)) + { + JITDUMP(FMT_BB " => BBF_NO_CSE_IN\n", jti.m_block->bbNum); + jti.m_block->SetFlags(BBF_NO_CSE_IN); + } continue; } @@ -1652,6 +1682,12 @@ bool Compiler::optJumpThreadCore(JumpThreadInfo& jti) predBlock->bbNum, jti.m_block->bbNum, predBlock->bbNum, jti.m_trueTarget->bbNum); fgReplaceJumpTarget(predBlock, jti.m_block, jti.m_trueTarget); + + if (setNoCseIn && !jti.m_trueTarget->HasFlag(BBF_NO_CSE_IN)) + { + JITDUMP(FMT_BB " => BBF_NO_CSE_IN\n", jti.m_trueTarget->bbNum); + jti.m_trueTarget->SetFlags(BBF_NO_CSE_IN); + } } else { @@ -1660,28 +1696,11 @@ bool Compiler::optJumpThreadCore(JumpThreadInfo& jti) predBlock->bbNum, jti.m_block->bbNum, predBlock->bbNum, jti.m_falseTarget->bbNum); fgReplaceJumpTarget(predBlock, jti.m_block, jti.m_falseTarget); - } - } - // If this is a phi-based threading, and the block we're bypassing has - // a memory phi, mark the block with BBF_NO_CSE_IN so we can block CSE propagation - // into the block. - // - if (jti.m_isPhiBased) - { - for (MemoryKind memoryKind : allMemoryKinds()) - { - if ((memoryKind == ByrefExposed) && byrefStatesMatchGcHeapStates) - { - continue; - } - - if (jti.m_block->bbMemorySsaPhiFunc[memoryKind] != nullptr) + if (setNoCseIn && !jti.m_falseTarget->HasFlag(BBF_NO_CSE_IN)) { - JITDUMP(FMT_BB " has %s memory phi; marking as BBF_NO_CSE_IN\n", jti.m_block->bbNum, - memoryKindNames[memoryKind]); - jti.m_block->SetFlags(BBF_NO_CSE_IN); - break; + JITDUMP(FMT_BB " => BBF_NO_CSE_IN\n", jti.m_falseTarget->bbNum); + jti.m_falseTarget->SetFlags(BBF_NO_CSE_IN); } } } diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 1b795678e7b00..6d4062325fe7e 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -6770,6 +6770,138 @@ const char* ValueNumStore::VNRelationString(VN_RELATION_KIND vrk) } #endif +bool ValueNumStore::IsVNPhiDef(ValueNum vn) +{ + VNFuncApp funcAttr; + if (!GetVNFunc(vn, &funcAttr)) + { + return false; + } + + return funcAttr.m_func == VNF_PhiDef; +} +//------------------------------------------------------------------------ +// AreVNsEquivalent: returns true iff VNs represent the same value +// +// Arguments: +// vn1 - first value number to consider +// vn2 - second value number ot consider +// +// Notes: +// Normally if vn1 != vn2 then we cannot say if the two values +// are equivalent or different. PhiDef VNs don't represent the +// phi def values in this way, and can be proven equivalent even +// for different value numbers. +// +bool ValueNumStore::AreVNsEquivalent(ValueNum vn1, ValueNum vn2) +{ + if (vn1 == vn2) + { + return true; + } + + VNFuncApp funcAttr1; + if (!GetVNFunc(vn1, &funcAttr1)) + { + return false; + } + + if (funcAttr1.m_func != VNF_PhiDef) + { + return false; + } + + VNFuncApp funcAttr2; + if (!GetVNFunc(vn2, &funcAttr2)) + { + return false; + } + + if (funcAttr2.m_func != VNF_PhiDef) + { + return false; + } + + // We have two PhiDefs. They may be equivalent, if + // they come from Phis in the same block. + // + const unsigned lclNum1 = unsigned(funcAttr1.m_args[0]); + const unsigned ssaDefNum1 = unsigned(funcAttr1.m_args[1]); + + LclVarDsc* const varDsc1 = m_pComp->lvaGetDesc(lclNum1); + LclSsaVarDsc* const varSsaDsc1 = varDsc1->GetPerSsaData(ssaDefNum1); + GenTree* const varDefTree1 = varSsaDsc1->GetDefNode(); + BasicBlock* const varDefBlock1 = varSsaDsc1->GetBlock(); + + const unsigned lclNum2 = unsigned(funcAttr2.m_args[0]); + const unsigned ssaDefNum2 = unsigned(funcAttr2.m_args[1]); + + LclVarDsc* const varDsc2 = m_pComp->lvaGetDesc(lclNum2); + LclSsaVarDsc* const varSsaDsc2 = varDsc2->GetPerSsaData(ssaDefNum2); + GenTree* const varDefTree2 = varSsaDsc2->GetDefNode(); + BasicBlock* const varDefBlock2 = varSsaDsc2->GetBlock(); + + if (varDefBlock1 != varDefBlock2) + { + return false; + } + + if ((varDefTree1 == nullptr) || (varDefTree2 == nullptr)) + { + return false; + } + + // PhiDefs are from same block. Walk the phi args + // + GenTreePhi* const treePhi1 = varDefTree1->AsLclVar()->Data()->AsPhi(); + GenTreePhi* const treePhi2 = varDefTree2->AsLclVar()->Data()->AsPhi(); + GenTreePhi::UseIterator treeIter1 = treePhi1->Uses().begin(); + GenTreePhi::UseIterator treeEnd1 = treePhi1->Uses().end(); + GenTreePhi::UseIterator treeIter2 = treePhi2->Uses().begin(); + GenTreePhi::UseIterator treeEnd2 = treePhi2->Uses().end(); + + bool phiArgsAreEquivalent = true; + + for (; (treeIter1 != treeEnd1) && (treeIter2 != treeEnd2); ++treeIter1, ++treeIter2) + { + GenTreePhiArg* const treePhiArg1 = treeIter1->GetNode()->AsPhiArg(); + GenTreePhiArg* const treePhiArg2 = treeIter2->GetNode()->AsPhiArg(); + + assert(treePhiArg1->gtPredBB == treePhiArg2->gtPredBB); + + ValueNum treePhiArgVN1 = treePhiArg1->gtVNPair.GetConservative(); + ValueNum treePhiArgVN2 = treePhiArg2->gtVNPair.GetConservative(); + + // If the PhiArg VNs differ, the phis are not equivalent. + // (Note we don't recurse into AreVNsEquivalent as we can't + // handle possible cycles in the SSA graph). + // + if (treePhiArgVN1 != treePhiArgVN2) + { + phiArgsAreEquivalent = false; + break; + } + + // If we failed to find meaningful VNs, the phis are not equivalent + // + if (treePhiArgVN1 == ValueNumStore::NoVN) + { + phiArgsAreEquivalent = false; + break; + } + } + + // If we didn't verify all phi args we have failed to prove equivalence + // + if (phiArgsAreEquivalent) + { + phiArgsAreEquivalent &= (treeIter1 == treeEnd1); + phiArgsAreEquivalent &= (treeIter2 == treeEnd2); + } + + return phiArgsAreEquivalent; +} + bool ValueNumStore::IsVNRelop(ValueNum vn) { VNFuncApp funcAttr; @@ -10342,38 +10474,15 @@ PhaseStatus Compiler::fgValueNumber() // SSA has already computed a post-order taking EH successors into account. // Visiting that in reverse will ensure we visit a block's predecessors // before itself whenever possible. + // + EnsureBasicBlockEpoch(); + BlockSet visitedBlocks(BlockSetOps::MakeEmpty(this)); BasicBlock** postOrder = m_dfsTree->GetPostOrder(); unsigned postOrderCount = m_dfsTree->GetPostOrderCount(); for (unsigned i = postOrderCount; i != 0; i--) { - BasicBlock* block = postOrder[i - 1]; - JITDUMP("Visiting " FMT_BB "\n", block->bbNum); - - if (block != fgFirstBB) - { - bool anyPredReachable = false; - for (FlowEdge* pred = BlockPredsWithEH(block); pred != nullptr; pred = pred->getNextPredEdge()) - { - BasicBlock* predBlock = pred->getSourceBlock(); - if (!vs.IsReachableThroughPred(block, predBlock)) - { - JITDUMP(" Unreachable through pred " FMT_BB "\n", predBlock->bbNum); - continue; - } - - JITDUMP(" Reachable through pred " FMT_BB "\n", predBlock->bbNum); - anyPredReachable = true; - break; - } - - if (!anyPredReachable) - { - JITDUMP(" " FMT_BB " was proven unreachable\n", block->bbNum); - vs.SetUnreachable(block); - } - } - - fgValueNumberBlock(block); + BasicBlock* const block = postOrder[i - 1]; + fgValueNumberBlocks(block, visitedBlocks); } #ifdef DEBUG @@ -10386,99 +10495,98 @@ PhaseStatus Compiler::fgValueNumber() return PhaseStatus::MODIFIED_EVERYTHING; } -void Compiler::fgValueNumberBlock(BasicBlock* blk) +//------------------------------------------------------------------------ +// fgValueNumberBlocks: Run value numbering for a block or blocks in a loop +// +// Arguments: +// block -- block to value number (may already have been numbered) +// visitedBlocks -- blocks that have already had VNs assigned +// +// Notes: +// +// Within the overall reverse post order, we want to ensure we visit all blocks in +// a loop before any possible loop block successors. So if we visit a loop +// header, we switch to visiting the loop blocks in RPO, and once we finish +// that visitation, we try and refine loop header PHIs. +// +void Compiler::fgValueNumberBlocks(BasicBlock* block, BlockSet& visitedBlocks) { - compCurBB = blk; + // Because we're not following the strict RPO, we may have already visisted this block. + // + if (BlockSetOps::IsMember(this, visitedBlocks, block->bbNum)) + { + return; + } - Statement* stmt = blk->firstStmt(); + JITDUMP("Visiting " FMT_BB "\n", block->bbNum); - // First: visit phis and check to see if all phi args have the same value. - for (; (stmt != nullptr) && stmt->IsPhiDefnStmt(); stmt = stmt->GetNextStmt()) + if (block != fgFirstBB) { - GenTreeLclVar* newSsaDef = stmt->GetRootNode()->AsLclVar(); - GenTreePhi* phiNode = newSsaDef->AsLclVar()->Data()->AsPhi(); - ValueNumPair phiVNP; - ValueNumPair sameVNP; - - for (GenTreePhi::Use& use : phiNode->Uses()) + bool anyPredReachable = false; + for (FlowEdge* pred = BlockPredsWithEH(block); pred != nullptr; pred = pred->getNextPredEdge()) { - GenTreePhiArg* phiArg = use.GetNode()->AsPhiArg(); - if (!vnState->IsReachableThroughPred(blk, phiArg->gtPredBB)) + BasicBlock* predBlock = pred->getSourceBlock(); + if (!vnState->IsReachableThroughPred(block, predBlock)) { - JITDUMP(" Phi arg [%06u] is unnecessary; path through pred " FMT_BB " cannot be taken\n", - dspTreeID(phiArg), phiArg->gtPredBB->bbNum); - - if ((use.GetNext() != nullptr) || (phiVNP.GetLiberal() != ValueNumStore::NoVN)) - { - continue; - } - - assert(!vnState->IsReachable(blk)); - JITDUMP(" ..but no other path can, so we are using it anyway\n"); + JITDUMP(" Unreachable through pred " FMT_BB "\n", predBlock->bbNum); + continue; } - ValueNum phiArgSsaNumVN = vnStore->VNForIntCon(phiArg->GetSsaNum()); - ValueNumPair phiArgVNP = lvaGetDesc(phiArg)->GetPerSsaData(phiArg->GetSsaNum())->m_vnPair; - - phiArg->gtVNPair = phiArgVNP; - - if (phiVNP.GetLiberal() == ValueNumStore::NoVN) - { - // This is the first PHI argument - phiVNP = ValueNumPair(phiArgSsaNumVN, phiArgSsaNumVN); - sameVNP = phiArgVNP; - } - else - { - phiVNP = vnStore->VNPairForFuncNoFolding(newSsaDef->TypeGet(), VNF_Phi, - ValueNumPair(phiArgSsaNumVN, phiArgSsaNumVN), phiVNP); + JITDUMP(" Reachable through pred " FMT_BB "\n", predBlock->bbNum); + anyPredReachable = true; + break; + } - if ((sameVNP.GetLiberal() != phiArgVNP.GetLiberal()) || - (sameVNP.GetConservative() != phiArgVNP.GetConservative())) - { - // If this argument's VNs are different from "same" then change "same" to NoVN. - // Note that this means that if any argument's VN is NoVN then the final result - // will also be NoVN, which is what we want. - sameVNP.SetBoth(ValueNumStore::NoVN); - } - } + if (!anyPredReachable) + { + JITDUMP(" " FMT_BB " was proven unreachable\n", block->bbNum); + vnState->SetUnreachable(block); } + } - // We should have visited at least one phi arg in the loop above - assert(phiVNP.GetLiberal() != ValueNumStore::NoVN); - assert(phiVNP.GetConservative() != ValueNumStore::NoVN); + fgValueNumberBlock(block); - ValueNumPair newSsaDefVNP; + // Mark block as visited + // + BlockSetOps::AddElemD(this, visitedBlocks, block->bbNum); - if (sameVNP.BothDefined()) + // Is block the head of a loop? + // + FlowGraphNaturalLoop* const loop = m_blockToLoop->GetLoop(block); + if ((loop != nullptr) && (block == loop->GetHeader())) + { + // Yes. Visit all other loop blocks using the within-loop RPO. + // + loop->VisitLoopBlocksReversePostOrder([&](BasicBlock* block) { + fgValueNumberBlocks(block, visitedBlocks); + return BasicBlockVisit::Continue; + }); + + // Re-value number all the PhiDefs in block + // + for (Statement* stmt = block->firstStmt(); (stmt != nullptr) && stmt->IsPhiDefnStmt(); + stmt = stmt->GetNextStmt()) { - // If all the args of the phi had the same value(s, liberal and conservative), then there wasn't really - // a reason to have the phi -- just pass on that value. - newSsaDefVNP = sameVNP; + GenTreeLclVar* const newSsaDef = stmt->GetRootNode()->AsLclVar(); + fgValueNumberPhiDef(newSsaDef, block, /* isUpdate */ true); } - else - { - // They were not the same; we need to create a phi definition. - ValueNum lclNumVN = ValueNum(newSsaDef->GetLclNum()); - ValueNum ssaNumVN = ValueNum(newSsaDef->GetSsaNum()); - newSsaDefVNP = vnStore->VNPairForFunc(newSsaDef->TypeGet(), VNF_PhiDef, ValueNumPair(lclNumVN, lclNumVN), - ValueNumPair(ssaNumVN, ssaNumVN), phiVNP); - } + // TODO: Propagate those new VNs to their uses? + // + } +} - LclSsaVarDsc* newSsaDefDsc = lvaGetDesc(newSsaDef)->GetPerSsaData(newSsaDef->GetSsaNum()); - newSsaDefDsc->m_vnPair = newSsaDefVNP; -#ifdef DEBUG - if (verbose) - { - printf("SSA PHI definition: set VN of local %d/%d to ", newSsaDef->GetLclNum(), newSsaDef->GetSsaNum()); - vnpPrint(newSsaDefVNP, 1); - printf(" %s.\n", sameVNP.BothDefined() ? "(all same)" : ""); - } -#endif // DEBUG +void Compiler::fgValueNumberBlock(BasicBlock* blk) +{ + compCurBB = blk; - newSsaDef->gtVNPair = vnStore->VNPForVoid(); - phiNode->gtVNPair = newSsaDefVNP; + Statement* stmt = blk->firstStmt(); + + // First: visit phis and check to see if all phi args have the same value. + for (; (stmt != nullptr) && stmt->IsPhiDefnStmt(); stmt = stmt->GetNextStmt()) + { + GenTreeLclVar* newSsaDef = stmt->GetRootNode()->AsLclVar(); + fgValueNumberPhiDef(newSsaDef, blk); } // Now do the same for each MemoryKind. @@ -10625,6 +10733,123 @@ void Compiler::fgValueNumberBlock(BasicBlock* blk) compCurBB = nullptr; } +//------------------------------------------------------------------------ +// fgValueNumberRegisterConstFieldSeq: If a VN'd integer constant has a +// field sequence we want to keep track of, then register it in the side table. +// +// Arguments: +// newSsaDef - lcl var node representing a phi definition +// blk - block with the phi +// isUpdate - true if the phi has already been numbered and this is a renumbering +// +void Compiler::fgValueNumberPhiDef(GenTreeLclVar* newSsaDef, BasicBlock* blk, bool isUpdate) +{ + GenTreePhi* phiNode = newSsaDef->AsLclVar()->Data()->AsPhi(); + ValueNumPair phiVNP; + ValueNumPair sameVNP; + + for (GenTreePhi::Use& use : phiNode->Uses()) + { + GenTreePhiArg* phiArg = use.GetNode()->AsPhiArg(); + if (!vnState->IsReachableThroughPred(blk, phiArg->gtPredBB)) + { + JITDUMP(" Phi arg [%06u] is unnecessary; path through pred " FMT_BB " cannot be taken\n", + dspTreeID(phiArg), phiArg->gtPredBB->bbNum); + + if ((use.GetNext() != nullptr) || (phiVNP.GetLiberal() != ValueNumStore::NoVN)) + { + continue; + } + + assert(!vnState->IsReachable(blk) || isUpdate); + JITDUMP(" ..but no other path can, so we are using it anyway\n"); + } + + ValueNum phiArgSsaNumVN = vnStore->VNForIntCon(phiArg->GetSsaNum()); + ValueNumPair phiArgVNP = lvaGetDesc(phiArg)->GetPerSsaData(phiArg->GetSsaNum())->m_vnPair; + + if (isUpdate && (phiArgVNP != phiArg->gtVNPair)) + { + JITDUMP("Updating phi arg [%06u] VN from ", dspTreeID(phiArg)); + JITDUMPEXEC(vnpPrint(phiArg->gtVNPair, 0)); + JITDUMP(" to "); + JITDUMPEXEC(vnpPrint(phiArgVNP, 0)); + JITDUMP("\n"); + } + + phiArg->gtVNPair = phiArgVNP; + + if (phiVNP.GetLiberal() == ValueNumStore::NoVN) + { + // This is the first PHI argument + phiVNP = ValueNumPair(phiArgSsaNumVN, phiArgSsaNumVN); + sameVNP = phiArgVNP; + } + else + { + phiVNP = vnStore->VNPairForFuncNoFolding(newSsaDef->TypeGet(), VNF_Phi, + ValueNumPair(phiArgSsaNumVN, phiArgSsaNumVN), phiVNP); + + if ((sameVNP.GetLiberal() != phiArgVNP.GetLiberal()) || + (sameVNP.GetConservative() != phiArgVNP.GetConservative())) + { + // If this argument's VNs are different from "same" then change "same" to NoVN. + // Note that this means that if any argument's VN is NoVN then the final result + // will also be NoVN, which is what we want. + sameVNP.SetBoth(ValueNumStore::NoVN); + } + } + } + + // We should have visited at least one phi arg in the loop above + assert(phiVNP.GetLiberal() != ValueNumStore::NoVN); + assert(phiVNP.GetConservative() != ValueNumStore::NoVN); + + ValueNumPair newSsaDefVNP; + + if (sameVNP.BothDefined()) + { + // If all the args of the phi had the same value(s, liberal and conservative), then there wasn't really + // a reason to have the phi -- just pass on that value. + newSsaDefVNP = sameVNP; + } + else + { + // They were not the same; we need to create a phi definition. + ValueNum lclNumVN = ValueNum(newSsaDef->GetLclNum()); + ValueNum ssaNumVN = ValueNum(newSsaDef->GetSsaNum()); + + newSsaDefVNP = vnStore->VNPairForFunc(newSsaDef->TypeGet(), VNF_PhiDef, ValueNumPair(lclNumVN, lclNumVN), + ValueNumPair(ssaNumVN, ssaNumVN), phiVNP); + } + + LclSsaVarDsc* newSsaDefDsc = lvaGetDesc(newSsaDef)->GetPerSsaData(newSsaDef->GetSsaNum()); + +#ifdef DEBUG + if (isUpdate) + { + assert(!newSsaDefDsc->m_updated); + newSsaDefDsc->m_origVNPair = newSsaDefDsc->m_vnPair; + newSsaDefDsc->m_updated = true; + } +#endif + + newSsaDefDsc->m_vnPair = newSsaDefVNP; + +#ifdef DEBUG + if (verbose) + { + printf("SSA PHI definition: set VN of local %d/%d to ", newSsaDef->GetLclNum(), newSsaDef->GetSsaNum()); + vnpPrint(newSsaDefVNP, 1); + printf(" %s.\n", sameVNP.BothDefined() ? "(all same)" : ""); + } +#endif // DEBUG + + newSsaDef->gtVNPair = vnStore->VNPForVoid(); + + phiNode->gtVNPair = newSsaDefVNP; +} + ValueNum Compiler::fgMemoryVNForLoopSideEffects(MemoryKind memoryKind, BasicBlock* entryBlock, FlowGraphNaturalLoop* loop) diff --git a/src/coreclr/jit/valuenum.h b/src/coreclr/jit/valuenum.h index ce33be31e19b0..0138e6e5aecf7 100644 --- a/src/coreclr/jit/valuenum.h +++ b/src/coreclr/jit/valuenum.h @@ -1066,6 +1066,13 @@ class ValueNumStore // Returns true iff the VN represents a relop bool IsVNRelop(ValueNum vn); + // Returns true iff the VN is a phi definition + bool IsVNPhiDef(ValueNum vn); + + // Returns true if the two VNs represent the same value + // despite being different VNs. Useful for phi def VNs. + bool AreVNsEquivalent(ValueNum vn1, ValueNum vn2); + enum class VN_RELATION_KIND { VRK_Inferred, // (x ? y)