JIT: Elide transitive array bounds checks

src/coreclr/jit/assertionprop.cpp

@@ -1768,6 +1768,27 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree)
         return NO_ASSERTION_INDEX;
     }
 
+    ValueNumStore::UnsignedCompareCheckedBoundInfo unsignedCompareBnd;
+    bool isUnsignedCompareCheckedBound = vnStore->IsVNUnsignedCompareCheckedBound(relopVN, &unsignedCompareBnd);
+
+    bool arrLenIsOp1 = !isUnsignedCompareCheckedBound && vnStore->IsVNArrLen(op1VN) &&
+                       relopFuncApp.FuncIs(VNF_LT_UN, VNF_GE_UN);
+    bool arrLenIsOp2 = !isUnsignedCompareCheckedBound && vnStore->IsVNArrLen(op2VN) &&
+                       relopFuncApp.FuncIs(VNF_GT_UN, VNF_LE_UN);
+    if (arrLenIsOp1 || arrLenIsOp2)
+    {
+        if (arrLenIsOp1)
+        {
+            relopFunc = ValueNumStore::SwapRelop(relopFunc);
+            std::swap(op1VN, op2VN);
+        }
+
+        AssertionDsc   dsc = AssertionDsc::CreateCompareCheckedBound(this, relopFunc, op1VN, op2VN, 0, true);
+        AssertionIndex idx = optAddAssertion(dsc);
+        optCreateComplementaryAssertion(idx);
+        return idx;
+    }
+
     // "CheckedBnd <relop> X"
     if (!isUnsignedRelop && vnStore->IsVNCheckedBound(op1VN))
     {
@@ -1800,6 +1821,16 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree)
         }
     }
 
+    if (!isUnsignedCompareCheckedBound && isUnsignedRelop && (op1VN != op2VN) && !vnStore->IsVNConstant(op1VN) &&
+        !vnStore->IsVNConstant(op2VN) && vnStore->IsVNCheckedBoundIndex(op1VN) &&
+        optAssertionHasAssertionsForVN(op2VN))
+    {
+        AssertionDsc   dsc = AssertionDsc::CreateRelopVN(this, relopFunc, op1VN, op2VN);
+        AssertionIndex idx = optAddAssertion(dsc);
+        optCreateComplementaryAssertion(idx);
+        return idx;
+    }
+
     // The remaining "(CheckedBnd + CNS) <relop> X" cases are only useful when the
     // comparison is ordered (LT/LE/GT/GE). For equality relops we don't produce
     // CheckedBoundAddConst-shaped assertions; the consumers (RangeCheck) only
@@ -1833,8 +1864,7 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree)
 
     // Loop condition like "(uint)i < (uint)bnd" or equivalent
     // Assertion: "no throw" since this condition guarantees that i is both >= 0 and < bnd (on the appropriate edge)
-    ValueNumStore::UnsignedCompareCheckedBoundInfo unsignedCompareBnd;
-    if (vnStore->IsVNUnsignedCompareCheckedBound(relopVN, &unsignedCompareBnd))
+    if (isUnsignedCompareCheckedBound)
     {
         ValueNum idxVN = vnStore->VNNormalValue(unsignedCompareBnd.vnIdx);
         ValueNum lenVN = vnStore->VNNormalValue(unsignedCompareBnd.vnBound);

src/coreclr/jit/compiler.h

@@ -9068,7 +9068,7 @@ class Compiler
 
         // Create "i <relop> (bnd + cns)" assertion
         static AssertionDsc CreateCompareCheckedBound(
-            const Compiler* comp, VNFunc relop, ValueNum op1VN, ValueNum checkedBndVN, int cns)
+            const Compiler* comp, VNFunc relop, ValueNum op1VN, ValueNum checkedBndVN, int cns, bool isVNNeverNegative = false)
         {
             assert(op1VN != ValueNumStore::NoVN);
             assert(checkedBndVN != ValueNumStore::NoVN);
@@ -9080,7 +9080,7 @@ class Compiler
             dsc.m_op2.m_kind              = O2K_VN_ADD_CNS;
             dsc.m_op2.m_vn                = checkedBndVN;
             dsc.m_op2.m_icon.m_iconVal    = cns;
-            dsc.m_op2.m_isVNNeverNegative = comp->vnStore->IsVNNeverNegative(checkedBndVN);
+            dsc.m_op2.m_isVNNeverNegative = isVNNeverNegative || comp->vnStore->IsVNNeverNegative(checkedBndVN);
             return dsc;
         }
 

src/coreclr/jit/rangecheck.cpp

@@ -1260,6 +1260,17 @@ void RangeCheck::MergeEdgeAssertionsWorker(Compiler*                        comp
             cmpOper = Compiler::AssertionDsc::ToCompareOper(curAssertion.GetKind(), &isUnsigned);
             limit   = Limit(Limit::keConstant, maxValue);
         }
+        // Current assertion is "normalLclVN u<= preferredBoundVN".
+        else if (canUseCheckedBounds && curAssertion.KindIs(Compiler::OAK_LE_UN) &&
+                 (curAssertion.GetOp1().GetVN() == normalLclVN) &&
+                 curAssertion.GetOp2().KindIs(Compiler::O2K_VN_ADD_CNS) &&
+                 curAssertion.GetOp2().IsVNNeverNegative() &&
+                 (curAssertion.GetOp2().GetVN() == preferredBoundVN) && (curAssertion.GetOp2().GetCns() == 0))
+        {
+            cmpOper    = GT_LE;
+            limit      = Limit(Limit::keBinOpArray, preferredBoundVN, 0);
+            isUnsigned = true;
+        }
         // Current assertion is of the form "i <relop> (vn + cns)" where vn is a real
         // (length-like) checked bound. The arbitrary-VN sub-form of O2K_VN_ADD_CNS (created by
         // CreateRelopVN, where op2.vn is not a checked bound) is intentionally excluded here so
@@ -1505,6 +1516,30 @@ void RangeCheck::MergeEdgeAssertionsWorker(Compiler*                        comp
                 continue;
             }
         }
+        // Current assertion is "normalLclVN u< boundVN". Get boundVN's range using our preferred bound.
+        else if (canUseCheckedBounds && pRange->LowerLimit().IsUnknown() && pRange->UpperLimit().IsUnknown() &&
+                 curAssertion.KindIs(Compiler::OAK_LT_UN) &&
+                 (curAssertion.GetOp1().GetVN() == normalLclVN) &&
+                 curAssertion.GetOp2().KindIs(Compiler::O2K_VN_ADD_CNS) &&
+                 (curAssertion.GetOp2().GetVN() != preferredBoundVN) && (curAssertion.GetOp2().GetCns() == 0) &&
+                 (budget > 0))
+        {
+            Range boundRange = GetRangeFromType(comp->vnStore->TypeOfVN(curAssertion.GetOp2().GetVN()));
+            MergeEdgeAssertionsWorker(comp, curAssertion.GetOp2().GetVN(), preferredBoundVN, assertions, &boundRange,
+                                      canUseCheckedBounds, budget - 1, visited);
+
+            if (boundRange.LowerLimit().IsConstant() && (boundRange.LowerLimit().GetConstant() >= 0) &&
+                boundRange.UpperLimit().IsBinOpArray() && (boundRange.UpperLimit().vn == preferredBoundVN))
+            {
+                cmpOper    = GT_LT;
+                limit      = boundRange.UpperLimit();
+                isUnsigned = true;
+            }
+            else
+            {
+                continue;
+            }
+        }
         // Current assertion is of the form "X <relop> Y" where both X and Y are arbitrary VNs
         // We try to derive a bound on normalLclVN by recursively computing the range of the other operand.
         //

src/coreclr/jit/valuenum.cpp

@@ -423,6 +423,7 @@ ValueNumStore::ValueNumStore(Compiler* comp, CompAllocator alloc)
     , m_nextChunkBase(0)
     , m_fixedPointMapSels(alloc, 8)
     , m_checkedBoundVNs(alloc)
+    , m_checkedBoundIndexVNs(alloc)
     , m_chunks(alloc, 8)
     , m_intCnsMap(nullptr)
     , m_longCnsMap(nullptr)
@@ -7785,6 +7786,12 @@ bool ValueNumStore::IsVNCheckedBound(ValueNum vn)
     return false;
 }
 
+bool ValueNumStore::IsVNCheckedBoundIndex(ValueNum vn)
+{
+    bool dummy;
+    return m_checkedBoundIndexVNs.TryGetValue(vn, &dummy);
+}
+
 //----------------------------------------------------------------------------------
 // IsVNCastToULong: checks whether the given VN represents (ulong)op cast
 //
@@ -7812,14 +7819,14 @@ bool ValueNumStore::IsVNCastToULong(ValueNum vn, ValueNum* castedOp)
     return false;
 }
 
-void ValueNumStore::SetVNIsCheckedBound(ValueNum vn)
+void ValueNumStore::SetVNIsCheckedBound(ValueNum vn, bool isIndex)
 {
-    // This is meant to flag VNs for lengths that aren't known at compile time, so we can
+    // This is meant to flag VNs for bounds check operands that aren't known at compile time, so we can
     // form and propagate assertions about them.  Ensure that callers filter out constant
     // VNs since they're not what we're looking to flag, and assertion prop can reason
     // directly about constants.
     assert(!IsVNConstant(vn));
-    m_checkedBoundVNs.AddOrUpdate(vn, true);
+    (isIndex ? m_checkedBoundIndexVNs : m_checkedBoundVNs).AddOrUpdate(vn, true);
 }
 
 #ifdef FEATURE_HW_INTRINSICS
@@ -13446,8 +13453,15 @@ void Compiler::fgValueNumberTree(GenTree* tree)
                         // next add the bounds check exception set for the current tree node
                         fgValueNumberAddExceptionSet(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.
+                        // Record non-constant value numbers that are used as arguments to bounds checks, so that
+                        // assertion prop will know that comparisons against them are worth analyzing.
+                        ValueNum indexVN =
+                            vnStore->VNNormalValue(tree->AsBoundsChk()->GetIndex()->gtVNPair.GetConservative());
+                        if ((indexVN != ValueNumStore::NoVN) && !vnStore->IsVNConstant(indexVN))
+                        {
+                            vnStore->SetVNIsCheckedBound(indexVN, true);
+                        }
+
                         ValueNum lengthVN =
                             vnStore->VNNormalValue(tree->AsBoundsChk()->GetArrayLength()->gtVNPair.GetConservative());
                         if ((lengthVN != ValueNumStore::NoVN) && !vnStore->IsVNConstant(lengthVN))

src/coreclr/jit/valuenum.h

@@ -1100,12 +1100,15 @@ class ValueNumStore
     // of the length argument to a GT_BOUNDS_CHECK node.
     bool IsVNCheckedBound(ValueNum vn);
 
+    // Returns true if the VN appears as the index argument to a GT_BOUNDS_CHECK node.
+    bool IsVNCheckedBoundIndex(ValueNum vn);
+
     // Returns true if the VN is known to be a cast to ulong
     bool IsVNCastToULong(ValueNum vn, ValueNum* castedOp);
 
-    // Record that a VN is known to appear as the conservative value number of the length
-    // argument to a GT_BOUNDS_CHECK node.
-    void SetVNIsCheckedBound(ValueNum vn);
+    // Record that a VN is known to appear as the conservative value number of an argument
+    // to a GT_BOUNDS_CHECK node.
+    void SetVNIsCheckedBound(ValueNum vn, bool isIndex = false);
 
     // Information about the individual components of a value number representing an unsigned
     // comparison of some value against a checked bound VN.
@@ -1658,8 +1661,9 @@ class ValueNumStore
     // Returns true if "sel(map, ind)" is a member of "m_fixedPointMapSels".
     bool SelectIsBeingEvaluatedRecursively(ValueNum map, ValueNum ind);
 
-    // This is the set of value numbers that have been flagged as arguments to bounds checks, in the length position.
+    // These are the value numbers flagged as length and index arguments to bounds checks.
     CheckedBoundVNSet m_checkedBoundVNs;
+    CheckedBoundVNSet m_checkedBoundIndexVNs;
 
     // This is a map from "chunk number" to the attributes of the chunk.
     JitExpandArrayStack<Chunk*> m_chunks;