Eliminate bound checks for "% arr.Length" when possible

src/coreclr/jit/assertionprop.cpp

@@ -5653,11 +5653,46 @@ GenTree* Compiler::optAssertionProp_BndsChk(ASSERT_VALARG_TP assertions,
             }
         }
     }
-    else if (vnStore->IsVNBinFunc(vnCurIdx, VNF_UMOD, &idxOp0, &idxOp1) && (idxOp1 == vnCurLen))
+    else if (vnStore->IsVNBinFunc(vnCurIdx, VNF_UMOD, &idxOp0, &idxOp1))
     {
-        // If arr.Length is 0 we technically should keep the bounds check, but since the expression
-        // has to throw DivideByZeroException anyway - no special handling needed.
-        return dropBoundsCheck(INDEBUG("a[X u% a.Length] is always within bounds"));
+        if (idxOp0 == vnCurLen)
+        {
+            Range lenRng = GetRange(this, arrBndsChkLen, block, assertions, /*fast*/ true);
+
+            if (lenRng.LowerLimit().IsConstant() && lenRng.LowerLimit().GetConstant() > 0)
+            {
+                return dropBoundsCheck(
+                    INDEBUG("a[a.Length u% X] is always within bounds when a.Length is known to be > 0"));
+            }
+        }
+        else if (idxOp1 == vnCurLen)
+        {
+            // If arr.Length is 0 we technically should keep the bounds check, but since the expression
+            // has to throw DivideByZeroException anyway - no special handling needed.
+            return dropBoundsCheck(INDEBUG("a[X u% a.Length] is always within bounds"));
+        }
+    }
+    else if (vnStore->IsVNBinFunc(vnCurIdx, VNF_MOD, &idxOp0, &idxOp1))
+    {
+        if (idxOp0 == vnCurLen)
+        {
+            Range lenRng = GetRange(this, arrBndsChkLen, block, assertions, /*fast*/ true);
+
+            if (lenRng.LowerLimit().IsConstant() && lenRng.LowerLimit().GetConstant() > 0)
+            {
+                return dropBoundsCheck(
+                    INDEBUG("a[a.Length % X] is always within bounds when a.Length is known to be > 0"));
+            }
+        }
+        else if (idxOp1 == vnCurLen)
+        {
+            Range idxRng = GetRange(this, arrBndsChkIdx, block, assertions, /*fast*/ true);
+
+            if (idxRng.LowerLimit().IsConstant() && idxRng.LowerLimit().GetConstant() >= 0)
+            {
+                return dropBoundsCheck(INDEBUG("a[X % a.Length] is always within bounds when X is known to be >= 0"));
+            }
+        }
     }
 
     // Let's see if we can remove the bounds check based on the ranges.

src/coreclr/jit/rangecheck.cpp

@@ -711,6 +711,7 @@ Range RangeCheck::GetRangeFromAssertionsWorker(
             case VNF_RSH:
             case VNF_RSZ:
             case VNF_UMOD:
+            case VNF_MOD:
             case VNF_UDIV:
             {
                 // Get ranges of both operands and perform the same operation on the ranges.
@@ -793,6 +794,9 @@ Range RangeCheck::GetRangeFromAssertionsWorker(
                     case VNF_UMOD:
                         binOpResult = RangeOps::UnsignedMod(r1, r2);
                         break;
+                    case VNF_MOD:
+                        binOpResult = RangeOps::Mod(r1, r2);
+                        break;
                     case VNF_UDIV:
                         binOpResult = RangeOps::UnsignedDivide(r1, r2);
                         break;

src/coreclr/jit/rangecheck.h

@@ -521,6 +521,70 @@ struct RangeOps
         return Range(Limit(Limit::keUnknown));
     }
 
+    static Range Mod(const Range& r1, const Range& r2)
+    {
+        if (r1.LowerLimit().IsConstant())
+        {
+            int r2ConstVal;
+
+            if (r2.IsSingleValueConstant(&r2ConstVal))
+            {
+                if (r2ConstVal == 0)
+                {
+                    return Range(Limit(Limit::keUnknown));
+                }
+
+                int r1ConstVal;
+
+                if (r1.IsSingleValueConstant(&r1ConstVal))
+                {
+                    return Range(Limit(Limit::keConstant, r1ConstVal % r2ConstVal));
+                }
+            }
+
+            const auto SafeAbsMinusOne = [](const int value) {
+                return value == INT32_MIN ? INT32_MAX : (abs(value) - 1);
+            };
+
+            const int r1lo = r1.LowerLimit().GetConstant();
+
+            if (r1lo >= 0)
+            {
+                if (r2.IsConstantRange())
+                {
+                    const int r2lo        = r2.LowerLimit().GetConstant();
+                    const int r2hi        = r2.UpperLimit().GetConstant();
+                    // x % [-4..2] -> [0..3]
+                    const int maxAbsRight = max(SafeAbsMinusOne(r2lo), SafeAbsMinusOne(r2hi));
+                    const int rightLimit =
+                        r1.UpperLimit().IsConstant() ? min(r1.UpperLimit().GetConstant(), maxAbsRight) : maxAbsRight;
+
+                    return Range(Limit(Limit::keConstant, 0), Limit(Limit::keConstant, rightLimit));
+                }
+
+                return Range(Limit(Limit::keConstant, 0), Limit(Limit::keUnknown));
+            }
+            else
+            {
+                if (r2.IsConstantRange())
+                {
+                    const int r2lo        = r2.LowerLimit().GetConstant();
+                    const int r2hi        = r2.UpperLimit().GetConstant();
+                    // x % [-4..2] -> [0..3]
+                    const int maxAbsRight = max(SafeAbsMinusOne(r2lo), SafeAbsMinusOne(r2hi));
+
+                    const int leftLimit = max(r1lo, -maxAbsRight);
+
+                    return Range(Limit(Limit::keConstant, leftLimit), Limit(Limit::keConstant, -0));
+                }
+
+                return Range(Limit(Limit::keUnknown), Limit(Limit::keConstant, -0));
+            }
+        }
+
+        return Range(Limit(Limit::keUnknown));
+    }
+
     static Range UnsignedDivide(const Range& r1, const Range& r2)
     {
         // We only handle constant ranges for both operands.

src/tests/JIT/opt/RangeChecks/ModLength.cs

@@ -36,6 +36,14 @@ public static void TestEntryPoint()
         Test6(new int[10], 0);
         Test7(new int[10], 0);
         Throws<DivideByZeroException>(() => Test8(new int[10], 0));
+        Test10(new int[10], 100);
+        Test11(new int[10], 0);
+        Test11(new int[10], 100);
+        Throws<DivideByZeroException>(() => Test11(new int[0], 10));
+        Throws<DivideByZeroException>(() => Test12(new int[0], 10));
+        Test12(new int[10], 0);
+        Test12(new int[10], 100);
+        Throws<DivideByZeroException>(() => Test12(new int[10], 0));
     }
 
     static void Throws<T>(Action action, [CallerLineNumber] int line = 0)
@@ -80,7 +88,7 @@ static int Test6(int[] arr, int index)
         var span = arr.AsSpan();
         return span[(int)index % (int)span.Length];
     }
-
+    
     [MethodImpl(MethodImplOptions.NoInlining)]
     static int Test7(int[] arr, int index)
     {
@@ -97,4 +105,37 @@ static int Test8(int[] arr, int index)
 
     [MethodImpl(MethodImplOptions.NoInlining)]
     static int Test9(int[] arr, int index) => arr[index / arr.Length];
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    static int Test10(int[] arr, uint index)
+    {
+        if (arr.Length > 0)
+        {
+            return arr[arr.Length % index];
+        }
+
+        return 1234;
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    static int Test11(int[] arr, int index)
+    {
+        if (index >= 0)
+        {
+            return arr[index % arr.Length];
+        }
+
+        return 1234;
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    static int Test12(int[] arr, int index)
+    {
+        if (arr.Length > 0)
+        {
+            return arr[arr.Length % index];
+        }
+
+        return 1234;
+    }
 }