diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 6f51248f60d9c1..cf5ef037da0ec4 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -5635,6 +5635,15 @@ GenTree* Compiler::optAssertionProp_BndsChk(ASSERT_VALARG_TP assertions, // 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) && (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")); + } + } if (getIdxRng().IsConstantRange() && getLenRng().IsConstantRange()) { diff --git a/src/coreclr/jit/rangecheck.cpp b/src/coreclr/jit/rangecheck.cpp index a1b1faa2a28696..9f8a4000d83851 100644 --- a/src/coreclr/jit/rangecheck.cpp +++ b/src/coreclr/jit/rangecheck.cpp @@ -735,6 +735,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. @@ -817,6 +818,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; diff --git a/src/coreclr/jit/rangecheck.h b/src/coreclr/jit/rangecheck.h index a722fa4bf06c09..e63a80c51b9a81 100644 --- a/src/coreclr/jit/rangecheck.h +++ b/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. diff --git a/src/tests/JIT/opt/RangeChecks/ModLength.cs b/src/tests/JIT/opt/RangeChecks/ModLength.cs index fc9737b99e17c0..09c0bbfe10d565 100644 --- a/src/tests/JIT/opt/RangeChecks/ModLength.cs +++ b/src/tests/JIT/opt/RangeChecks/ModLength.cs @@ -7,7 +7,7 @@ public class ModLength { - [Fact] + [Fact] public static void TestEntryPoint() { Throws(() => Test1(new int[0], 0)); @@ -36,6 +36,38 @@ public static void TestEntryPoint() Test6(new int[10], 0); Test7(new int[10], 0); Throws(() => Test8(new int[10], 0)); + + Test10(new int[10], 9U); + Throws(() => Test10(new int[10], 0U)); + + Test11(new int[10], 0); + Test11(new int[10], 100); + Throws(() => Test11(new int[0], 10)); + + Test12(new int[10], 9); + Throws(() => Test12(new int[10], 0)); + + Test13(new int[10], 9U); + Test13(new int[10], 10U); + Throws(() => Test13(new int[10], 0U)); + + Test14(new int[10], 9); + Test14(new int[10], 10); + Throws(() => Test14(new int[10], 0)); + + Test15(new int[10], 9U); + Throws(() => Test15(new int[10], 0U)); + + Test16(new int[10], 9); + Throws(() => Test16(new int[10], 0)); + + Test17(new int[10], 9U); + Test17(new int[10], 10U); + Throws(() => Test17(new int[10], 0U)); + + Test18(new int[10], 9); + Test18(new int[10], 10); + Throws(() => Test18(new int[10], 0)); } static void Throws(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; + } } diff --git a/src/tests/run.py b/src/tests/run.py index 0be0b8514259d2..b022e227afb2cf 100755 --- a/src/tests/run.py +++ b/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 `` tag within ``. @@ -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,