From 6210350903b778e5068ba6021dccaf4a64abea52 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Thu, 10 Sep 2026 14:19:45 -0700 Subject: [PATCH 1/2] JIT: fix checked subtraction loop unrolling Normalize subtraction increments to additive steps before computing loop direction and repetition counts. Use widened arithmetic so negating int.MinValue remains well-defined. Fixes #133583. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c9f8aa93-53c3-44cb-bc92-371747be895c --- src/coreclr/jit/optimizer.cpp | 61 +++++----- .../JitBlue/Runtime_133583/Runtime_133583.cs | 104 ++++++++++++++++++ .../Runtime_133583/Runtime_133583.csproj | 12 ++ 3 files changed, 143 insertions(+), 34 deletions(-) create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_133583/Runtime_133583.cs create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_133583/Runtime_133583.csproj diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp index bd37f5731be4cd..c5d6728c1e785c 100644 --- a/src/coreclr/jit/optimizer.cpp +++ b/src/coreclr/jit/optimizer.cpp @@ -794,6 +794,7 @@ bool Compiler::optComputeLoopRep(int constInit, int64_t constInitX; int64_t constLimitX; + int64_t iterIncX; unsigned loopCount; int iterSign; @@ -847,17 +848,24 @@ bool Compiler::optComputeLoopRep(int constInit, NO_WAY("Bad type"); } - // If iterInc is zero we have an infinite loop. - if (iterInc == 0) + // Normalize subtraction into an additive step before reasoning about loop direction. + iterIncX = iterInc; + if (iterOper == GT_SUB) + { + iterIncX = -iterIncX; + } + + // If iterIncX is zero we have an infinite loop. + if (iterIncX == 0) { return false; } - iterSign = (iterInc > 0) ? +1 : -1; + iterSign = (iterIncX > 0) ? +1 : -1; loopCount = 0; // bail if count is based on wrap-around math - if (iterInc > 0) + if (iterIncX > 0) { if (constLimitX < constInitX) { @@ -886,12 +894,12 @@ bool Compiler::optComputeLoopRep(int constInit, // If "mod iterInc" is not zero then the limit test will miss and a wrap will occur // which is probably not what the end user wanted, but it is legal. - if (iterInc > 0) + if (iterIncX > 0) { // Stepping by one, i.e. Mod with 1 is always zero. - if (iterInc != 1) + if (iterIncX != 1) { - if (((constLimitX - constInitX) % iterInc) != 0) + if (((constLimitX - constInitX) % iterIncX) != 0) { return false; } @@ -900,9 +908,9 @@ bool Compiler::optComputeLoopRep(int constInit, else { // Stepping by -1, i.e. Mod with 1 is always zero. - if (iterInc != -1) + if (iterIncX != -1) { - if (((constInitX - constLimitX) % (-iterInc)) != 0) + if (((constInitX - constLimitX) % (-iterIncX)) != 0) { return false; } @@ -912,16 +920,13 @@ bool Compiler::optComputeLoopRep(int constInit, switch (iterOper) { case GT_SUB: - iterInc = -iterInc; - FALLTHROUGH; - case GT_ADD: if (constInitX != constLimitX) { - loopCount += (unsigned)((constLimitX - constInitX - iterSign) / iterInc) + 1; + loopCount += (unsigned)((constLimitX - constInitX - iterSign) / iterIncX) + 1; } - iterAtExitX = (int)(constInitX + iterInc * (int)loopCount); + iterAtExitX = (int)(constInitX + iterIncX * (int)loopCount); if (unsTest) { @@ -959,16 +964,13 @@ bool Compiler::optComputeLoopRep(int constInit, switch (iterOper) { case GT_SUB: - iterInc = -iterInc; - FALLTHROUGH; - case GT_ADD: if (constInitX < constLimitX) { - loopCount += (unsigned)((constLimitX - constInitX - iterSign) / iterInc) + 1; + loopCount += (unsigned)((constLimitX - constInitX - iterSign) / iterIncX) + 1; } - iterAtExitX = (int)(constInitX + iterInc * (int)loopCount); + iterAtExitX = (int)(constInitX + iterIncX * (int)loopCount); if (unsTest) { @@ -1006,16 +1008,13 @@ bool Compiler::optComputeLoopRep(int constInit, switch (iterOper) { case GT_SUB: - iterInc = -iterInc; - FALLTHROUGH; - case GT_ADD: if (constInitX <= constLimitX) { - loopCount += (unsigned)((constLimitX - constInitX) / iterInc) + 1; + loopCount += (unsigned)((constLimitX - constInitX) / iterIncX) + 1; } - iterAtExitX = (int)(constInitX + iterInc * (int)loopCount); + iterAtExitX = (int)(constInitX + iterIncX * (int)loopCount); if (unsTest) { @@ -1053,16 +1052,13 @@ bool Compiler::optComputeLoopRep(int constInit, switch (iterOper) { case GT_SUB: - iterInc = -iterInc; - FALLTHROUGH; - case GT_ADD: if (constInitX > constLimitX) { - loopCount += (unsigned)((constLimitX - constInitX - iterSign) / iterInc) + 1; + loopCount += (unsigned)((constLimitX - constInitX - iterSign) / iterIncX) + 1; } - iterAtExitX = (int)(constInitX + iterInc * (int)loopCount); + iterAtExitX = (int)(constInitX + iterIncX * (int)loopCount); if (unsTest) { @@ -1100,16 +1096,13 @@ bool Compiler::optComputeLoopRep(int constInit, switch (iterOper) { case GT_SUB: - iterInc = -iterInc; - FALLTHROUGH; - case GT_ADD: if (constInitX >= constLimitX) { - loopCount += (unsigned)((constLimitX - constInitX) / iterInc) + 1; + loopCount += (unsigned)((constLimitX - constInitX) / iterIncX) + 1; } - iterAtExitX = (int)(constInitX + iterInc * (int)loopCount); + iterAtExitX = (int)(constInitX + iterIncX * (int)loopCount); if (unsTest) { diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_133583/Runtime_133583.cs b/src/tests/JIT/Regression/JitBlue/Runtime_133583/Runtime_133583.cs new file mode 100644 index 00000000000000..d33e8a53e5058d --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_133583/Runtime_133583.cs @@ -0,0 +1,104 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Runtime.CompilerServices; +using Xunit; + +public class Runtime_133583 +{ + [Fact] + public static void TestEntryPoint() + { + Assert.Equal(2, NotEqual()); + Assert.Equal(2, LessThan()); + Assert.Equal(3, LessThanOrEqual()); + Assert.Equal(2, GreaterThan()); + Assert.Equal(3, GreaterThanOrEqual()); + Assert.Throws(() => { _ = NotEqualOverflow(); }); + } + + [MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.AggressiveOptimization)] + private static int NotEqual() + { + int count = 0; + int i = 0; + while (i != 2) + { + count++; + i = checked(i - (-1)); + } + + return count; + } + + [MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.AggressiveOptimization)] + private static int LessThan() + { + int count = 0; + int i = -2; + while (i < 0) + { + count++; + i = checked(i - (-1)); + } + + return count; + } + + [MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.AggressiveOptimization)] + private static int LessThanOrEqual() + { + int count = 0; + int i = -2; + while (i <= 0) + { + count++; + i = checked(i - (-1)); + } + + return count; + } + + [MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.AggressiveOptimization)] + private static int GreaterThan() + { + int count = 0; + int i = 2; + while (i > 0) + { + count++; + i = checked(i - 1); + } + + return count; + } + + [MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.AggressiveOptimization)] + private static int GreaterThanOrEqual() + { + int count = 0; + int i = 2; + while (i >= 0) + { + count++; + i = checked(i - 1); + } + + return count; + } + + [MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.AggressiveOptimization)] + private static int NotEqualOverflow() + { + int count = 0; + int i = int.MaxValue - 1; + while (i != int.MaxValue - 2) + { + count++; + i = checked(i - (-1)); + } + + return count; + } +} diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_133583/Runtime_133583.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_133583/Runtime_133583.csproj new file mode 100644 index 00000000000000..0f887574c5b813 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_133583/Runtime_133583.csproj @@ -0,0 +1,12 @@ + + + True + 1 + true + + + + + + + From 195170f08234a46bb62c83ca8d1adfc37f5fc1d9 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Fri, 11 Sep 2026 07:39:25 -0700 Subject: [PATCH 2/2] JIT: test subtraction by int.MinValue Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c9f8aa93-53c3-44cb-bc92-371747be895c --- .../JitBlue/Runtime_133583/Runtime_133583.cs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_133583/Runtime_133583.cs b/src/tests/JIT/Regression/JitBlue/Runtime_133583/Runtime_133583.cs index d33e8a53e5058d..84b39609603d19 100644 --- a/src/tests/JIT/Regression/JitBlue/Runtime_133583/Runtime_133583.cs +++ b/src/tests/JIT/Regression/JitBlue/Runtime_133583/Runtime_133583.cs @@ -15,6 +15,7 @@ public static void TestEntryPoint() Assert.Equal(3, LessThanOrEqual()); Assert.Equal(2, GreaterThan()); Assert.Equal(3, GreaterThanOrEqual()); + Assert.Equal(1, SubtractIntMinValue()); Assert.Throws(() => { _ = NotEqualOverflow(); }); } @@ -88,6 +89,20 @@ private static int GreaterThanOrEqual() return count; } + [MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.AggressiveOptimization)] + private static int SubtractIntMinValue() + { + int count = 0; + int i = int.MinValue; + while (i != 0) + { + count++; + i = checked(i - int.MinValue); + } + + return count; + } + [MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.AggressiveOptimization)] private static int NotEqualOverflow() {