Jit: Remove bounds checks with tests against length.

src/coreclr/src/jit/assertionprop.cpp

@@ -1625,7 +1625,6 @@ void Compiler::optDebugCheckAssertion(AssertionDsc* assertion)
                     assert(assertion->op2.u1.iconFlags != 0);
                     break;
                 case O1K_LCLVAR:
-                case O1K_ARR_BND:
                     assert((lvaTable[assertion->op1.lcl.lclNum].lvType != TYP_REF) || (assertion->op2.u1.iconVal == 0));
                     break;
                 case O1K_VALUE_NUMBER:
@@ -1959,12 +1958,48 @@ AssertionInfo Compiler::optAssertionGenJtrue(GenTree* tree)
     {
         std::swap(op1, op2);
     }
+
+    ValueNum op1VN = vnStore->VNConservativeNormalValue(op1->gtVNPair);
+    ValueNum op2VN = vnStore->VNConservativeNormalValue(op2->gtVNPair);
     // If op1 is lcl and op2 is const or lcl, create assertion.
     if ((op1->gtOper == GT_LCL_VAR) &&
         ((op2->OperKind() & GTK_CONST) || (op2->gtOper == GT_LCL_VAR))) // Fix for Dev10 851483
     {
         return optCreateJtrueAssertions(op1, op2, assertionKind);
     }
+    else if (vnStore->IsVNCheckedBound(op1VN) && vnStore->IsVNInt32Constant(op2VN))
+    {
+        int con = vnStore->ConstantValue<int>(op2VN);
+        if (con >= 0)
+        {
+            AssertionDsc dsc;
+            if (con == 0)
+            {
+                dsc.assertionKind = OAK_NOT_EQUAL;
+                dsc.op1.bnd.vnIdx = vnStore->VNForIntCon(0);
+            }
+            else
+            {
+                dsc.assertionKind = OAK_EQUAL;
+                dsc.op1.bnd.vnIdx = vnStore->VNForIntCon(con - 1);
+            }
+
+            dsc.op1.vn           = op1VN;
+            dsc.op1.kind         = O1K_ARR_BND;
+            dsc.op1.bnd.vnLen    = op1VN;
+            dsc.op2.vn           = vnStore->VNConservativeNormalValue(op2->gtVNPair);
+            dsc.op2.kind         = O2K_CONST_INT;
+            dsc.op2.u1.iconFlags = 0;
+            dsc.op2.u1.iconVal   = 0;
+
+            AssertionIndex index = optAddAssertion(&dsc);
+            if (relop->OperIs(GT_NE) != (con == 0))
+            {
+                return AssertionInfo::ForNextEdge(index);
+            }
+            return index;
+        }
+    }
 
     // Check op1 and op2 for an indirection of a GT_LCL_VAR and keep it in op1.
     if (((op1->gtOper != GT_IND) || (op1->AsOp()->gtOp1->gtOper != GT_LCL_VAR)) &&

src/coreclr/src/jit/compiler.h

@@ -6786,6 +6786,11 @@ class Compiler
             return ((assertionKind == OAK_EQUAL) && (op1.kind == O1K_LCLVAR) && (op2.kind == O2K_LCLVAR_COPY));
         }
 
+        bool IsConstantInt32Assertion()
+        {
+            return ((assertionKind == OAK_EQUAL) || (assertionKind == OAK_NOT_EQUAL)) && (op2.kind == O2K_CONST_INT);
+        }
+
         static bool SameKind(AssertionDsc* a1, AssertionDsc* a2)
         {
             return a1->assertionKind == a2->assertionKind && a1->op1.kind == a2->op1.kind &&

src/coreclr/src/jit/rangecheck.cpp

@@ -60,7 +60,7 @@ int RangeCheck::GetArrLength(ValueNum vn)
 }
 
 // Check if the computed range is within bounds.
-bool RangeCheck::BetweenBounds(Range& range, int lower, GenTree* upper)
+bool RangeCheck::BetweenBounds(Range& range, int lower, GenTree* upper, int arrSize)
 {
 #ifdef DEBUG
     if (m_pCompiler->verbose)
@@ -85,26 +85,7 @@ bool RangeCheck::BetweenBounds(Range& range, int lower, GenTree* upper)
     JITDUMP("\n");
 #endif
 
-    int arrSize = 0;
-
-    if (vnStore->IsVNConstant(uLimitVN))
-    {
-        ssize_t  constVal  = -1;
-        unsigned iconFlags = 0;
-
-        if (m_pCompiler->optIsTreeKnownIntValue(true, upper, &constVal, &iconFlags))
-        {
-            arrSize = (int)constVal;
-        }
-    }
-    else if (vnStore->IsVNArrLen(uLimitVN))
-    {
-        // Get the array reference from the length.
-        ValueNum arrRefVN = vnStore->GetArrForLenVn(uLimitVN);
-        // Check if array size can be obtained.
-        arrSize = vnStore->GetNewArrSize(arrRefVN);
-    }
-    else if (!vnStore->IsVNCheckedBound(uLimitVN))
+    if ((arrSize <= 0) && !vnStore->IsVNCheckedBound(uLimitVN))
     {
         // If the upper limit is not length, then bail.
         return false;
@@ -231,6 +212,17 @@ void RangeCheck::OptimizeRangeCheck(BasicBlock* block, Statement* stmt, GenTree*
 #endif // FEATURE_SIMD
     {
         arrSize = GetArrLength(arrLenVn);
+        if (arrSize <= 0)
+        {
+            // see if there are any assertions about the array size we can use
+            JITDUMP("Looking for array size assertions for: " FMT_VN "\n", arrLenVn);
+            Range arrLength = Range(Limit(Limit::keDependent));
+            MergeEdgeAssertions(arrLenVn, block->bbAssertionIn, &arrLength);
+            if (arrLength.lLimit.IsConstant())
+            {
+                arrSize = arrLength.lLimit.GetConstant();
+            }
+        }
     }
 
     JITDUMP("ArrSize for lengthVN:%03X = %d\n", arrLenVn, arrSize);
@@ -286,7 +278,7 @@ void RangeCheck::OptimizeRangeCheck(BasicBlock* block, Statement* stmt, GenTree*
     }
 
     // Is the range between the lower and upper bound values.
-    if (BetweenBounds(range, 0, bndsChk->gtArrLen))
+    if (BetweenBounds(range, 0, bndsChk->gtArrLen, arrSize))
     {
         JITDUMP("[RangeCheck::OptimizeRangeCheck] Between bounds\n");
         m_pCompiler->optRemoveRangeCheck(treeParent, stmt);
@@ -532,18 +524,36 @@ void RangeCheck::SetDef(UINT64 hash, Location* loc)
 }
 #endif
 
-// Merge assertions on the edge flowing into the block about a variable.
 void RangeCheck::MergeEdgeAssertions(GenTreeLclVarCommon* lcl, ASSERT_VALARG_TP assertions, Range* pRange)
+{
+    if (lcl->GetSsaNum() == SsaConfig::RESERVED_SSA_NUM)
+    {
+        return;
+    }
+
+    LclVarDsc* varDsc = m_pCompiler->lvaGetDesc(lcl);
+    if (varDsc->CanBeReplacedWithItsField(m_pCompiler))
+    {
+        varDsc = m_pCompiler->lvaGetDesc(varDsc->lvFieldLclStart);
+    }
+    LclSsaVarDsc* ssaData     = varDsc->GetPerSsaData(lcl->GetSsaNum());
+    ValueNum      normalLclVN = m_pCompiler->vnStore->VNConservativeNormalValue(ssaData->m_vnPair);
+    MergeEdgeAssertions(normalLclVN, assertions, pRange);
+}
+
+// Merge assertions on the edge flowing into the block about a variable.
+void RangeCheck::MergeEdgeAssertions(ValueNum normalLclVN, ASSERT_VALARG_TP assertions, Range* pRange)
 {
     if (BitVecOps::IsEmpty(m_pCompiler->apTraits, assertions))
     {
         return;
     }
 
-    if (lcl->GetSsaNum() == SsaConfig::RESERVED_SSA_NUM)
+    if (normalLclVN == ValueNumStore::NoVN)
     {
         return;
     }
+
     // Walk through the "assertions" to check if the apply.
     BitVecOps::Iter iter(m_pCompiler->apTraits, assertions);
     unsigned        index = 0;
@@ -554,15 +564,8 @@ void RangeCheck::MergeEdgeAssertions(GenTreeLclVarCommon* lcl, ASSERT_VALARG_TP
         Compiler::AssertionDsc* curAssertion = m_pCompiler->optGetAssertion(assertionIndex);
 
         Limit      limit(Limit::keUndef);
-        genTreeOps cmpOper = GT_NONE;
-
-        LclVarDsc* varDsc = m_pCompiler->lvaGetDesc(lcl);
-        if (varDsc->CanBeReplacedWithItsField(m_pCompiler))
-        {
-            varDsc = m_pCompiler->lvaGetDesc(varDsc->lvFieldLclStart);
-        }
-        LclSsaVarDsc* ssaData     = varDsc->GetPerSsaData(lcl->GetSsaNum());
-        ValueNum      normalLclVN = m_pCompiler->vnStore->VNConservativeNormalValue(ssaData->m_vnPair);
+        genTreeOps cmpOper             = GT_NONE;
+        bool       isConstantAssertion = false;
 
         // Current assertion is of the form (i < len - cns) != 0
         if (curAssertion->IsCheckedBoundArithBound())
@@ -602,13 +605,20 @@ void RangeCheck::MergeEdgeAssertions(GenTreeLclVarCommon* lcl, ASSERT_VALARG_TP
             m_pCompiler->vnStore->GetCompareCheckedBound(curAssertion->op1.vn, &info);
 
             // If we don't have the same variable we are comparing against, bail.
-            if (normalLclVN != info.cmpOp)
+            if (normalLclVN == info.cmpOp)
+            {
+                cmpOper = (genTreeOps)info.cmpOper;
+                limit   = Limit(Limit::keBinOpArray, info.vnBound, 0);
+            }
+            else if (normalLclVN == info.vnBound)
+            {
+                cmpOper = GenTree::SwapRelop((genTreeOps)info.cmpOper);
+                limit   = Limit(Limit::keBinOpArray, info.cmpOp, 0);
+            }
+            else
             {
                 continue;
             }
-
-            limit   = Limit(Limit::keBinOpArray, info.vnBound, 0);
-            cmpOper = (genTreeOps)info.cmpOper;
         }
         // Current assertion is of the form (i < 100) != 0
         else if (curAssertion->IsConstantBound())
@@ -627,6 +637,31 @@ void RangeCheck::MergeEdgeAssertions(GenTreeLclVarCommon* lcl, ASSERT_VALARG_TP
             limit   = Limit(Limit::keConstant, info.constVal);
             cmpOper = (genTreeOps)info.cmpOper;
         }
+        // Current assertion is of the form i == 100
+        else if (curAssertion->IsConstantInt32Assertion())
+        {
+            if (curAssertion->op1.vn != normalLclVN)
+            {
+                continue;
+            }
+
+            int cnstLimit = m_pCompiler->vnStore->ConstantValue<int>(curAssertion->op2.vn);
+
+            if ((cnstLimit == 0) && (curAssertion->assertionKind == Compiler::OAK_NOT_EQUAL) &&
+                m_pCompiler->vnStore->IsVNCheckedBound(curAssertion->op1.vn))
+            {
+                // we have arr.Len != 0, so the length must be atleast one
+                limit   = Limit(Limit::keConstant, 1);
+                cmpOper = GT_GE;
+            }
+            else
+            {
+                limit   = Limit(Limit::keConstant, cnstLimit);
+                cmpOper = GT_EQ;
+            }
+
+            isConstantAssertion = true;
+        }
         // Current assertion is not supported, ignore it
         else
         {
@@ -636,7 +671,7 @@ void RangeCheck::MergeEdgeAssertions(GenTreeLclVarCommon* lcl, ASSERT_VALARG_TP
         assert(limit.IsBinOpArray() || limit.IsConstant());
 
         // Make sure the assertion is of the form != 0 or == 0.
-        if (curAssertion->op2.vn != m_pCompiler->vnStore->VNZeroForType(TYP_INT))
+        if ((curAssertion->op2.vn != m_pCompiler->vnStore->VNZeroForType(TYP_INT)) && !isConstantAssertion)
         {
             continue;
         }
@@ -647,6 +682,17 @@ void RangeCheck::MergeEdgeAssertions(GenTreeLclVarCommon* lcl, ASSERT_VALARG_TP
         }
 #endif
 
+        // Limits are sometimes made with the form vn + constant, where vn is a known constant
+        // see if we can simplify this to just a constant
+        if (limit.IsBinOpArray() && m_pCompiler->vnStore->IsVNInt32Constant(limit.vn))
+        {
+            Limit tempLimit = Limit(Limit::keConstant, m_pCompiler->vnStore->ConstantValue<int>(limit.vn));
+            if (tempLimit.AddConstant(limit.cns))
+            {
+                limit = tempLimit;
+            }
+        }
+
         ValueNum arrLenVN = m_pCompiler->vnStore->VNConservativeNormalValue(m_pCurBndsChk->gtArrLen->gtVNPair);
 
         if (m_pCompiler->vnStore->IsVNConstant(arrLenVN))
@@ -663,6 +709,7 @@ void RangeCheck::MergeEdgeAssertions(GenTreeLclVarCommon* lcl, ASSERT_VALARG_TP
         //      (i < length) != 0
         //      (i < 100) == 0
         //      (i < 100) != 0
+        //      i == 100
         //
         // At this point, we have detected that op1.vn is (i < length) or (i < length + cns) or
         // (i < 100) and the op2.vn is 0.
@@ -673,12 +720,12 @@ void RangeCheck::MergeEdgeAssertions(GenTreeLclVarCommon* lcl, ASSERT_VALARG_TP
         // If we have an assertion of the form == 0 (i.e., equals false), then reverse relop.
         // The relop has to be reversed because we have: (i < length) is false which is the same
         // as (i >= length).
-        if (curAssertion->assertionKind == Compiler::OAK_EQUAL)
+        if ((curAssertion->assertionKind == Compiler::OAK_EQUAL) && !isConstantAssertion)
         {
             cmpOper = GenTree::ReverseRelop(cmpOper);
         }
 
-        // Bounds are inclusive, so add -1 for upper bound when "<". But make sure we won't overflow.
+        // Bounds are inclusive, so add -1 for upper bound when "<". But make sure we won't underflow.
         if (cmpOper == GT_LT && !limit.AddConstant(-1))
         {
             continue;
@@ -744,6 +791,10 @@ void RangeCheck::MergeEdgeAssertions(GenTreeLclVarCommon* lcl, ASSERT_VALARG_TP
                 pRange->lLimit = limit;
                 break;
 
+            case GT_EQ:
+                pRange->uLimit = limit;
+                pRange->lLimit = limit;
+
             default:
                 // All other 'cmpOper' kinds leave lLimit/uLimit unchanged
                 break;

src/coreclr/src/jit/rangecheck.h

@@ -438,7 +438,7 @@ class RangeCheck
     // assumes that the lower range is resolved and upper range is symbolic as in an
     // increasing loop.
     // TODO-CQ: This is not general enough.
-    bool BetweenBounds(Range& range, int lower, GenTree* upper);
+    bool BetweenBounds(Range& range, int lower, GenTree* upper, int arrSize);
 
     // Entry point to optimize range checks in the block. Assumes value numbering
     // and assertion prop phases are completed.
@@ -474,6 +474,9 @@ class RangeCheck
     // refine the "pRange" value.
     void MergeEdgeAssertions(GenTreeLclVarCommon* lcl, ASSERT_VALARG_TP assertions, Range* pRange);
 
+    // Inspect the assertions about the current ValueNum to refine pRange
+    void MergeEdgeAssertions(ValueNum num, ASSERT_VALARG_TP assertions, Range* pRange);
+
     // The maximum possible value of the given "limit." If such a value could not be determined
     // return "false." For example: ARRLEN_MAX for array length.
     bool GetLimitMax(Limit& limit, int* pMax);

src/libraries/System.Private.CoreLib/src/System/String.cs

@@ -487,13 +487,9 @@ public char[] ToCharArray(int startIndex, int length)
         [NonVersionable]
         public static bool IsNullOrEmpty([NotNullWhen(false)] string? value)
         {
-            // Using 0u >= (uint)value.Length rather than
-            // value.Length == 0 as it will elide the bounds check to
-            // the first char: value[0] if that is performed following the test
-            // for the same test cost.
             // Ternary operator returning true/false prevents redundant asm generation:
             // https://github.com/dotnet/runtime/issues/4207
-            return (value == null || 0u >= (uint)value.Length) ? true : false;
+            return (value == null || 0 == value.Length) ? true : false;
         }
 
         public static bool IsNullOrWhiteSpace([NotNullWhen(false)] string? value)