diff --git a/src/coreclr/jit/rangecheck.cpp b/src/coreclr/jit/rangecheck.cpp index 459a2f80709534..6d3d5f5c157dce 100644 --- a/src/coreclr/jit/rangecheck.cpp +++ b/src/coreclr/jit/rangecheck.cpp @@ -869,6 +869,90 @@ Range RangeCheck::GetRangeFromAssertions(Compiler* comp, ValueNum num, ASSERT_VA return result; } +//------------------------------------------------------------------------ +// TightenLimit: Given two limits for the same variable, return a tighter limit +// Example: l1: 10 and l2: 20. For lower limit, we can tighten to 20. For upper limit, we can tighten to 10. +// +// Arguments: +// l1 - The first limit +// l2 - The second limit +// preferredBound - We might prefer this VN limit over the other. +// isLower - true if the limits are lower limits, false if the limits are upper limits. +// +// Return Value: +// The more tightened limit between l1 and l2. +// +Limit RangeCheck::TightenLimit(Limit l1, Limit l2, ValueNum preferredBound, bool isLower) +{ + // 1) One of the limits is undef, unknown or dependent + if (l1.IsUndef() || l2.IsUndef()) + { + // Anything is better than undef. + return l1.IsUndef() ? l2 : l1; + } + if (l1.IsUnknown() || l2.IsUnknown()) + { + // Anything is better than unknown. + return l1.IsUnknown() ? l2 : l1; + } + if (l1.IsDependent() || l2.IsDependent()) + { + // Anything is better than dependent. + return l1.IsDependent() ? l2 : l1; + } + + // 2) Both limits are constants + if (l1.IsConstant() && l2.IsConstant()) + { + // isLower: whatever is higher is better. + // !isLower: whatever is lower is better. + return isLower ? (l1.cns > l2.cns ? l1 : l2) : (l1.cns < l2.cns ? l1 : l2); + } + + // 3) Both limits are BinOpArray (which is "arrLen + cns") + if (l1.IsBinOpArray() && l2.IsBinOpArray()) + { + assert((l1.vn != ValueNumStore::NoVN) && (l2.vn != ValueNumStore::NoVN)); + + // If one of them is preferredBound and the other is not, use the preferredBound. + if ((l1.vn == preferredBound) && (l2.vn != preferredBound)) + { + return l1; + } + if ((l2.vn == preferredBound) && (l1.vn != preferredBound)) + { + return l2; + } + + // Otherwise, just use the one with the higher/lower constant. + // even if they use different arrLen. + return isLower ? (l1.cns > l2.cns ? l1 : l2) : (l1.cns < l2.cns ? l1 : l2); + } + + // 4) One of the limits is a constant and the other is BinOpArray + if ((l1.IsConstant() && l2.IsBinOpArray()) || (l2.IsConstant() && l1.IsBinOpArray())) + { + // l1 - BinOpArray, l2 - constant + if (l1.IsConstant()) + { + std::swap(l1, l2); + } + + assert(l1.vn != ValueNumStore::NoVN); + if (l1.vn != preferredBound) + { + // if we don't have a preferred bound, + // or it doesn't match l1.vn, use the constant (l2). + return l2; + } + + // Otherwise, prefer the BinOpArray(preferredBound) over the constant for the upper bound + // and the constant for the lower bound. + return isLower ? l2 : l1; + } + unreached(); +} + //------------------------------------------------------------------------ // MergeEdgeAssertions: Merge assertions on the edge flowing into the block about a variable // @@ -913,9 +997,37 @@ void RangeCheck::MergeEdgeAssertions(Compiler* comp, genTreeOps cmpOper = GT_NONE; bool isUnsigned = false; + // o1: (normalLclVN + CNS1) - OAK_LT_UN - o2: (preferredBoundVN + 0) + // We can deduce normalLclVN's upper bound to be preferredBoundVN - CNS1 + // Example: "(uint)(i + 2) < (uint)span.Length" + if (canUseCheckedBounds && curAssertion.KindIs(Compiler::OAK_LT_UN) && + curAssertion.GetOp2().KindIs(Compiler::O2K_CHECKED_BOUND_ADD_CNS) && + // Normally, checked bound doesn't directly imply non-negativity, so we check + // that explicitly here: + curAssertion.GetOp2().IsCheckedBoundNeverNegative() && + (curAssertion.GetOp2().GetCheckedBound() == preferredBoundVN) && + (curAssertion.GetOp1().GetVN() != normalLclVN)) + { + ValueNum addOpVN; + int addOpCns; + if (comp->vnStore->IsVNBinFuncWithConst(curAssertion.GetOp1().GetVN(), VNF_ADD, &addOpVN, &addOpCns) && + (addOpVN == normalLclVN) && (addOpCns >= 0)) + { + cmpOper = GT_LT; + limit = Limit(Limit::keBinOpArray, preferredBoundVN, -addOpCns); + isUnsigned = false; + // The comparison being unsigned may also hint that the lower bound is -CNS1, but it's + // unlikely to be useful, so we ignore it for now. The whole thing will work only if some other + // assertion proves that the normalLclVN's lower bound is non-negative. + } + else + { + continue; + } + } // Current assertion is of the form "i (checkedBndVN + cns)" - if (curAssertion.KindIs(Compiler::OAK_GE, Compiler::OAK_GT, Compiler::OAK_LE, Compiler::OAK_LT) && - curAssertion.GetOp2().KindIs(Compiler::O2K_CHECKED_BOUND_ADD_CNS)) + 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)) { if (canUseCheckedBounds && (normalLclVN == curAssertion.GetOp1().GetVN())) { @@ -1155,78 +1267,6 @@ void RangeCheck::MergeEdgeAssertions(Compiler* comp, break; } - // We have two ranges - we need to merge (tighten) them. - - auto tightenLimit = [](Limit l1, Limit l2, ValueNum preferredBound, bool isLower) -> Limit { - // 1) One of the limits is undef, unknown or dependent - if (l1.IsUndef() || l2.IsUndef()) - { - // Anything is better than undef. - return l1.IsUndef() ? l2 : l1; - } - if (l1.IsUnknown() || l2.IsUnknown()) - { - // Anything is better than unknown. - return l1.IsUnknown() ? l2 : l1; - } - if (l1.IsDependent() || l2.IsDependent()) - { - // Anything is better than dependent. - return l1.IsDependent() ? l2 : l1; - } - - // 2) Both limits are constants - if (l1.IsConstant() && l2.IsConstant()) - { - // isLower: whatever is higher is better. - // !isLower: whatever is lower is better. - return isLower ? (l1.cns > l2.cns ? l1 : l2) : (l1.cns < l2.cns ? l1 : l2); - } - - // 3) Both limits are BinOpArray (which is "arrLen + cns") - if (l1.IsBinOpArray() && l2.IsBinOpArray()) - { - assert((l1.vn != ValueNumStore::NoVN) && (l2.vn != ValueNumStore::NoVN)); - - // If one of them is preferredBound and the other is not, use the preferredBound. - if ((l1.vn == preferredBound) && (l2.vn != preferredBound)) - { - return l1; - } - if ((l2.vn == preferredBound) && (l1.vn != preferredBound)) - { - return l2; - } - - // Otherwise, just use the one with the higher/lower constant. - // even if they use different arrLen. - return isLower ? (l1.cns > l2.cns ? l1 : l2) : (l1.cns < l2.cns ? l1 : l2); - } - - // 4) One of the limits is a constant and the other is BinOpArray - if ((l1.IsConstant() && l2.IsBinOpArray()) || (l2.IsConstant() && l1.IsBinOpArray())) - { - // l1 - BinOpArray, l2 - constant - if (l1.IsConstant()) - { - std::swap(l1, l2); - } - - assert(l1.vn != ValueNumStore::NoVN); - if (l1.vn != preferredBound) - { - // if we don't have a preferred bound, - // or it doesn't match l1.vn, use the constant (l2). - return l2; - } - - // Otherwise, prefer the BinOpArray(preferredBound) over the constant for the upper bound - // and the constant for the lower bound. - return isLower ? l2 : l1; - } - unreached(); - }; - if (!assertedRange.IsValid()) { JITDUMP("assertedRange is invalid: [%s] - bail out\n", assertedRange.ToString(comp)); @@ -1237,8 +1277,8 @@ void RangeCheck::MergeEdgeAssertions(Compiler* comp, assertedRange.ToString(comp)); Range copy = *pRange; - copy.lLimit = tightenLimit(assertedRange.lLimit, copy.lLimit, preferredBoundVN, true); - copy.uLimit = tightenLimit(assertedRange.uLimit, copy.uLimit, preferredBoundVN, false); + copy.lLimit = TightenLimit(assertedRange.lLimit, copy.lLimit, preferredBoundVN, true); + copy.uLimit = TightenLimit(assertedRange.uLimit, copy.uLimit, preferredBoundVN, false); JITDUMP("[%s]\n", copy.ToString(comp)); if (copy.IsValid()) diff --git a/src/coreclr/jit/rangecheck.h b/src/coreclr/jit/rangecheck.h index 835ec0f4eb3a66..68395b2537edb7 100644 --- a/src/coreclr/jit/rangecheck.h +++ b/src/coreclr/jit/rangecheck.h @@ -803,6 +803,8 @@ class RangeCheck // refine the "pRange" value. void MergeEdgeAssertions(GenTreeLclVarCommon* lcl, ASSERT_VALARG_TP assertions, Range* pRange); + static Limit TightenLimit(Limit l1, Limit l2, ValueNum preferredBound, bool isLower); + // Inspect the assertions about the current ValueNum to refine pRange static void MergeEdgeAssertions(Compiler* comp, ValueNum num, diff --git a/src/tests/JIT/opt/RangeChecks/ElidedBoundsChecks.cs b/src/tests/JIT/opt/RangeChecks/ElidedBoundsChecks.cs index 4f8f09347e0c77..e9438a1ba78934 100644 --- a/src/tests/JIT/opt/RangeChecks/ElidedBoundsChecks.cs +++ b/src/tests/JIT/opt/RangeChecks/ElidedBoundsChecks.cs @@ -65,6 +65,24 @@ static byte AndByLength(int i) return span[i & (span.Length - 1)]; } + [MethodImpl(MethodImplOptions.NoInlining)] + static bool IndexPlusConstLessThanLen(ReadOnlySpan span) + { + // X64-NOT: CORINFO_HELP_RNGCHKFAIL + // ARM64-NOT: CORINFO_HELP_RNGCHKFAIL + for (int i = 0; i < span.Length; i++) + { + if (span[i] == '%' && (uint)(i + 2) < (uint)span.Length) + { + if (span[i + 1] == 'F' && span[i + 2] == 'F') + { + return true; + } + } + } + return false; + } + [Fact] public static int TestEntryPoint() { @@ -92,6 +110,15 @@ public static int TestEntryPoint() if (AndByLength(255) != 4) return 0; + if (IndexPlusConstLessThanLen("%FF".AsSpan()) != true) + return 0; + + if (IndexPlusConstLessThanLen("%F".AsSpan()) != false) + return 0; + + if (IndexPlusConstLessThanLen("hello".AsSpan()) != false) + return 0; + return 100; } }