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

src/coreclr/jit/assertionprop.cpp

@@ -5617,11 +5617,118 @@ 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)
+            {
+                BitVecOps::Iter iter(apTraits, assertions);
+                unsigned        index = 0;
+                while (iter.NextElem(&index))
+                {
+                    AssertionIndex assertionIndex = GetAssertionIndex(index);
+                    const Compiler::AssertionDsc& curAssertion = optGetAssertion(assertionIndex);
+
+                    if (curAssertion.GetOp1().GetVN() == vnCurLen && curAssertion.GetOp2().GetVN() == idxOp1)
+                    {
+                        if (curAssertion.KindIs(OAK_GT))
+                        {
+                            // when length > index, length % index has the range [0..length - 1]
+                            return dropBoundsCheck(INDEBUG(
+                                "a[a.Length u% X] is always within bounds when a.Length is known to be > 0 and a.Length > X"));
+                        }
+                        else if (curAssertion.KindIs(OAK_GE))
+                        {
+                            // when length >= index, length % index has the range [0..length - 1]
+                            return dropBoundsCheck(INDEBUG(
+                                "a[a.Length u% X] is always within bounds when a.Length is known to be > 0 and a.Length >= X"));
+                        }
+                    }
+                    else if (curAssertion.GetOp1().GetVN() == idxOp1 && curAssertion.GetOp2().GetVN() == vnCurLen)
+                    {
+                        if (curAssertion.KindIs(OAK_LT))
+                        {
+                            // when index < length, length % index has the range [0..length - 1]
+                            return dropBoundsCheck(INDEBUG(
+                                "a[a.Length u% X] is always within bounds when a.Length is known to be > 0 and X < a.Length"));
+                        }
+                        else if (curAssertion.KindIs(OAK_LE))
+                        {
+                            // when index <= length, length % index has the range [0..length - 1]
+                            return dropBoundsCheck(INDEBUG(
+                                "a[a.Length u% X] is always within bounds when a.Length is known to be > 0 and X <= a.Length"));
+                        }
+                    }
+                }
+            }
+        }
+        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)
+            {
+                BitVecOps::Iter iter(apTraits, assertions);
+                unsigned        index = 0;
+                while (iter.NextElem(&index))
+                {
+                    AssertionIndex assertionIndex = GetAssertionIndex(index);
+                    const Compiler::AssertionDsc& curAssertion = optGetAssertion(assertionIndex);
+
+                    if (curAssertion.GetOp1().GetVN() == vnCurLen && curAssertion.GetOp2().GetVN() == idxOp1)
+                    {
+                        if (curAssertion.KindIs(OAK_GT))
+                        {
+                            // when length > index, length % index has the range [0..length - 1]
+                            return dropBoundsCheck(
+                                INDEBUG("a[a.Length % X] is always within bounds when a.Length is known to be > 0 and a.Length > X"));
+                        }
+                        else if (curAssertion.KindIs(OAK_GE))
+                        {
+                            // when length >= index, length % index has the range [0..length - 1]
+                            return dropBoundsCheck(
+                                INDEBUG("a[a.Length % X] is always within bounds when a.Length is known to be > 0 and a.Length >= X"));
+                        }
+                    }
+                    else if (curAssertion.GetOp1().GetVN() == idxOp1 && curAssertion.GetOp2().GetVN() == vnCurLen)
+                    {
+                        if (curAssertion.KindIs(OAK_LT))
+                        {
+                            // when index < length, length % index has the range [0..length - 1]
+                            return dropBoundsCheck(INDEBUG(
+                                "a[a.Length % X] is always within bounds when a.Length is known to be > 0 and X < a.Length"));
+                        }
+                        else if (curAssertion.KindIs(OAK_LE))
+                        {
+                            // when index <= length, length % index has the range [0..length - 1]
+                            return dropBoundsCheck(INDEBUG(
+                                "a[a.Length % X] is always within bounds when a.Length is known to be > 0 and X <= a.Length"));
+                        }
+                    }
+                }
+            }
+        }
+        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,62 @@ struct RangeOps
         return Range(Limit(Limit::keUnknown));
     }
 
+    static Range Mod(const Range& r1, const Range& r2)
+    {
+        if (r1.LowerLimit().IsConstant())
+        {
+            int r1ConstVal;
+            if (r1.IsSingleValueConstant(&r1ConstVal) && (r1ConstVal == 0))
+            {
+                return Range(Limit(Limit::keConstant, 0));
+            }
+
+            int r2ConstVal;
+            if (r2.IsSingleValueConstant(&r2ConstVal))
+            {
+                if (r2ConstVal == 0)
+                {
+                    return Range(Limit(Limit::keUnknown));
+                }
+
+                if (r1.IsSingleValueConstant())
+                {
+                    return Range(Limit(Limit::keConstant, r1ConstVal % r2ConstVal));
+                }
+            }
+
+            const int r1lo = r1.LowerLimit().GetConstant();
+
+            const bool isAlwaysPositive = r1lo >= 0;
+            const bool isAlwaysNegative = r1.UpperLimit().IsConstant() && r1.UpperLimit().GetConstant() <= 0;
+
+            if (r2.IsConstantRange())
+            {
+                const auto SafeAbsMinusOne = [](const int value) {
+                    return value == INT32_MIN ? INT32_MAX : (abs(value) - 1);
+                };
+
+                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  = isAlwaysPositive ? 0 : max(r1lo, -maxAbsRight);
+                const int rightLimit = isAlwaysNegative ? -0
+                                        : r1.UpperLimit().IsConstant()
+                                            ? min(r1.UpperLimit().GetConstant(), maxAbsRight)
+                                            : maxAbsRight;
+
+                return Range(Limit(Limit::keConstant, leftLimit), Limit(Limit::keConstant, rightLimit));
+            }
+
+            return Range(isAlwaysPositive ? Limit(Limit::keConstant, 0) : Limit(Limit::keUnknown),
+                isAlwaysNegative ? Limit(Limit::keConstant, -0) : Limit(Limit::keUnknown));
+        }
+
+        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

@@ -7,7 +7,7 @@
 
 public class ModLength
 {
-    [Fact]
+        [Fact]
     public static void TestEntryPoint()
     {
         Throws<DivideByZeroException>(() => Test1(new int[0], 0));
@@ -36,6 +36,38 @@ 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], 9U);
+        Throws<DivideByZeroException>(() => Test10(new int[10], 0U));
+
+        Test11(new int[10], 0);
+        Test11(new int[10], 100);
+        Throws<DivideByZeroException>(() => Test11(new int[0], 10));
+
+        Test12(new int[10], 9);
+        Throws<DivideByZeroException>(() => Test12(new int[10], 0));
+
+        Test13(new int[10], 9U);
+        Test13(new int[10], 10U);
+        Throws<DivideByZeroException>(() => Test13(new int[10], 0U));
+
+        Test14(new int[10], 9);
+        Test14(new int[10], 10);
+        Throws<DivideByZeroException>(() => Test14(new int[10], 0));
+
+        Test15(new int[10], 9U);
+        Throws<DivideByZeroException>(() => Test15(new int[10], 0U));
+
+        Test16(new int[10], 9);
+        Throws<DivideByZeroException>(() => Test16(new int[10], 0));
+
+        Test17(new int[10], 9U);
+        Test17(new int[10], 10U);
+        Throws<DivideByZeroException>(() => Test17(new int[10], 0U));
+
+        Test18(new int[10], 9);
+        Test18(new int[10], 10);
+        Throws<DivideByZeroException>(() => Test18(new int[10], 0));
     }
 
     static void Throws<T>(Action action, [CallerLineNumber] int line = 0)
@@ -97,4 +129,103 @@ 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 && arr.Length > index)
+        {
+            return arr[(uint)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 && arr.Length > index)
+        {
+            return arr[arr.Length % index];
+        }
+
+        return 1234;
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    static int Test13(int[] arr, uint index)
+    {
+        if (arr.Length > 0 && arr.Length >= index)
+        {
+            return arr[(uint)arr.Length % index];
+        }
+
+        return 1234;
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    static int Test14(int[] arr, int index)
+    {
+        if (arr.Length > 0 && arr.Length >= index)
+        {
+            return arr[arr.Length % index];
+        }
+
+        return 1234;
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    static int Test15(int[] arr, uint index)
+    {
+        if (arr.Length > 0 && index < arr.Length)
+        {
+            return arr[(uint)arr.Length % index];
+        }
+
+        return 1234;
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    static int Test16(int[] arr, int index)
+    {
+        if (arr.Length > 0 && index < arr.Length)
+        {
+            return arr[arr.Length % index];
+        }
+
+        return 1234;
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    static int Test17(int[] arr, uint index)
+    {
+        if (arr.Length > 0 && index <= arr.Length)
+        {
+            return arr[(uint)arr.Length % index];
+        }
+
+        return 1234;
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    static int Test18(int[] arr, int index)
+    {
+        if (arr.Length > 0 && index <= arr.Length)
+        {
+            return arr[arr.Length % index];
+        }
+
+        return 1234;
+    }
 }

src/tests/run.py

@@ -1358,7 +1358,7 @@ def parse_test_results_xml_file(args, item, item_name, tests, assemblies):
                 "failed": 0,
                 "skipped": 0,
             })
-        assembly_info["time"] += float(assembly.attrib["time"])
+        assembly_info["time"] += float(assembly.attrib["time"].replace(",", "."))
 
         for collection in assembly:
             # In the non-merged tests model, we expect to see a `<errors />` tag within `<assembly>`.
@@ -1387,7 +1387,7 @@ def parse_test_results_xml_file(args, item, item_name, tests, assemblies):
                     if len(name) > 0:
                         test_name += " (" + name + ")"
                     result = test.attrib["result"]
-                    time = float(collection.attrib["time"])
+                    time = float(collection.attrib["time"].replace(",", "."))
                     test_output = test.findtext("output")
                     tests.append(defaultdict(lambda: None, {
                         "name": test_name,