diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 68a4c29f3c3f7..607cc870d0c26 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -7272,14 +7272,6 @@ class Compiler CSEdsc** optCSEhash; CSEdsc** optCSEtab; - typedef JitHashTable, GenTree*> NodeToNodeMap; - - NodeToNodeMap* optCseCheckedBoundMap; // Maps bound nodes to ancestor compares that should be - // re-numbered with the bound to improve range check elimination - - // Given a compare, look for a cse candidate checked bound feeding it and add a map entry if found. - void optCseUpdateCheckedBoundMap(GenTree* compare); - void optCSEstop(); CSEdsc* optCSEfindDsc(unsigned index); diff --git a/src/coreclr/jit/optcse.cpp b/src/coreclr/jit/optcse.cpp index 79fb36f2ad701..e55dd3d29ae63 100644 --- a/src/coreclr/jit/optcse.cpp +++ b/src/coreclr/jit/optcse.cpp @@ -466,9 +466,6 @@ void Compiler::optValnumCSE_Init() optCSECandidateCount = 0; optDoCSE = false; // Stays false until we find duplicate CSE tree - - // optCseCheckedBoundMap is unused in most functions, allocated only when used - optCseCheckedBoundMap = nullptr; } unsigned optCSEKeyToHashIndex(size_t key, size_t optCSEhashSize) @@ -765,7 +762,6 @@ unsigned Compiler::optValnumCSE_Index(GenTree* tree, Statement* stmt) hashDsc->csdUseWtCnt = 0; hashDsc->defExcSetPromise = vnStore->VNForEmptyExcSet(); hashDsc->defExcSetCurrent = vnStore->VNForNull(); // uninit value - hashDsc->defConservNormVN = vnStore->VNForNull(); // uninit value hashDsc->csdTree = tree; hashDsc->csdStmt = stmt; @@ -864,14 +860,6 @@ bool Compiler::optValnumCSE_Locate(CSE_HeuristicCommon* heuristic) bool stmtHasArrLenCandidate = false; for (GenTree* const tree : stmt->TreeList()) { - if (tree->OperIsCompare() && stmtHasArrLenCandidate) - { - // Check if this compare is a function of (one of) the checked - // bound candidate(s); we may want to update its value number. - // if the array length gets CSEd - optCseUpdateCheckedBoundMap(tree); - } - if (!heuristic->ConsiderTree(tree, isReturn)) { continue; @@ -908,100 +896,6 @@ bool Compiler::optValnumCSE_Locate(CSE_HeuristicCommon* heuristic) return true; } -//------------------------------------------------------------------------ -// optCseUpdateCheckedBoundMap: Check if this compare is a tractable function of -// a checked bound that is a CSE candidate, and insert -// an entry in the optCseCheckedBoundMap if so. This facilitates -// subsequently updating the compare's value number if -// the bound gets CSEd. -// -// Arguments: -// compare - The compare node to check -// -void Compiler::optCseUpdateCheckedBoundMap(GenTree* compare) -{ - assert(compare->OperIsCompare()); - - ValueNum compareVN = compare->gtVNPair.GetConservative(); - VNFuncApp cmpVNFuncApp; - - if (!vnStore->GetVNFunc(compareVN, &cmpVNFuncApp) || (cmpVNFuncApp.m_func != GetVNFuncForNode(compare))) - { - // Value numbering inferred this compare as something other - // than its own operator; leave its value number alone. - return; - } - - // Now look for a checked bound feeding the compare - ValueNumStore::CompareCheckedBoundArithInfo info; - - GenTree* boundParent = nullptr; - - if (vnStore->IsVNCompareCheckedBound(compareVN)) - { - // Simple compare of an bound against something else. - - vnStore->GetCompareCheckedBound(compareVN, &info); - boundParent = compare; - } - else if (vnStore->IsVNCompareCheckedBoundArith(compareVN)) - { - // Compare of a bound +/- some offset to something else. - - GenTree* op1 = compare->gtGetOp1(); - GenTree* op2 = compare->gtGetOp2(); - - vnStore->GetCompareCheckedBoundArithInfo(compareVN, &info); - if (GetVNFuncForNode(op1) == (VNFunc)info.arrOper) - { - // The arithmetic node is the bound's parent. - boundParent = op1; - } - else if (GetVNFuncForNode(op2) == (VNFunc)info.arrOper) - { - // The arithmetic node is the bound's parent. - boundParent = op2; - } - } - - if (boundParent != nullptr) - { - GenTree* bound = nullptr; - - // Find which child of boundParent is the bound. Abort if neither - // conservative value number matches the one from the compare VN. - - GenTree* child1 = boundParent->gtGetOp1(); - if ((info.vnBound == child1->gtVNPair.GetConservative()) && IS_CSE_INDEX(child1->gtCSEnum)) - { - bound = child1; - } - else - { - GenTree* child2 = boundParent->gtGetOp2(); - if ((info.vnBound == child2->gtVNPair.GetConservative()) && IS_CSE_INDEX(child2->gtCSEnum)) - { - bound = child2; - } - } - - if (bound != nullptr) - { - // Found a checked bound feeding a compare that is a tractable function of it; - // record this in the map so we can update the compare VN if the bound - // node gets CSEd. - - if (optCseCheckedBoundMap == nullptr) - { - // Allocate map on first use. - optCseCheckedBoundMap = new (getAllocator(CMK_CSE)) NodeToNodeMap(getAllocator()); - } - - optCseCheckedBoundMap->Set(bound, compare); - } - } -} - /***************************************************************************** * * Compute each blocks bbCseGen @@ -1600,31 +1494,6 @@ void Compiler::optValnumCSE_Availability() } } - // For shared const CSE we don't set/use the defConservNormVN - // - if (!Is_Shared_Const_CSE(desc->csdHashKey)) - { - // Record or update the value of desc->defConservNormVN - // - ValueNum theConservNormVN = vnStore->VNConservativeNormalValue(tree->gtVNPair); - - // Is defConservNormVN still set to the uninit marker value of VNForNull() ? - if (desc->defConservNormVN == vnStore->VNForNull()) - { - // This is the first def that we have visited, set defConservNormVN - desc->defConservNormVN = theConservNormVN; - } - else - { - // Check to see if all defs have the same conservative normal VN - if (theConservNormVN != desc->defConservNormVN) - { - // This candidate has defs with differing conservative normal VNs, mark it with NoVN - desc->defConservNormVN = ValueNumStore::NoVN; // record the marker for differing VNs - } - } - } - // If we get here we have accepted this node as a valid CSE def desc->csdDefCount += 1; @@ -4963,325 +4832,308 @@ void CSE_HeuristicCommon::PerformCSE(CSE_Candidate* successfulCandidate) } #endif // DEBUG - // Setup 'lst' to point at the start of this candidate list - lst = dsc->csdTreeList; - noway_assert(lst); + IncrementalSsaBuilder ssaBuilder(m_pCompiler, cseLclVarNum); + + ArrayStack defUses(m_pCompiler->getAllocator(CMK_CSE)); - do + // First process the defs. + for (lst = dsc->csdTreeList; lst != nullptr; lst = lst->tslNext) { - /* Process the next node in the list */ GenTree* const exp = lst->tslTree; Statement* const stmt = lst->tslStmt; BasicBlock* const blk = lst->tslBlock; - /* Advance to the next node in the list */ - lst = lst->tslNext; - - // We may have cleared this CSE in optValuenumCSE_Availability - // due to different exception sets. - // - // Ignore this node if the gtCSEnum value has been cleared - if (!IS_CSE_INDEX(exp->gtCSEnum)) + if (!IS_CSE_DEF(exp->gtCSEnum)) { continue; } - // Assert if we used DEBUG_DESTROY_NODE on this CSE exp - assert(exp->gtOper != GT_COUNT); - - /* Make sure we update the weighted ref count correctly */ - m_pCompiler->optCSEweight = blk->getBBWeight(m_pCompiler); - - /* Figure out the actual type of the value */ - var_types expTyp = genActualType(exp->TypeGet()); - - // The cseLclVarType must be a compatible with expTyp - // - ValueNumStore* vnStore = m_pCompiler->vnStore; - noway_assert(IsCompatibleType(cseLclVarTyp, expTyp) || (dsc->csdConstDefVN != vnStore->VNForNull())); - - // This will contain the replacement tree for exp - // It will either be the CSE def or CSE ref - // - GenTree* cse = nullptr; - bool isDef; +#ifdef DEBUG + if (m_pCompiler->verbose) + { + printf("\n" FMT_CSE " def at ", GET_CSE_INDEX(exp->gtCSEnum)); + Compiler::printTreeID(exp); + printf(" replaced in " FMT_BB " with def of V%02u\n", blk->bbNum, cseLclVarNum); + } +#endif // DEBUG - if (IS_CSE_USE(exp->gtCSEnum)) + GenTree* val = exp; + if (isSharedConst) { - /* This is a use of the CSE */ - isDef = false; -#ifdef DEBUG - if (m_pCompiler->verbose) + ValueNum currVN = m_pCompiler->vnStore->VNLiberalNormalValue(exp->gtVNPair); + ssize_t curValue = m_pCompiler->vnStore->CoercedConstantValue(currVN); + ssize_t delta = curValue - dsc->csdConstDefValue; + if (delta != 0) { - printf("\nWorking on the replacement of the " FMT_CSE " use at ", exp->gtCSEnum); - Compiler::printTreeID(exp); - printf(" in " FMT_BB "\n", blk->bbNum); + val = m_pCompiler->gtNewIconNode(dsc->csdConstDefValue, cseLclVarTyp); + val->gtVNPair.SetBoth(dsc->csdConstDefVN); } -#endif // DEBUG + } - // We will replace the CSE ref with a new tree - // this is typically just a simple use of the new CSE LclVar - // + // Create a store of the value to the temp + GenTree* store = m_pCompiler->gtNewTempStore(cseLclVarNum, val); + GenTree* origStore = store; - // Create a reference to the CSE temp - GenTreeLclVar* cseLclVar = m_pCompiler->gtNewLclvNode(cseLclVarNum, cseLclVarTyp); - cseLclVar->gtVNPair.SetBoth(dsc->csdConstDefVN); + if (!store->OperIs(GT_STORE_LCL_VAR)) + { + // This can only be the case for a struct in which the 'val' was a COMMA, so + // the store is sunk below it. + store = store->gtEffectiveVal(); + noway_assert(origStore->OperIs(GT_COMMA) && (origStore == val)); + } + else + { + noway_assert(store->Data() == val); + } - cse = cseLclVar; - if (isSharedConst) + // Assign the proper Value Numbers. + ValueNumPair valExc = m_pCompiler->vnStore->VNPExceptionSet(val->gtVNPair); + store->gtVNPair = m_pCompiler->vnStore->VNPWithExc(ValueNumStore::VNPForVoid(), valExc); + noway_assert(store->OperIs(GT_STORE_LCL_VAR)); + + // Move the information about the CSE def to the store; it now indicates a completed + // CSE def instead of just a candidate. optCSE_canSwap uses this information to reason + // about evaluation order in between substitutions of CSE defs/uses, and we use it + // below to insert the locals into SSA. + store->gtCSEnum = exp->gtCSEnum; + exp->gtCSEnum = NO_CSE; + + // Create a reference to the CSE temp + GenTreeLclVar* cseLclVar = m_pCompiler->gtNewLclvNode(cseLclVarNum, cseLclVarTyp); + cseLclVar->gtVNPair = m_pCompiler->vnStore->VNPNormalPair(val->gtVNPair); + + GenTree* cseUse = cseLclVar; + if (isSharedConst) + { + ValueNum currVN = m_pCompiler->vnStore->VNLiberalNormalValue(exp->gtVNPair); + ssize_t curValue = m_pCompiler->vnStore->CoercedConstantValue(currVN); + ssize_t delta = curValue - dsc->csdConstDefValue; + if (delta != 0) { - ValueNum currVN = m_pCompiler->vnStore->VNLiberalNormalValue(exp->gtVNPair); - ssize_t curValue = m_pCompiler->vnStore->CoercedConstantValue(currVN); - ssize_t delta = curValue - dsc->csdConstDefValue; - if (delta != 0) - { - GenTree* deltaNode = m_pCompiler->gtNewIconNode(delta, cseLclVarTyp); - cse = m_pCompiler->gtNewOperNode(GT_ADD, cseLclVarTyp, cseLclVar, deltaNode); - cse->SetDoNotCSE(); - } + GenTree* deltaNode = m_pCompiler->gtNewIconNode(delta, cseLclVarTyp); + cseUse = m_pCompiler->gtNewOperNode(GT_ADD, cseLclVarTyp, cseLclVar, deltaNode); + cseUse->SetDoNotCSE(); + cseUse->gtVNPair.SetBoth(currVN); } + } - // assign the proper ValueNumber, A CSE use discards any exceptions - cse->gtVNPair = vnStore->VNPNormalPair(exp->gtVNPair); + // Create a comma node for the CSE assignment + GenTree* cse = m_pCompiler->gtNewOperNode(GT_COMMA, genActualType(exp), origStore, cseUse); + // The comma's value is the same as 'val' as the store to the CSE + // LclVar cannot add any new exceptions + cse->gtVNPair = val->gtVNPair; - // shared const CSE has the correct value number assigned - // and both liberal and conservative are identical - // and they do not use theConservativeVN - // - if (!isSharedConst) - { - ValueNum theConservativeVN = successfulCandidate->CseDsc()->defConservNormVN; + ReplaceCSENode(stmt, exp, cse); - if (theConservativeVN != ValueNumStore::NoVN) - { - // All defs of this CSE share the same normal conservative VN, and we are rewriting this - // use to fetch the same value with no reload, so we can safely propagate that - // conservative VN to this use. This can help range check elimination later on. - cse->gtVNPair.SetConservative(theConservativeVN); - - // If the old VN was flagged as a checked bound, propagate that to the new VN - // to make sure assertion prop will pay attention to this VN. - ValueNum oldVN = exp->gtVNPair.GetConservative(); - if (!vnStore->IsVNConstant(theConservativeVN) && vnStore->IsVNCheckedBound(oldVN)) - { - vnStore->SetVNIsCheckedBound(theConservativeVN); - } + ssaBuilder.InsertDef(UseDefLocation(blk, stmt, store->AsLclVar())); - GenTree* cmp; - if ((m_pCompiler->optCseCheckedBoundMap != nullptr) && - (m_pCompiler->optCseCheckedBoundMap->Lookup(exp, &cmp))) - { - // Propagate the new value number to this compare node as well, since - // subsequent range check elimination will try to correlate it with - // the other appearances that are getting CSEd. + // Record the new use we created as part of this def. + defUses.Emplace(blk, stmt, cseLclVar); + } - ValueNum oldCmpVN = cmp->gtVNPair.GetConservative(); - ValueNum newCmpArgVN; + bool insertIntoSsa = ssaBuilder.FinalizeDefs(); - ValueNumStore::CompareCheckedBoundArithInfo info; - if (vnStore->IsVNCompareCheckedBound(oldCmpVN)) - { - // Comparison is against the bound directly. + // Start out by inserting all the uses we created as part of defs into SSA. + if (insertIntoSsa) + { + JITDUMP("Inserting each use created for defs into SSA\n"); + for (int i = 0; i < defUses.Height(); i++) + { + InsertUseIntoSsa(ssaBuilder, defUses.BottomRef(i)); + } + } - newCmpArgVN = theConservativeVN; - vnStore->GetCompareCheckedBound(oldCmpVN, &info); - } - else - { - // Comparison is against the bound +/- some offset. + // Now process the actual uses. + for (lst = dsc->csdTreeList; lst != nullptr; lst = lst->tslNext) + { + GenTree* const exp = lst->tslTree; + Statement* const stmt = lst->tslStmt; + BasicBlock* const blk = lst->tslBlock; - assert(vnStore->IsVNCompareCheckedBoundArith(oldCmpVN)); - vnStore->GetCompareCheckedBoundArithInfo(oldCmpVN, &info); + if (!IS_CSE_USE(exp->gtCSEnum)) + { + continue; + } - ValueNum arrOp1 = info.arrOpLHS ? info.arrOp : theConservativeVN; - ValueNum arrOp2 = info.arrOpLHS ? theConservativeVN : info.arrOp; + // Make sure we update the weighted ref count correctly + m_pCompiler->optCSEweight = blk->getBBWeight(m_pCompiler); - newCmpArgVN = - vnStore->VNForFunc(vnStore->TypeOfVN(info.arrOp), (VNFunc)info.arrOper, arrOp1, arrOp2); - } - ValueNum newCmpVN = vnStore->VNForFunc(vnStore->TypeOfVN(oldCmpVN), (VNFunc)info.cmpOper, - info.cmpOp, newCmpArgVN); - cmp->gtVNPair.SetConservative(newCmpVN); - } - } - } + // This is a use of the CSE #ifdef DEBUG - cse->gtDebugFlags |= GTF_DEBUG_VAR_CSE_REF; + if (m_pCompiler->verbose) + { + printf("\nWorking on the replacement of the " FMT_CSE " use at ", exp->gtCSEnum); + Compiler::printTreeID(exp); + printf(" in " FMT_BB "\n", blk->bbNum); + } #endif // DEBUG - // Now we need to unmark any nested CSE's uses that are found in 'exp' - // As well we extract any nested CSE defs that are found in 'exp' and - // these are appended to the sideEffList + // We will replace the CSE ref with a new tree + // this is typically just a simple use of the new CSE LclVar + // - // Afterwards the set of nodes in the 'sideEffectList' are preserved and - // all other nodes are removed. - // - exp->gtCSEnum = NO_CSE; // clear the gtCSEnum field + // Create a reference to the CSE temp + GenTreeLclVar* cseLclVar = m_pCompiler->gtNewLclvNode(cseLclVarNum, cseLclVarTyp); + GenTree* cse = cseLclVar; - GenTree* sideEffList = m_pCompiler->optExtractSideEffectsForCSE(exp); + if (isSharedConst) + { + cseLclVar->gtVNPair.SetBoth(dsc->csdConstDefVN); - // If we have any side effects or extracted CSE defs then we need to create a GT_COMMA tree instead - // - if (sideEffList != nullptr) + ValueNum currVN = m_pCompiler->vnStore->VNLiberalNormalValue(exp->gtVNPair); + ssize_t curValue = m_pCompiler->vnStore->CoercedConstantValue(currVN); + ssize_t delta = curValue - dsc->csdConstDefValue; + if (delta != 0) { -#ifdef DEBUG - if (m_pCompiler->verbose) - { - printf("\nThis CSE use has side effects and/or nested CSE defs. The sideEffectList:\n"); - m_pCompiler->gtDispTree(sideEffList); - printf("\n"); - } -#endif - ValueNumPair sideEffExcSet = vnStore->VNPExceptionSet(sideEffList->gtVNPair); - ValueNumPair cseWithSideEffVNPair = vnStore->VNPWithExc(cse->gtVNPair, sideEffExcSet); - - // Create a comma node with the sideEffList as op1 - cse = m_pCompiler->gtNewOperNode(GT_COMMA, expTyp, sideEffList, cse); - cse->gtVNPair = cseWithSideEffVNPair; + GenTree* deltaNode = m_pCompiler->gtNewIconNode(delta, cseLclVarTyp); + cse = m_pCompiler->gtNewOperNode(GT_ADD, cseLclVarTyp, cse, deltaNode); + cse->SetDoNotCSE(); + cse->gtVNPair.SetBoth(currVN); } } else { - /* This is a def of the CSE */ - isDef = true; -#ifdef DEBUG - if (m_pCompiler->verbose) - { - printf("\n" FMT_CSE " def at ", GET_CSE_INDEX(exp->gtCSEnum)); - Compiler::printTreeID(exp); - printf(" replaced in " FMT_BB " with def of V%02u\n", blk->bbNum, cseLclVarNum); - } -#endif // DEBUG + // Use the VNP that was on the expression. The conservative VN + // might not match the reaching def, but if things are in SSA we + // will fix that up later. + cse->gtVNPair = m_pCompiler->vnStore->VNPNormalPair(exp->gtVNPair); + } - GenTree* val = exp; - if (isSharedConst) - { - ValueNum currVN = m_pCompiler->vnStore->VNLiberalNormalValue(exp->gtVNPair); - ssize_t curValue = m_pCompiler->vnStore->CoercedConstantValue(currVN); - ssize_t delta = curValue - dsc->csdConstDefValue; - if (delta != 0) - { - val = m_pCompiler->gtNewIconNode(dsc->csdConstDefValue, cseLclVarTyp); - val->gtVNPair.SetBoth(dsc->csdConstDefVN); - } - } + INDEBUG(cse->gtDebugFlags |= GTF_DEBUG_VAR_CSE_REF); - /* Create a store of the value to the temp */ - GenTree* store = m_pCompiler->gtNewTempStore(cseLclVarNum, val); - GenTree* origStore = store; + // Now we need to unmark any nested CSE's uses that are found in 'exp' + // As well we extract any nested CSE defs that are found in 'exp' and + // these are appended to the sideEffList - if (!store->OperIs(GT_STORE_LCL_VAR)) - { - // This can only be the case for a struct in which the 'val' was a COMMA, so - // the store is sunk below it. - store = store->gtEffectiveVal(); - noway_assert(origStore->OperIs(GT_COMMA) && (origStore == val)); - } - else + // Afterwards the set of nodes in the 'sideEffectList' are preserved and + // all other nodes are removed. + // + exp->gtCSEnum = NO_CSE; // clear the gtCSEnum field + + GenTree* sideEffList = m_pCompiler->optExtractSideEffectsForCSE(exp); + + // If we have any side effects or extracted CSE defs then we need to create a GT_COMMA tree instead + // + if (sideEffList != nullptr) + { +#ifdef DEBUG + if (m_pCompiler->verbose) { - noway_assert(store->Data() == val); + printf("\nThis CSE use has side effects and/or nested CSE defs. The sideEffectList:\n"); + m_pCompiler->gtDispTree(sideEffList); + printf("\n"); } +#endif + ValueNumPair sideEffExcSet = m_pCompiler->vnStore->VNPExceptionSet(sideEffList->gtVNPair); + ValueNumPair cseWithSideEffVNPair = m_pCompiler->vnStore->VNPWithExc(cse->gtVNPair, sideEffExcSet); - // Assign the proper Value Numbers. - store->gtVNPair = ValueNumStore::VNPForVoid(); // The store node itself is $VN.Void. - noway_assert(store->OperIs(GT_STORE_LCL_VAR)); + // Create a comma node with the sideEffList as op1 + cse = m_pCompiler->gtNewOperNode(GT_COMMA, genActualType(exp), sideEffList, cse); + cse->gtVNPair = cseWithSideEffVNPair; + } - // Move the information about the CSE def to the store; it now indicates a completed - // CSE def instead of just a candidate. optCSE_canSwap uses this information to reason - // about evaluation order in between substitutions of CSE defs/uses. - store->gtCSEnum = exp->gtCSEnum; - exp->gtCSEnum = NO_CSE; + ReplaceCSENode(stmt, exp, cse); - /* Create a reference to the CSE temp */ - GenTree* cseLclVar = m_pCompiler->gtNewLclvNode(cseLclVarNum, cseLclVarTyp); - cseLclVar->gtVNPair.SetBoth(dsc->csdConstDefVN); + if (insertIntoSsa) + { + ValueNumPair oldLclVNP = cseLclVar->gtVNPair; + InsertUseIntoSsa(ssaBuilder, UseDefLocation(blk, stmt, cseLclVar)); - GenTree* cseUse = cseLclVar; - if (isSharedConst) + // Update conservative VN of comma node in case we changed + // conservative VNs due to a new reaching def + if ((sideEffList != nullptr) && (cseLclVar->gtVNPair != oldLclVNP)) { - ValueNum currVN = m_pCompiler->vnStore->VNLiberalNormalValue(exp->gtVNPair); - ssize_t curValue = m_pCompiler->vnStore->CoercedConstantValue(currVN); - ssize_t delta = curValue - dsc->csdConstDefValue; - if (delta != 0) - { - GenTree* deltaNode = m_pCompiler->gtNewIconNode(delta, cseLclVarTyp); - cseUse = m_pCompiler->gtNewOperNode(GT_ADD, cseLclVarTyp, cseLclVar, deltaNode); - cseUse->SetDoNotCSE(); - } + // For shared const CSE we should never change VN when finding a new reaching def. + assert(!isSharedConst && (cse->gtEffectiveVal() == cseLclVar)); + ValueNumPair sideEffExcSet = m_pCompiler->vnStore->VNPExceptionSet(sideEffList->gtVNPair); + cse->gtVNPair = m_pCompiler->vnStore->VNPWithExc(cseLclVar->gtVNPair, sideEffExcSet); } - cseUse->gtVNPair = exp->gtVNPair; // The 'cseUse' is equal to the original expression. - - /* Create a comma node for the CSE assignment */ - cse = m_pCompiler->gtNewOperNode(GT_COMMA, expTyp, origStore, cseUse); - cse->gtVNPair = cseUse->gtVNPair; // The comma's value is the same as 'val' - // as the store to the CSE LclVar - // cannot add any new exceptions } + } +} - cse->CopyReg(exp); // The cse inheirits any reg num property from the original exp node - exp->ClearRegNum(); // The exp node (for a CSE def) no longer has a register requirement +//------------------------------------------------------------------------ +// ReplaceCSENode: +// Replace a particular node with a new node by finding its parent and +// updating the link. +// +// Parameters: +// stmt - Statement that contains the node +// exp - Tree to replace +// newNode - New node to replace with +// +void CSE_HeuristicCommon::ReplaceCSENode(Statement* stmt, GenTree* exp, GenTree* newNode) +{ + newNode->CopyReg(exp); // The cse inheirits any reg num property from the original exp node + exp->ClearRegNum(); // The exp node (for a CSE def) no longer has a register requirement - // Walk the statement 'stmt' and find the pointer - // in the tree is pointing to 'exp' - // - Compiler::FindLinkData linkData = m_pCompiler->gtFindLink(stmt, exp); - GenTree** link = linkData.result; + // Walk the statement 'stmt' and find the pointer + // in the tree is pointing to 'exp' + // + Compiler::FindLinkData linkData = m_pCompiler->gtFindLink(stmt, exp); + GenTree** link = linkData.result; #ifdef DEBUG - if (link == nullptr) - { - printf("\ngtFindLink failed: stm="); - Compiler::printStmtID(stmt); - printf(", exp="); - Compiler::printTreeID(exp); - printf("\n"); - printf("stm ="); - m_pCompiler->gtDispStmt(stmt); - printf("\n"); - printf("exp ="); - m_pCompiler->gtDispTree(exp); - printf("\n"); - } + if (link == nullptr) + { + printf("\ngtFindLink failed: stm="); + Compiler::printStmtID(stmt); + printf(", exp="); + Compiler::printTreeID(exp); + printf("\n"); + printf("stm ="); + m_pCompiler->gtDispStmt(stmt); + printf("\n"); + printf("exp ="); + m_pCompiler->gtDispTree(exp); + printf("\n"); + } #endif // DEBUG - noway_assert(link); + noway_assert(link); - // Mutate this link, thus replacing the old exp with the new CSE representation - // - *link = cse; + // Mutate this link, thus replacing the old exp with the new CSE representation + // + *link = newNode; - m_pCompiler->gtSetStmtInfo(stmt); - m_pCompiler->fgSetStmtSeq(stmt); - m_pCompiler->gtUpdateStmtSideEffects(stmt); + m_pCompiler->gtSetStmtInfo(stmt); + m_pCompiler->fgSetStmtSeq(stmt); + m_pCompiler->gtUpdateStmtSideEffects(stmt); +} - } while (lst != nullptr); +//------------------------------------------------------------------------ +// InsertUseIntoSsa: +// Insert a use into SSA form, updating its conservative VN to match its +// reaching definition in the process. +// +// Parameters: +// ssaBuilder - Incremental SSA builder that has already had definitions inserted +// useDefLoc - Location of the new use +// +void CSE_HeuristicCommon::InsertUseIntoSsa(IncrementalSsaBuilder& ssaBuilder, const UseDefLocation& useDefLoc) +{ + if (!ssaBuilder.InsertUse(useDefLoc)) + { + return; + } - ArrayStack defs(m_pCompiler->getAllocator(CMK_CSE)); - ArrayStack uses(m_pCompiler->getAllocator(CMK_CSE)); + GenTreeLclVar* lcl = useDefLoc.Tree; + assert(lcl->HasSsaName()); - lst = dsc->csdTreeList; - do - { - Statement* lstStmt = lst->tslStmt; - for (GenTree* tree : lstStmt->TreeList()) - { - if (tree->OperIs(GT_LCL_VAR) && (tree->AsLclVar()->GetLclNum() == cseLclVarNum)) - { - uses.Push(UseDefLocation(lst->tslBlock, lstStmt, tree->AsLclVar())); - } - if (tree->OperIs(GT_STORE_LCL_VAR) && (tree->AsLclVar()->GetLclNum() == cseLclVarNum)) - { - defs.Push(UseDefLocation(lst->tslBlock, lstStmt, tree->AsLclVar())); - } - } + LclVarDsc* lclDsc = m_pCompiler->lvaGetDesc(lcl); + // Fix up the conservative VN using information about the reaching def. + LclSsaVarDsc* ssaDsc = lclDsc->GetPerSsaData(lcl->GetSsaNum()); - do - { - lst = lst->tslNext; - } while ((lst != nullptr) && (lst->tslStmt == lstStmt)); - } while (lst != nullptr); + ValueNum oldConservativeVN = lcl->gtVNPair.GetConservative(); + lcl->gtVNPair = ssaDsc->m_vnPair; - SsaBuilder::InsertInSsa(m_pCompiler, cseLclVarNum, defs, uses); + // If the old VN was flagged as a checked bound then propagate that to the + // new VN to make sure assertion prop will pay attention to this VN. + if ((oldConservativeVN != ssaDsc->m_vnPair.GetConservative()) && + m_pCompiler->vnStore->IsVNCheckedBound(oldConservativeVN)) + { + m_pCompiler->vnStore->SetVNIsCheckedBound(ssaDsc->m_vnPair.GetConservative()); + } } void CSE_Heuristic::AdjustHeuristic(CSE_Candidate* successfulCandidate) diff --git a/src/coreclr/jit/optcse.h b/src/coreclr/jit/optcse.h index 48fba4e50e13a..0a072f9a868f1 100644 --- a/src/coreclr/jit/optcse.h +++ b/src/coreclr/jit/optcse.h @@ -92,6 +92,10 @@ class CSE_HeuristicCommon JITDUMP("%s\n", Name()); } #endif + +private: + void ReplaceCSENode(Statement* stmt, GenTree* exp, GenTree* newNode); + void InsertUseIntoSsa(class IncrementalSsaBuilder& ssaBuilder, const struct UseDefLocation& useDefLoc); }; #ifdef DEBUG @@ -365,11 +369,6 @@ struct CSEdsc // The set of exceptions we currently can use for CSE uses. ValueNum defExcSetCurrent; - // if all def occurrences share the same conservative normal value - // number, this will reflect it; otherwise, NoVN. - // not used for shared const CSE's - ValueNum defConservNormVN; - // Number of distinct locals referenced (in first def tree) // and total number of local nodes. // diff --git a/src/coreclr/jit/ssabuilder.cpp b/src/coreclr/jit/ssabuilder.cpp index 40d9562e30249..abdaf3803b4c7 100644 --- a/src/coreclr/jit/ssabuilder.cpp +++ b/src/coreclr/jit/ssabuilder.cpp @@ -1362,21 +1362,6 @@ void Compiler::JitTestCheckSSA() } #endif // DEBUG -class IncrementalLiveInBuilder -{ - Compiler* m_comp; - ArrayStack m_queue; - -public: - IncrementalLiveInBuilder(Compiler* comp) - : m_comp(comp) - , m_queue(comp->getAllocator(CMK_SSA)) - { - } - - void MarkLiveInBackwards(unsigned lclNum, const UseDefLocation& use, const UseDefLocation& reachingDef); -}; - //------------------------------------------------------------------------ // MarkLiveInBackwards: Given a use and its reaching definition, mark that // local as live-in into all blocks on the path from the reaching definition to @@ -1427,45 +1412,6 @@ void IncrementalLiveInBuilder::MarkLiveInBackwards(unsigned lclNum, } } -class IncrementalSsaBuilder -{ - Compiler* m_comp; - unsigned m_lclNum; - ArrayStack& m_defs; - ArrayStack& m_uses; - BitVecTraits m_poTraits; - BitVec m_defBlocks; - BitVec m_iteratedDominanceFrontiers; - IncrementalLiveInBuilder m_liveInBuilder; - - UseDefLocation FindOrCreateReachingDef(const UseDefLocation& use); - bool FindReachingDefInBlock(const UseDefLocation& use, BasicBlock* block, UseDefLocation* def); - bool FindReachingDefInSameStatement(const UseDefLocation& use, UseDefLocation* def); - Statement* LatestStatement(Statement* stmt1, Statement* stmt2); -public: - IncrementalSsaBuilder(Compiler* comp, - unsigned lclNum, - ArrayStack& defs, - ArrayStack& uses) - : m_comp(comp) - , m_lclNum(lclNum) - , m_defs(defs) - , m_uses(uses) - , m_poTraits(comp->m_dfsTree->PostOrderTraits()) - , m_defBlocks(BitVecOps::MakeEmpty(&m_poTraits)) - , m_iteratedDominanceFrontiers(BitVecOps::MakeEmpty(&m_poTraits)) - , m_liveInBuilder(comp) - { - } - - bool Insert(); - static void MarkLiveInBackwards(Compiler* comp, - unsigned lclNum, - const UseDefLocation& use, - const UseDefLocation& reachingDef, - BitVec& visitedSet); -}; - //------------------------------------------------------------------------ // FindOrCreateReachingDef: Given a use indicated by a block and potentially a // statement and tree, find the reaching definition for it, potentially @@ -1687,15 +1633,79 @@ Statement* IncrementalSsaBuilder::LatestStatement(Statement* stmt1, Statement* s } //------------------------------------------------------------------------ -// Insert: Insert the uses and definitions in SSA. +// InsertDef: Record a definition. +// +// Parameters: +// def - The location of the definition +// +void IncrementalSsaBuilder::InsertDef(const UseDefLocation& def) +{ + assert(!m_finalizedDefs); + m_defs.Push(def); +} + +//------------------------------------------------------------------------ +// FinalizeDefs: +// Finalize information about defs after all definitions for the local have +// been added with "InsertDef". // // Returns: -// True if we were able to insert the local into SSA. False if we gave up -// (due to hitting internal limits). +// True if the local can be inserted into SSA (in which case calling +// InsertUse is allowed). // -bool IncrementalSsaBuilder::Insert() +bool IncrementalSsaBuilder::FinalizeDefs() { - FlowGraphDfsTree* dfsTree = m_comp->m_dfsTree; + assert(!m_finalizedDefs); + +#ifdef DEBUG + if (m_comp->verbose) + { + printf("Finalizing defs for SSA insertion of V%02u\n", m_lclNum); + printf(" %d defs:", m_defs.Height()); + for (int i = 0; i < m_defs.Height(); i++) + { + printf(" [%06u]", Compiler::dspTreeID(m_defs.Bottom(i).Tree)); + } + printf("\n"); + } +#endif + + LclVarDsc* dsc = m_comp->lvaGetDesc(m_lclNum); + + if (m_defs.Height() == 1) + { + // Single-def case, no need to compute flow graph annotations + JITDUMP(" Single-def local; putting into SSA directly\n"); + + UseDefLocation& def = m_defs.BottomRef(0); + + unsigned ssaNum = dsc->lvPerSsaData.AllocSsaNum(m_comp->getAllocator(CMK_SSA), def.Block, def.Tree); + def.Tree->SetSsaNum(ssaNum); + JITDUMP(" [%06u] d:%u\n", Compiler::dspTreeID(def.Tree), ssaNum); + dsc->lvInSsa = true; + dsc->GetPerSsaData(ssaNum)->m_vnPair = m_comp->vnStore->VNPNormalPair(def.Tree->Data()->gtVNPair); + m_finalizedDefs = true; + return true; + } + + if (m_comp->m_dfsTree == nullptr) + { + m_comp->m_dfsTree = m_comp->fgComputeDfs(); + } + + if (m_comp->m_domTree == nullptr) + { + m_comp->m_domTree = FlowGraphDominatorTree::Build(m_comp->m_dfsTree); + } + + if (m_comp->m_domFrontiers == nullptr) + { + m_comp->m_domFrontiers = FlowGraphDominanceFrontiers::Build(m_comp->m_domTree); + } + + m_poTraits = BitVecTraits(m_comp->m_dfsTree->PostOrderTraits()); + m_defBlocks = BitVecOps::MakeEmpty(&m_poTraits); + m_iteratedDominanceFrontiers = BitVecOps::MakeEmpty(&m_poTraits); // Compute iterated dominance frontiers of all real definitions. These are // the blocks that unpruned phi definitions would be inserted into. We @@ -1723,12 +1733,11 @@ bool IncrementalSsaBuilder::Insert() return false; } - LclVarDsc* dsc = m_comp->lvaGetDesc(m_lclNum); // Alloc SSA numbers for all real definitions. for (int i = 0; i < m_defs.Height(); i++) { UseDefLocation& def = m_defs.BottomRef(i); - if (!dfsTree->Contains(def.Block)) + if (!m_comp->m_dfsTree->Contains(def.Block)) { continue; } @@ -1738,122 +1747,60 @@ bool IncrementalSsaBuilder::Insert() unsigned ssaNum = dsc->lvPerSsaData.AllocSsaNum(m_comp->getAllocator(CMK_SSA), def.Block, def.Tree); def.Tree->SetSsaNum(ssaNum); LclSsaVarDsc* ssaDsc = dsc->GetPerSsaData(ssaNum); - ssaDsc->m_vnPair = def.Tree->Data()->gtVNPair; + ssaDsc->m_vnPair = m_comp->vnStore->VNPNormalPair(def.Tree->Data()->gtVNPair); JITDUMP(" [%06u] d:%u\n", Compiler::dspTreeID(def.Tree), ssaNum); } - // Finally compute all the reaching defs for the uses. - for (int i = 0; i < m_uses.Height(); i++) - { - UseDefLocation& use = m_uses.BottomRef(i); - if (!dfsTree->Contains(use.Block)) - { - continue; - } - - UseDefLocation def = FindOrCreateReachingDef(use); - use.Tree->SetSsaNum(def.Tree->GetSsaNum()); - dsc->GetPerSsaData(def.Tree->GetSsaNum())->AddUse(use.Block); - JITDUMP(" [%06u] u:%u\n", Compiler::dspTreeID(use.Tree), def.Tree->GetSsaNum()); - - m_liveInBuilder.MarkLiveInBackwards(m_lclNum, use, def); - } - + dsc->lvInSsa = true; + m_finalizedDefs = true; return true; } //------------------------------------------------------------------------ -// InsertInSsa: Insert a specified local in SSA given its local number and all -// of its definitions and uses in the IR. +// InsertUse: +// Insert a use into SSA. // // Parameters: -// comp - Compiler instance -// lclNum - The local that is being inserted into SSA -// defs - All STORE_LCL_VAR definitions of the local -// uses - All LCL_VAR uses of the local +// use - Location of the use // // Returns: -// True if we were able to insert the local into SSA. False if we gave up -// (due to hitting internal limits). +// True if the use was in a reachable block and thus has a reaching def; +// otherwise false. // // Remarks: // All uses are required to never read an uninitialized value of the local. // That is, this function requires that all paths through the function go -// through one of the defs in "defs" before any use in "uses". +// through one of the defs in "defs" before any use in "uses" for uses that +// are statically reachable. // -bool SsaBuilder::InsertInSsa(Compiler* comp, - unsigned lclNum, - ArrayStack& defs, - ArrayStack& uses) +bool IncrementalSsaBuilder::InsertUse(const UseDefLocation& use) { - LclVarDsc* dsc = comp->lvaGetDesc(lclNum); - assert(!dsc->lvInSsa); + assert(m_finalizedDefs); - JITDUMP("Putting V%02u into SSA form\n", lclNum); - JITDUMP(" %d defs:", defs.Height()); - for (int i = 0; i < defs.Height(); i++) - { - JITDUMP(" [%06u]", Compiler::dspTreeID(defs.Bottom(i).Tree)); - } + JITDUMP("Inserting use [%06u] into SSA\n", Compiler::dspTreeID(use.Tree)); - JITDUMP("\n %d uses:", uses.Height()); - for (int i = 0; i < uses.Height(); i++) + UseDefLocation reachingDef; + if (m_defs.Height() == 1) { - JITDUMP(" [%06u]", Compiler::dspTreeID(uses.Bottom(i).Tree)); + reachingDef = m_defs.BottomRef(0); } - - JITDUMP("\n"); - - if (defs.Height() == 1) + else { - JITDUMP(" Single-def local; putting into SSA directly\n"); - - UseDefLocation& def = defs.BottomRef(0); - - unsigned ssaNum = dsc->lvPerSsaData.AllocSsaNum(comp->getAllocator(CMK_SSA), def.Block, def.Tree); - def.Tree->SetSsaNum(ssaNum); - JITDUMP(" [%06u] d:%u\n", Compiler::dspTreeID(def.Tree), ssaNum); - - LclSsaVarDsc* ssaDsc = dsc->GetPerSsaData(ssaNum); - ssaDsc->m_vnPair = def.Tree->Data()->gtVNPair; - - IncrementalLiveInBuilder liveIn(comp); - - for (int i = 0; i < uses.Height(); i++) + if (!m_comp->m_dfsTree->Contains(use.Block)) { - UseDefLocation& use = uses.BottomRef(i); - use.Tree->SetSsaNum(ssaNum); - ssaDsc->AddUse(use.Block); - JITDUMP(" [%06u] u:%u\n", Compiler::dspTreeID(use.Tree), ssaNum); - - liveIn.MarkLiveInBackwards(lclNum, use, def); + JITDUMP(" Use is in unreachable block " FMT_BB "\n", use.Block->bbNum); + return false; } - dsc->lvInSsa = true; - return true; + reachingDef = FindOrCreateReachingDef(use); } - if (comp->m_dfsTree == nullptr) - { - comp->m_dfsTree = comp->fgComputeDfs(); - } + JITDUMP(" Reaching def is [%06u] d:%d\n", Compiler::dspTreeID(reachingDef.Tree), reachingDef.Tree->GetSsaNum()); - if (comp->m_domTree == nullptr) - { - comp->m_domTree = FlowGraphDominatorTree::Build(comp->m_dfsTree); - } + use.Tree->SetSsaNum(reachingDef.Tree->GetSsaNum()); + m_liveInBuilder.MarkLiveInBackwards(m_lclNum, use, reachingDef); - if (comp->m_domFrontiers == nullptr) - { - comp->m_domFrontiers = FlowGraphDominanceFrontiers::Build(comp->m_domTree); - } - - IncrementalSsaBuilder builder(comp, lclNum, defs, uses); - if (builder.Insert()) - { - dsc->lvInSsa = true; - return true; - } - - return false; + LclVarDsc* dsc = m_comp->lvaGetDesc(m_lclNum); + dsc->GetPerSsaData(reachingDef.Tree->GetSsaNum())->AddUse(use.Block); + return true; } diff --git a/src/coreclr/jit/ssabuilder.h b/src/coreclr/jit/ssabuilder.h index 446f7894e1da4..8d738ed790553 100644 --- a/src/coreclr/jit/ssabuilder.h +++ b/src/coreclr/jit/ssabuilder.h @@ -53,10 +53,6 @@ class SsaBuilder // variable are stored in the "per SSA data" on the local descriptor. void Build(); - static bool InsertInSsa(Compiler* comp, - unsigned lclNum, - ArrayStack& defs, - ArrayStack& uses); private: // Insert a new GT_PHI statement. static Statement* InsertPhi(Compiler* comp, BasicBlock* block, unsigned lclNum); @@ -109,3 +105,51 @@ class SsaBuilder CompAllocator m_allocator; SsaRenameState m_renameStack; }; + +class IncrementalLiveInBuilder +{ + Compiler* m_comp; + ArrayStack m_queue; + +public: + IncrementalLiveInBuilder(Compiler* comp) + : m_comp(comp) + , m_queue(comp->getAllocator(CMK_SSA)) + { + } + + void MarkLiveInBackwards(unsigned lclNum, const UseDefLocation& use, const UseDefLocation& reachingDef); +}; + +class IncrementalSsaBuilder +{ + Compiler* m_comp; + unsigned m_lclNum; + ArrayStack m_defs; + BitVecTraits m_poTraits; + BitVec m_defBlocks = BitVecOps::UninitVal(); + BitVec m_iteratedDominanceFrontiers = BitVecOps::UninitVal(); + IncrementalLiveInBuilder m_liveInBuilder; + +#ifdef DEBUG + bool m_finalizedDefs = false; +#endif + + UseDefLocation FindOrCreateReachingDef(const UseDefLocation& use); + bool FindReachingDefInBlock(const UseDefLocation& use, BasicBlock* block, UseDefLocation* def); + bool FindReachingDefInSameStatement(const UseDefLocation& use, UseDefLocation* def); + Statement* LatestStatement(Statement* stmt1, Statement* stmt2); +public: + IncrementalSsaBuilder(Compiler* comp, unsigned lclNum) + : m_comp(comp) + , m_lclNum(lclNum) + , m_defs(comp->getAllocator(CMK_SSA)) + , m_poTraits(0, comp) + , m_liveInBuilder(comp) + { + } + + void InsertDef(const UseDefLocation& def); + bool FinalizeDefs(); + bool InsertUse(const UseDefLocation& use); +}; diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 1fd0a2711ba70..0e607e7642d4e 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -12719,7 +12719,8 @@ void Compiler::fgValueNumberTree(GenTree* tree) // Record non-constant value numbers that are used as the length argument to bounds checks, so // that assertion prop will know that comparisons against them are worth analyzing. - ValueNum lengthVN = tree->AsBoundsChk()->GetArrayLength()->gtVNPair.GetConservative(); + ValueNum lengthVN = + vnStore->VNNormalValue(tree->AsBoundsChk()->GetArrayLength()->gtVNPair.GetConservative()); if ((lengthVN != ValueNumStore::NoVN) && !vnStore->IsVNConstant(lengthVN)) { vnStore->SetVNIsCheckedBound(lengthVN);