diff --git a/src/coreclr/jit/compiler.hpp b/src/coreclr/jit/compiler.hpp index 0ff1ab40625d97..0472160d20f3d2 100644 --- a/src/coreclr/jit/compiler.hpp +++ b/src/coreclr/jit/compiler.hpp @@ -5626,8 +5626,13 @@ Compiler::AssertVisit Compiler::optVisitReachingAssertions(ValueNum vn, TAssertV return AssertVisit::Abort; } - const ValueNum phiArgVN = vnStore->VNConservativeNormalValue(phiArg->gtVNPair); - ASSERT_TP assertions = optGetEdgeAssertions(ssaDef->GetBlock(), phiArg->gtPredBB); + // fgValueNumberPhiDef may leave gtVNPair as NoVN when it refuses to back-patch + // a loop-varying SSA-def VN into the phi-arg slot (a hygiene constraint for general + // gtVNPair consumers that does not apply here, since we hand the VN to the visitor + // together with this edge's assertion set and never write it back into any tree). + LclSsaVarDsc* phiArgSsaDef = lvaGetDesc(phiArg)->GetPerSsaData(phiArg->GetSsaNum()); + ValueNum phiArgVN = vnStore->VNConservativeNormalValue(phiArgSsaDef->m_vnPair); + ASSERT_TP assertions = optGetEdgeAssertions(ssaDef->GetBlock(), phiArg->gtPredBB); if (argVisitor(phiArgVN, assertions) == AssertVisit::Abort) { // The visitor wants to abort the walk. diff --git a/src/coreclr/jit/rangecheck.cpp b/src/coreclr/jit/rangecheck.cpp index dca63fb6efdb2a..a8f9ecd4e4a3fd 100644 --- a/src/coreclr/jit/rangecheck.cpp +++ b/src/coreclr/jit/rangecheck.cpp @@ -650,11 +650,34 @@ void RangeCheck::MergeEdgeAssertions(GenTreeLclVarCommon* lcl, ASSERT_VALARG_TP // comp - the compiler instance // num - the value number to analyze range for // assertions - the assertions to use +// budget - the remaining budget for recursive analysis // // Return Value: // The computed range // Range RangeCheck::GetRangeFromAssertions(Compiler* comp, ValueNum num, ASSERT_VALARG_TP assertions, int budget) +{ + ValueNumStore::SmallValueNumSet set; + return GetRangeFromAssertionsWorker(comp, num, assertions, budget, &set); +} + +//------------------------------------------------------------------------ +// GetRangeFromAssertionsWorker: Cheaper version of TryGetRange that is based purely on assertions +// and does not require a full range analysis based on SSA. +// +// Arguments: +// comp - the compiler instance +// num - the value number to analyze range for +// assertions - the assertions to use +// budget - the remaining budget for recursive analysis +// visited - the set of value numbers already visited in the current search +// path to prevent infinite recursion +// +// Return Value: +// The computed range +// +Range RangeCheck::GetRangeFromAssertionsWorker( + Compiler* comp, ValueNum num, ASSERT_VALARG_TP assertions, int budget, ValueNumStore::SmallValueNumSet* visited) { // Start with the widest possible constant range. Range result = Range(Limit(Limit::keConstant, INT32_MIN), Limit(Limit::keConstant, INT32_MAX)); @@ -699,7 +722,8 @@ Range RangeCheck::GetRangeFromAssertions(Compiler* comp, ValueNum num, ASSERT_VA // if its range is within the castTo range, we can use that (and the cast is basically a no-op). if (comp->vnStore->TypeOfVN(funcApp.m_args[0]) == TYP_INT) { - Range castOpRange = GetRangeFromAssertions(comp, funcApp.m_args[0], assertions, --budget); + Range castOpRange = + GetRangeFromAssertionsWorker(comp, funcApp.m_args[0], assertions, --budget, visited); if (castOpRange.IsConstantRange() && (castOpRange.LowerLimit().GetConstant() >= castToTypeRange.LowerLimit().GetConstant()) && (castOpRange.UpperLimit().GetConstant() <= castToTypeRange.UpperLimit().GetConstant())) @@ -713,7 +737,7 @@ Range RangeCheck::GetRangeFromAssertions(Compiler* comp, ValueNum num, ASSERT_VA case VNF_NEG: { - Range r1 = GetRangeFromAssertions(comp, funcApp.m_args[0], assertions, --budget); + Range r1 = GetRangeFromAssertionsWorker(comp, funcApp.m_args[0], assertions, --budget, visited); Range unaryOpResult = RangeOps::Negate(r1); // We can use the result only if it never overflows. @@ -732,8 +756,8 @@ Range RangeCheck::GetRangeFromAssertions(Compiler* comp, ValueNum num, ASSERT_VA case VNF_UMOD: { // Get ranges of both operands and perform the same operation on the ranges. - Range r1 = GetRangeFromAssertions(comp, funcApp.m_args[0], assertions, --budget); - Range r2 = GetRangeFromAssertions(comp, funcApp.m_args[1], assertions, --budget); + Range r1 = GetRangeFromAssertionsWorker(comp, funcApp.m_args[0], assertions, --budget, visited); + Range r2 = GetRangeFromAssertionsWorker(comp, funcApp.m_args[1], assertions, --budget, visited); Range binOpResult = Range(Limit(Limit::keUnknown)); switch (funcApp.m_func) { @@ -799,8 +823,8 @@ Range RangeCheck::GetRangeFromAssertions(Compiler* comp, ValueNum num, ASSERT_VA if ((genActualType(comp->vnStore->TypeOfVN(funcApp.m_args[0])) == TYP_INT) && (genActualType(comp->vnStore->TypeOfVN(funcApp.m_args[1])) == TYP_INT)) { - Range r1 = GetRangeFromAssertions(comp, funcApp.m_args[0], assertions, --budget); - Range r2 = GetRangeFromAssertions(comp, funcApp.m_args[1], assertions, --budget); + Range r1 = GetRangeFromAssertionsWorker(comp, funcApp.m_args[0], assertions, --budget, visited); + Range r2 = GetRangeFromAssertionsWorker(comp, funcApp.m_args[1], assertions, --budget, visited); bool isUnsigned = true; genTreeOps cmpOper; @@ -846,9 +870,9 @@ Range RangeCheck::GetRangeFromAssertions(Compiler* comp, ValueNum num, ASSERT_VA } Range phiRange = Range(Limit(Limit::keUndef)); - auto visitor = [comp, &phiRange, &budget](ValueNum reachingVN, ASSERT_TP reachingAssertions) { - // call GetRangeFromAssertions for each reaching VN using reachingAssertions - Range edgeRange = GetRangeFromAssertions(comp, reachingVN, reachingAssertions, --budget); + auto visitor = [comp, &phiRange, &budget, visited](ValueNum reachingVN, ASSERT_TP reachingAssertions) { + // call GetRangeFromAssertionsWorker for each reaching VN using reachingAssertions + Range edgeRange = GetRangeFromAssertionsWorker(comp, reachingVN, reachingAssertions, --budget, visited); // If phiRange is not yet set, set it to the first edgeRange // else merge it with the new edgeRange. Example: [10..100] U [50..150] = [10..150] @@ -864,7 +888,9 @@ Range RangeCheck::GetRangeFromAssertions(Compiler* comp, ValueNum num, ASSERT_VA return Compiler::AssertVisit::Abort; }; - if (comp->optVisitReachingAssertions(num, visitor) == Compiler::AssertVisit::Continue && !phiRange.IsUndef()) + // If it's a phi, we need to look at the reaching assertions for each of the phi args and merge them together. + if (comp->vnStore->IsPhiDef(num) && visited->Add(comp, num) && + (comp->optVisitReachingAssertions(num, visitor) == Compiler::AssertVisit::Continue) && !phiRange.IsUndef()) { assert(phiRange.IsConstantRange()); result = phiRange; @@ -1082,17 +1108,38 @@ void RangeCheck::MergeEdgeAssertions(Compiler* comp, else if (curAssertion.KindIs(Compiler::OAK_GE, Compiler::OAK_GT, Compiler::OAK_LE, Compiler::OAK_LT) && curAssertion.GetOp2().KindIs(Compiler::O2K_CHECKED_BOUND_ADD_CNS)) { + const ValueNum boundVN = curAssertion.GetOp2().GetCheckedBound(); + const int boundCns = curAssertion.GetOp2().GetCheckedBoundConstant(); + + // Check whether normalLclVN VN-equals the op2 expression "(boundVN + boundCns)": + // - trivially, when boundCns is 0 and normalLclVN == boundVN, or + // - when normalLclVN itself is the value-number "ADD(boundVN, boundCns)". + // + // The general case commonly arises on a loop back-edge where the induction variable + // is e.g. V = phi(V0, V - 8), and the back-edge JTRUE generates an assertion of the + // form "8 <= (V + -8)" against the phi's VN. We want to apply that assertion to the + // value of the back-edge phi-arg "(V - 8)" itself. + // + // Note: when boundCns == 0, the trivial check above is exhaustive -- VN normalizes + // ADD(x, 0) to x, so any IsVNBinFuncWithConst match would also have to satisfy + // normalLclVN == boundVN, which the trivial check already covers. Skip the lookup + // in that case to keep this fast-path cheap. + ValueNum addOp; + int addCns; + const bool normalLclVNMatchesOp2 = + ((normalLclVN == boundVN) && (boundCns == 0)) || + ((boundCns != 0) && comp->vnStore->IsVNBinFuncWithConst(normalLclVN, VNF_ADD, &addOp, &addCns) && + (addOp == boundVN) && (addCns == boundCns)); + if (canUseCheckedBounds && (normalLclVN == curAssertion.GetOp1().GetVN())) { cmpOper = Compiler::AssertionDsc::ToCompareOper(curAssertion.GetKind(), &isUnsigned); - limit = Limit(Limit::keBinOpArray, curAssertion.GetOp2().GetCheckedBound(), - curAssertion.GetOp2().GetCheckedBoundConstant()); + limit = Limit(Limit::keBinOpArray, boundVN, boundCns); } - else if ((normalLclVN == curAssertion.GetOp2().GetCheckedBound()) && - (curAssertion.GetOp2().GetCheckedBoundConstant() == 0)) + else if (normalLclVNMatchesOp2) { - // Check if it's "Const (checkedBndVN + 0)" or in other words "Const checkedBndVN" - // If normalLclVN == checkedBndVN, then we can deduce "normalLclVN Const" + // Check if it's "Const (checkedBndVN + cns)" or in other words "Const normalLclVN" + // If normalLclVN VN-equals (checkedBndVN + cns), we can deduce "normalLclVN Const" // // Example: We created "O1K_VN(vn1) - OAK_GT - O2K_CHECKED_BOUND_ADD_CNS(vn2, 0)" // if vn1 is a constant (say, 100) and vn2 is our normalLclVN, we can deduce "normalLclVN < 100" diff --git a/src/coreclr/jit/rangecheck.h b/src/coreclr/jit/rangecheck.h index 0d005e7acfcc23..cab98b29c85b60 100644 --- a/src/coreclr/jit/rangecheck.h +++ b/src/coreclr/jit/rangecheck.h @@ -776,6 +776,13 @@ class RangeCheck typedef JitHashTable, Range*> RangeMap; typedef JitHashTable, BasicBlock*> SearchPath; + // Cheaper version of TryGetRange that is based only on incoming assertions. + static Range GetRangeFromAssertionsWorker(Compiler* comp, + ValueNum num, + ASSERT_VALARG_TP assertions, + int budget, + ValueNumStore::SmallValueNumSet* visited); + int GetArrLength(ValueNum vn); // Check whether the computed range is within 0 and upper bounds. This function