diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 4a15e9b2a9269f..93c9219bb03d1c 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -42,12 +42,18 @@ bool IntegralRange::Contains(int64_t value) const // block - the BasicBlock in which "tree" is being evaluated. // assertions - the set of assertions to consider when computing the range. Can be null. // fast - fast is when we only use VN and assertions. slow is when we perform an SSA walk to compute the range. +// preferredBoundVN - (slow only) the bound VN to prefer for symbolic limits. // // Return Value: // Range of possible values for "tree" based on the given assertions and block context. // An unknown range if the range cannot be determined, or if the computation exceeds the visit budget. // -static Range GetRange(Compiler* comp, GenTree* tree, BasicBlock* block, ASSERT_VALARG_TP assertions, bool fast = true) +static Range GetRange(Compiler* comp, + GenTree* tree, + BasicBlock* block, + ASSERT_VALARG_TP assertions, + bool fast = true, + ValueNum preferredBoundVN = ValueNumStore::NoVN) { assert(block != nullptr); assert(tree != nullptr); @@ -73,13 +79,63 @@ static Range GetRange(Compiler* comp, GenTree* tree, BasicBlock* block, ASSERT_V } Range range = Limit(Limit::keUndef); - if (comp->GetRangeCheck(budget)->TryGetRange(block, tree, &range)) + if (comp->GetRangeCheck(budget)->TryGetRange(block, tree, &range, preferredBoundVN)) { return range; } return Limit(Limit::keUnknown); } +//------------------------------------------------------------------------ +// GetBoundMinusXRange: Compute the range of "Bnd - X" (e.g. "span.Length - i") from +// the range of X computed relative to "Bnd". +// +// Arguments: +// comp - the Compiler object. +// tree - either the SUB node or a local defined by it. +// block - the BasicBlock in which "tree" is being evaluated. +// bndVN - the never negative checked bound. +// +// Return Value: +// Range of "tree", or an unknown range. +// +static Range GetBoundMinusXRange(Compiler* comp, GenTree* tree, BasicBlock* block, ValueNum bndVN) +{ + // Walk through local copies, e.g. CSE uses "tmp = cse" and defs "tmp = COMMA(STORE(cse, Bnd - X), cse)". + // SSA copy chains are acyclic (they end at a PHI or a non-local value), so this terminates. + tree = tree->gtEffectiveVal(); + while (tree->OperIs(GT_LCL_VAR) && tree->AsLclVar()->HasSsaName()) + { + LclVarDsc* varDsc = comp->lvaGetDesc(tree->AsLclVar()); + LclSsaVarDsc* ssaDef = varDsc->GetPerSsaData(tree->AsLclVar()->GetSsaNum()); + GenTreeLclVarCommon* defStore = ssaDef->GetDefNode(); + if (!varDsc->TypeIs(TYP_INT) || (defStore == nullptr) || !defStore->OperIs(GT_STORE_LCL_VAR)) + { + return Limit(Limit::keUnknown); + } + tree = defStore->Data()->gtEffectiveVal(); + block = ssaDef->GetBlock(); + } + + if (!tree->OperIs(GT_SUB) || !tree->TypeIs(TYP_INT) || (comp->optConservativeNormalVN(tree->gtGetOp1()) != bndVN)) + { + return Limit(Limit::keUnknown); + } + + // X in [lo, Bnd + k] with lo >= 0 and k <= 0 means that "Bnd - X" can't overflow and is in [-k, Bnd - lo]. + // k == INT32_MIN is excluded since "-k" would overflow. + Range xRange = GetRange(comp, tree->gtGetOp2(), block, BitVecOps::UninitVal(), /*fast*/ false, bndVN); + const Limit& xLo = xRange.LowerLimit(); + const Limit& xHi = xRange.UpperLimit(); + if (xLo.IsConstant() && (xLo.GetConstant() >= 0) && xHi.IsBinOpArray() && (xHi.vn == bndVN) && + (xHi.GetConstant() <= 0) && (xHi.GetConstant() > INT32_MIN)) + { + return Range(Limit(Limit::keConstant, -xHi.GetConstant()), + Limit(Limit::keBinOpArray, bndVN, -xLo.GetConstant())); + } + return Limit(Limit::keUnknown); +} + #if defined(FEATURE_HW_INTRINSICS) //---------------------------------------------------------------------------------------------- // optAssertionProp_HWIntrinsic: Propagate VN-derived facts to local var metadata. @@ -4470,6 +4526,59 @@ GenTree* Compiler::optAssertionProp_RelOp(ASSERT_VALARG_TP assertions, return optAssertionPropLocal_RelOp(assertions, tree, stmt); } +//------------------------------------------------------------------------ +// optIsNeverNegativeCheckedBound: Check whether "vn" is a checked bound known to be never negative, +// which is required to express symbolic range limits with it. +// +// Arguments: +// vn - the value number to check +// assertions - set of live assertions +// +// Returns: +// true if "vn" is a never negative checked bound. +// +bool Compiler::optIsNeverNegativeCheckedBound(ValueNum vn, ASSERT_VALARG_TP assertions) +{ + if (!vnStore->IsVNCheckedBound(vn)) + { + return false; + } + Range rng = RangeCheck::GetRangeFromAssertions(this, vn, assertions); + return rng.LowerLimit().IsConstant() && (rng.LowerLimit().GetConstant() >= 0); +} + +//------------------------------------------------------------------------ +// optGetPreferredBoundForRange: Pick a checked bound to express symbolic range limits of "vn" with. +// +// Arguments: +// vn - the value number to compute the range for +// assertions - set of live assertions +// +// Returns: +// "Bnd" when there is a "vn relop (Bnd + cns)" assertion, NoVN otherwise. +// +ValueNum Compiler::optGetPreferredBoundForRange(ValueNum vn, ASSERT_VALARG_TP assertions) +{ + if (BitVecOps::MayBeUninit(assertions) || !optAssertionHasAssertionsForVN(vn)) + { + return ValueNumStore::NoVN; + } + + BitVecOps::Iter iter(apTraits, assertions); + unsigned index = 0; + while (iter.NextElem(&index)) + { + const AssertionDsc& curAssertion = optGetAssertion(GetAssertionIndex(index)); + if (curAssertion.IsRelop() && (curAssertion.GetOp1().GetVN() == vn) && + curAssertion.GetOp2().KindIs(O2K_VN_ADD_CNS) && + optIsNeverNegativeCheckedBound(curAssertion.GetOp2().GetVN(), assertions)) + { + return curAssertion.GetOp2().GetVN(); + } + } + return ValueNumStore::NoVN; +} + //------------------------------------------------------------------------ // optAssertionProp: try and optimize a relop via assertion propagation // @@ -4614,14 +4723,38 @@ GenTree* Compiler::optAssertionPropGlobal_RelOp(ASSERT_VALARG_TP assertions, } // Perform a slow, SSA-based range check analysis. - if (!relopRange.IsSingleValueConstant(&relopResult) && - // The few checks below ensure this analysis - // is only performed when beneficial according to SPMI, keeping the TP impact low. - op1->TypeIs(TYP_INT) && op2->IsIntCnsFitsInI32() && tree->OperIs(GT_LE, GT_LT, GT_GE, GT_GT)) - { - Range op1Rng = GetRange(this, op1, block, assertions, /*fast*/ false); - Range op2Rng = GetRange(this, op2, block, assertions, /*fast*/ false); - relopRange = RangeOps::EvalRelop(tree->OperGet(), tree->IsUnsigned(), op1Rng, op2Rng); + // The few checks below ensure this analysis is only performed when beneficial + // according to SPMI, keeping the TP impact low. + if (!relopRange.IsSingleValueConstant(&relopResult) && op1->TypeIs(TYP_INT) && + tree->OperIs(GT_LE, GT_LT, GT_GE, GT_GT)) + { + if (op2->IsIntCnsFitsInI32()) + { + Range op1Rng = Limit(Limit::keUnknown); + ValueNum bndVN; + if (vnStore->IsVNBinFunc(op1VN, VNF_SUB, &bndVN) && + optIsNeverNegativeCheckedBound(bndVN, assertions)) + { + op1Rng = GetBoundMinusXRange(this, op1, block, bndVN); + } + + if (op1Rng.LowerLimit().IsUnknown()) + { + op1Rng = GetRange(this, op1, block, assertions, /*fast*/ false, + optGetPreferredBoundForRange(op1VN, assertions)); + } + Range op2Rng = GetRange(this, op2, block, assertions, /*fast*/ false); + relopRange = RangeOps::EvalRelop(tree->OperGet(), tree->IsUnsigned(), op1Rng, op2Rng); + } + else if (tree->OperIs(GT_GT, GT_LE) && tree->IsUnsigned() && op2->TypeIs(TYP_INT) && + optIsNeverNegativeCheckedBound(op2VN, assertions)) + { + // "(uint)X > (uint)Bnd", e.g. "(uint)start > (uint)_length" in Span.Slice. + // LT/GE forms are mostly loop conditions that aren't worth the TP. + Range op1Rng = GetRange(this, op1, block, assertions, /*fast*/ false, op2VN); + relopRange = RangeOps::EvalRelop(tree->OperGet(), /*isUnsigned*/ true, op1Rng, + Range(Limit(Limit::keBinOpArray, op2VN, 0))); + } } if (relopRange.IsSingleValueConstant(&relopResult)) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 95d461d4a44536..fe6916141d65f8 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -9407,6 +9407,8 @@ class Compiler Statement* stmt, BasicBlock* block); GenTree* optAssertionPropLocal_RelOp(ASSERT_VALARG_TP assertions, GenTree* tree, Statement* stmt); + bool optIsNeverNegativeCheckedBound(ValueNum vn, ASSERT_VALARG_TP assertions); + ValueNum optGetPreferredBoundForRange(ValueNum vn, ASSERT_VALARG_TP assertions); GenTree* optAssertionProp_Update(GenTree* newTree, GenTree* tree, Statement* stmt); GenTree* optNonNullAssertionProp_Call(ASSERT_VALARG_TP assertions, GenTreeCall* call); bool optNonNullAssertionProp_Ind(ASSERT_VALARG_TP assertions, GenTree* indir); diff --git a/src/coreclr/jit/rangecheck.cpp b/src/coreclr/jit/rangecheck.cpp index 1e2fbd0cc8971e..d282c0b303e4b5 100644 --- a/src/coreclr/jit/rangecheck.cpp +++ b/src/coreclr/jit/rangecheck.cpp @@ -297,16 +297,44 @@ void RangeCheck::Widen(BasicBlock* block, GenTree* tree, Range* pRange) Range& range = *pRange; - // Try to deduce the lower bound, if it is not known already. - if (range.LowerLimit().IsDependent() || range.LowerLimit().IsUnknown()) + // Try to deduce the lower bound, if it is not known already. A negative constant lower bound is + // also worth improving: e.g. a weak "i > len - 8" loop-exit bound merged with the initial value. + const Limit lowerLimit = range.LowerLimit(); + if (lowerLimit.IsDependent() || lowerLimit.IsUnknown() || + (lowerLimit.IsConstant() && (lowerLimit.GetConstant() < 0))) { // To determine the lower bound, ask if the loop increases monotonically. bool increasing = IsMonotonicallyIncreasing(tree, false); if (increasing) { JITDUMP("[%06d] is monotonically increasing.\n", Compiler::dspTreeID(tree)); + + // Monotonicity assumes the defs don't overflow, but DoesVarDefOverflow skips that check for + // a non-dependent lower limit. Re-check with a dependent one, as for the dependent case. + if (lowerLimit.IsConstant() && + ComputeDoesOverflow(block, tree, Range(Limit(Limit::keDependent), range.UpperLimit()))) + { + JITDUMP("[%06d] may overflow, not widening.\n", Compiler::dspTreeID(tree)); + return; + } + ClearRangeMap(); - *pRange = GetRangeWorker(block, tree, true DEBUGARG(0)); + Range widened = GetRangeWorker(block, tree, true DEBUGARG(0)); + if (!lowerLimit.IsConstant()) + { + *pRange = widened; + } + else if (widened.LowerLimit().IsConstant() && + (widened.LowerLimit().GetConstant() > lowerLimit.GetConstant())) + { + // Both ranges are valid, so keep the original upper limit. + Range tightened = range; + tightened.lLimit = widened.LowerLimit(); + if (tightened.IsValid()) + { + *pRange = tightened; + } + } } } } @@ -1137,6 +1165,102 @@ void RangeCheck::MergeEdgeAssertions(Compiler* comp, /* budget */ 3, &visited); } +//------------------------------------------------------------------------ +// IsBoundLessOrEqual: Check whether the assertions prove "lo <= hi" (or "lo == hi" when equalOnly) +// via a short chain of assertions, e.g. "a == b" and "b <= c" prove "a <= c". +// +// Arguments: +// comp - the compiler instance +// assertions - the assertions to use +// lo - the value number expected to be the smaller one +// hi - the value number expected to be the bigger one +// equalOnly - only follow "==" assertions +// budget - the maximum chain length +// +// Return Value: +// true if the assertions prove "lo <= hi" (or "lo == hi" when equalOnly). +// +static bool IsBoundLessOrEqual( + Compiler* comp, ASSERT_VALARG_TP assertions, ValueNum lo, ValueNum hi, bool equalOnly, int budget) +{ + if (lo == hi) + { + return true; + } + if ((budget <= 0) || !comp->optAssertionHasAssertionsForVN(lo)) + { + return false; + } + + BitVecOps::Iter iter(comp->apTraits, assertions); + unsigned index = 0; + while (iter.NextElem(&index)) + { + const Compiler::AssertionDsc& curAssertion = comp->optGetAssertion(GetAssertionIndex(index)); + if (!curAssertion.GetOp1().KindIs(Compiler::O1K_VN) || + !curAssertion.GetOp2().KindIs(Compiler::O2K_VN_ADD_CNS) || (curAssertion.GetOp2().GetCns() != 0)) + { + continue; + } + + ValueNum op1VN = curAssertion.GetOp1().GetVN(); + ValueNum op2VN = curAssertion.GetOp2().GetVN(); + ValueNum next = ValueNumStore::NoVN; + if (curAssertion.KindIs(Compiler::OAK_EQUAL)) + { + next = (op1VN == lo) ? op2VN : ((op2VN == lo) ? op1VN : ValueNumStore::NoVN); + } + else if (!equalOnly && curAssertion.KindIs(Compiler::OAK_LE, Compiler::OAK_LT) && (op1VN == lo)) + { + next = op2VN; + } + else if (!equalOnly && curAssertion.KindIs(Compiler::OAK_GE, Compiler::OAK_GT) && (op2VN == lo)) + { + next = op1VN; + } + + if ((next != ValueNumStore::NoVN) && IsBoundLessOrEqual(comp, assertions, next, hi, equalOnly, budget - 1)) + { + return true; + } + } + return false; +} + +//------------------------------------------------------------------------ +// CanUsePreferredBound: Check whether "X (Bnd + cns)" assertion can be expressed as +// "X (PreferredBnd + cns)". +// +// Arguments: +// comp - the compiler instance +// assertions - the assertions to use +// assertion - the "X (Bnd + cns)" assertion +// preferredBoundVN - the never negative preferred bound +// +// Return Value: +// true if "Bnd == PreferredBnd", or "Bnd <= PreferredBnd" for an upper limit. +// +static bool CanUsePreferredBound(Compiler* comp, + ASSERT_VALARG_TP assertions, + const Compiler::AssertionDsc& assertion, + ValueNum preferredBoundVN) +{ + const int maxChain = 3; + const ValueNum boundVN = assertion.GetOp2().GetVN(); + const int boundCns = assertion.GetOp2().GetCns(); + + // "X <= Bnd + cns" and "Bnd <= PreferredBnd" imply "X <= PreferredBnd + cns" when neither + // "Bnd + cns" nor "PreferredBnd + cns" can overflow: cns <= 0 and Bnd is never negative. + if (assertion.KindIs(Compiler::OAK_LT, Compiler::OAK_LE) && (boundCns <= 0) && + IsBoundLessOrEqual(comp, assertions, boundVN, preferredBoundVN, /*equalOnly*/ false, maxChain) && + ((boundCns == 0) || assertion.GetOp2().IsVNNeverNegative() || + comp->optIsNeverNegativeCheckedBound(boundVN, assertions))) + { + return true; + } + return IsBoundLessOrEqual(comp, assertions, boundVN, preferredBoundVN, /*equalOnly*/ true, maxChain); +} + //------------------------------------------------------------------------ // MergeEdgeAssertionsWorker: Worker for MergeEdgeAssertions that takes a visited set and recursion // budget for VN-to-VN assertion lookups. @@ -1309,9 +1433,18 @@ void RangeCheck::MergeEdgeAssertionsWorker(Compiler* comp ((boundCns != 0) && comp->vnStore->IsVNBinFuncWithConst(normalLclVN, VNF_ADD, &addOp, &addCns) && (addOp == boundVN) && (addCns == boundCns)); - // A bound checked on another path need not be nonnegative here. + // The bound is related to the preferred bound, e.g. "a.Length == b.Length" or "a.Length <= b.Length". + // Checked first so the limit is expressed via the preferred bound even if the bound is never negative. if (canUseCheckedBounds && (normalLclVN == curAssertion.GetOp1().GetVN()) && - (curAssertion.GetOp2().IsVNNeverNegative() || (boundVN == preferredBoundVN))) + (preferredBoundVN != ValueNumStore::NoVN) && (boundVN != preferredBoundVN) && + CanUsePreferredBound(comp, assertions, curAssertion, preferredBoundVN)) + { + cmpOper = Compiler::AssertionDsc::ToCompareOper(curAssertion.GetKind(), &isUnsigned); + limit = Limit(Limit::keBinOpArray, preferredBoundVN, boundCns); + } + // A bound checked on another path need not be nonnegative here. + else if (canUseCheckedBounds && (normalLclVN == curAssertion.GetOp1().GetVN()) && + (curAssertion.GetOp2().IsVNNeverNegative() || (boundVN == preferredBoundVN))) { cmpOper = Compiler::AssertionDsc::ToCompareOper(curAssertion.GetKind(), &isUnsigned); limit = Limit(Limit::keBinOpArray, boundVN, boundCns); diff --git a/src/coreclr/jit/rangecheck.h b/src/coreclr/jit/rangecheck.h index ffbbf09c9d4538..9acb96f37213d9 100644 --- a/src/coreclr/jit/rangecheck.h +++ b/src/coreclr/jit/rangecheck.h @@ -764,28 +764,44 @@ struct RangeOps const Limit& xUpper = x.UpperLimit(); const Limit& yUpper = y.UpperLimit(); + // Computes "a - b" when both limits are constants or both are "$bnd + cns" with the same $bnd. + // $bnd is never negative, so "$bnd + cns" can't overflow (and is exact) when cns <= 0. + auto limitDiff = [](const Limit& a, const Limit& b, int64_t* diff) -> bool { + if ((a.IsConstant() && b.IsConstant()) || (a.IsBinOpArray() && b.IsBinOpArray() && (a.vn == b.vn) && + (a.GetConstant() <= 0) && (b.GetConstant() <= 0))) + { + *diff = (int64_t)a.GetConstant() - (int64_t)b.GetConstant(); + return true; + } + return false; + }; + // For unsigned comparisons, we only support non-negative ranges. // NOTE: it's not applicable for EQ and NE. if (isUnsigned && (relop != GT_EQ) && (relop != GT_NE)) { - if (!xLower.IsConstant() || !yLower.IsConstant() || (xLower.GetConstant() < 0) || - (yLower.GetConstant() < 0)) + auto isNonNegative = [](const Limit& l) { + return (l.IsConstant() && (l.GetConstant() >= 0)) || (l.IsBinOpArray() && (l.GetConstant() == 0)); + }; + + if (!isNonNegative(xLower) || !isNonNegative(yLower)) { // Relops always return either 0 or 1. return Range(Limit(Limit::keConstant, 0), Limit(Limit::keConstant, 1)); } } + int64_t diff; switch (relop) { case GT_GE: case GT_LT: - if (xLower.IsConstant() && yUpper.IsConstant() && (xLower.GetConstant() >= yUpper.GetConstant())) + if (limitDiff(xLower, yUpper, &diff) && (diff >= 0)) { return Range(Limit(Limit::keConstant, relop == GT_GE ? 1 : 0)); } - if (xUpper.IsConstant() && yLower.IsConstant() && (xUpper.GetConstant() < yLower.GetConstant())) + if (limitDiff(xUpper, yLower, &diff) && (diff < 0)) { return Range(Limit(Limit::keConstant, relop == GT_GE ? 0 : 1)); } @@ -793,12 +809,12 @@ struct RangeOps case GT_GT: case GT_LE: - if (xLower.IsConstant() && yUpper.IsConstant() && (xLower.GetConstant() > yUpper.GetConstant())) + if (limitDiff(xLower, yUpper, &diff) && (diff > 0)) { return Range(Limit(Limit::keConstant, relop == GT_GT ? 1 : 0)); } - if (xUpper.IsConstant() && yLower.IsConstant() && (xUpper.GetConstant() <= yLower.GetConstant())) + if (limitDiff(xUpper, yLower, &diff) && (diff <= 0)) { return Range(Limit(Limit::keConstant, relop == GT_GT ? 0 : 1)); } @@ -808,8 +824,8 @@ struct RangeOps case GT_NE: // If the ranges do not overlap, then EQ is always false, NE is always true. // Example: x = [6..10], y = [0..5] -> EQ is always false, NE is always true. - if ((xLower.IsConstant() && yUpper.IsConstant() && (xLower.GetConstant() > yUpper.GetConstant())) || - (xUpper.IsConstant() && yLower.IsConstant() && (xUpper.GetConstant() < yLower.GetConstant()))) + if ((limitDiff(xLower, yUpper, &diff) && (diff > 0)) || + (limitDiff(xUpper, yLower, &diff) && (diff < 0))) { return Range(Limit(Limit::keConstant, relop == GT_EQ ? 0 : 1)); }