diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 25006ed41eb17a..bb04e83442dae9 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -6130,6 +6130,10 @@ class Compiler GenTree* lpTestTree; // pointer to the node containing the loop test genTreeOps lpTestOper() const; // the type of the comparison between the iterator and the limit (GT_LE, GT_GE, // etc.) + + bool lpIsIncreasingLoop() const; // if the loop iterator increases from low to high value. + bool lpIsDecreasingLoop() const; // if the loop iterator decreases from high to low value. + void VERIFY_lpTestTree() const; bool lpIsReversed() const; // true if the iterator node is the second operand in the loop condition diff --git a/src/coreclr/jit/compiler.hpp b/src/coreclr/jit/compiler.hpp index f3938b37beaf5a..916520f78b8e7a 100644 --- a/src/coreclr/jit/compiler.hpp +++ b/src/coreclr/jit/compiler.hpp @@ -3520,6 +3520,27 @@ inline genTreeOps Compiler::LoopDsc::lpTestOper() const //----------------------------------------------------------------------------- +inline bool Compiler::LoopDsc::lpIsIncreasingLoop() const +{ + // Increasing loop is the one that has "+=" increment operation and "< or <=" limit check. + bool isLessThanLimitCheck = GenTree::StaticOperIs(lpTestOper(), GT_LT, GT_LE); + return (isLessThanLimitCheck && + (((lpIterOper() == GT_ADD) && (lpIterConst() > 0)) || ((lpIterOper() == GT_SUB) && (lpIterConst() < 0)))); +} + +//----------------------------------------------------------------------------- + +inline bool Compiler::LoopDsc::lpIsDecreasingLoop() const +{ + // Decreasing loop is the one that has "-=" decrement operation and "> or >=" limit check. If the operation is + // "+=", make sure the constant is negative to give an effect of decrementing the iterator. + bool isGreaterThanLimitCheck = GenTree::StaticOperIs(lpTestOper(), GT_GT, GT_GE); + return (isGreaterThanLimitCheck && + (((lpIterOper() == GT_ADD) && (lpIterConst() < 0)) || ((lpIterOper() == GT_SUB) && (lpIterConst() > 0)))); +} + +//----------------------------------------------------------------------------- + inline GenTree* Compiler::LoopDsc::lpIterator() const { VERIFY_lpTestTree(); diff --git a/src/coreclr/jit/loopcloning.cpp b/src/coreclr/jit/loopcloning.cpp index 8f22a4899d7eee..a5c4d8ea6ed309 100644 --- a/src/coreclr/jit/loopcloning.cpp +++ b/src/coreclr/jit/loopcloning.cpp @@ -1022,18 +1022,29 @@ bool Compiler::optDeriveLoopCloningConditions(unsigned loopNum, LoopCloneContext JITDUMP("------------------------------------------------------------\n"); JITDUMP("Deriving cloning conditions for " FMT_LP "\n", loopNum); - LoopDsc* loop = &optLoopTable[loopNum]; - JitExpandArrayStack* optInfos = context->GetLoopOptInfo(loopNum); + LoopDsc* loop = &optLoopTable[loopNum]; + JitExpandArrayStack* optInfos = context->GetLoopOptInfo(loopNum); + bool isIncreasingLoop = loop->lpIsIncreasingLoop(); + assert(isIncreasingLoop || loop->lpIsDecreasingLoop()); - if (GenTree::StaticOperIs(loop->lpTestOper(), GT_LT, GT_LE)) + if (GenTree::StaticOperIs(loop->lpTestOper(), GT_LT, GT_LE, GT_GT, GT_GE)) { - // Stride conditions - if (loop->lpIterConst() <= 0) + // We already know that this is either increasing or decreasing loop and the + // stride is (> 0) or (< 0). Here, just take the abs() value and check if it + // is beyond the limit. + int stride = abs(loop->lpIterConst()); + + if (stride >= 58) { - JITDUMP("> Stride %d is invalid\n", loop->lpIterConst()); + // Array.MaxLength can have maximum of 0X7FFFFFC7 elements, so make sure + // the stride increment doesn't overflow or underflow the index. Hence, + // the maximum stride limit is set to + // (int.MaxValue - (Array.MaxLength - 1) + 1), which is + // (0X7fffffff - 0x7fffffc7 + 2) = 0x3a or 58. return false; } + LC_Ident ident; // Init conditions if (loop->lpFlags & LPFLG_CONST_INIT) { @@ -1045,6 +1056,12 @@ bool Compiler::optDeriveLoopCloningConditions(unsigned loopNum, LoopCloneContext JITDUMP("> Init %d is invalid\n", loop->lpConstInit); return false; } + + if (!isIncreasingLoop) + { + // For decreasing loop, the init value needs to be checked against the array length + ident = LC_Ident(static_cast(loop->lpConstInit), LC_Ident::Const); + } } else { @@ -1056,13 +1073,22 @@ bool Compiler::optDeriveLoopCloningConditions(unsigned loopNum, LoopCloneContext return false; } - LC_Condition geZero(GT_GE, LC_Expr(LC_Ident(initLcl, LC_Ident::Var)), - LC_Expr(LC_Ident(0, LC_Ident::Const))); + LC_Condition geZero; + if (isIncreasingLoop) + { + geZero = LC_Condition(GT_GE, LC_Expr(LC_Ident(initLcl, LC_Ident::Var)), + LC_Expr(LC_Ident(0, LC_Ident::Const))); + } + else + { + // For decreasing loop, the init value needs to be checked against the array length + ident = LC_Ident(initLcl, LC_Ident::Var); + geZero = LC_Condition(GT_GE, LC_Expr(ident), LC_Expr(LC_Ident(0, LC_Ident::Const))); + } context->EnsureConditions(loopNum)->Push(geZero); } // Limit Conditions - LC_Ident ident; if (loop->lpFlags & LPFLG_CONST_LIMIT) { int limit = loop->lpConstLimit(); @@ -1071,7 +1097,12 @@ bool Compiler::optDeriveLoopCloningConditions(unsigned loopNum, LoopCloneContext JITDUMP("> limit %d is invalid\n", limit); return false; } - ident = LC_Ident(static_cast(limit), LC_Ident::Const); + + if (isIncreasingLoop) + { + // For increasing loop, thelimit value needs to be checked against the array length + ident = LC_Ident(static_cast(limit), LC_Ident::Const); + } } else if (loop->lpFlags & LPFLG_VAR_LIMIT) { @@ -1082,8 +1113,18 @@ bool Compiler::optDeriveLoopCloningConditions(unsigned loopNum, LoopCloneContext return false; } - ident = LC_Ident(limitLcl, LC_Ident::Var); - LC_Condition geZero(GT_GE, LC_Expr(ident), LC_Expr(LC_Ident(0, LC_Ident::Const))); + LC_Condition geZero; + if (isIncreasingLoop) + { + // For increasing loop, thelimit value needs to be checked against the array length + ident = LC_Ident(limitLcl, LC_Ident::Var); + geZero = LC_Condition(GT_GE, LC_Expr(ident), LC_Expr(LC_Ident(0, LC_Ident::Const))); + } + else + { + geZero = LC_Condition(GT_GE, LC_Expr(LC_Ident(limitLcl, LC_Ident::Var)), + LC_Expr(LC_Ident(0, LC_Ident::Const))); + } context->EnsureConditions(loopNum)->Push(geZero); } @@ -1107,15 +1148,22 @@ bool Compiler::optDeriveLoopCloningConditions(unsigned loopNum, LoopCloneContext return false; } - // GT_LT loop test: limit <= arrLen - // GT_LE loop test: limit < arrLen + // Increasing loops + // GT_LT loop test: (start < end) ==> (end <= arrLen) + // GT_LE loop test: (start <= end) ==> (end < arrLen) + // + // Decreasing loops + // GT_GT loop test: (end > start) ==> (end <= arrLen) + // GT_GE loop test: (end >= start) ==> (end < arrLen) genTreeOps opLimitCondition; switch (loop->lpTestOper()) { case GT_LT: + case GT_GT: opLimitCondition = GT_LE; break; case GT_LE: + case GT_GE: opLimitCondition = GT_LT; break; default: @@ -1603,13 +1651,6 @@ bool Compiler::optIsLoopClonable(unsigned loopInd) return false; } - // TODO-CQ: CLONE: Mark increasing or decreasing loops. - if ((loop.lpIterOper() != GT_ADD) || (loop.lpIterConst() != 1)) - { - JITDUMP("Loop cloning: rejecting loop " FMT_LP ". Loop iteration operator not matching.\n", loopInd); - return false; - } - if ((loop.lpFlags & LPFLG_CONST_LIMIT) == 0 && (loop.lpFlags & LPFLG_VAR_LIMIT) == 0 && (loop.lpFlags & LPFLG_ARRLEN_LIMIT) == 0) { @@ -1618,8 +1659,12 @@ bool Compiler::optIsLoopClonable(unsigned loopInd) return false; } - if (!((GenTree::StaticOperIs(loop.lpTestOper(), GT_LT, GT_LE) && (loop.lpIterOper() == GT_ADD)) || - (GenTree::StaticOperIs(loop.lpTestOper(), GT_GT, GT_GE) && (loop.lpIterOper() == GT_SUB)))) + // TODO-CQ: Handle other loops like: + // - The ones whose limit operator is "==" or "!=" + // - The incrementing operator is multiple and divide + // - The ones that are inverted are not handled here for cases like "i *= 2" because + // they are converted to "i + i". + if (!(loop.lpIsIncreasingLoop() || loop.lpIsDecreasingLoop())) { JITDUMP("Loop cloning: rejecting loop " FMT_LP ". Loop test (%s) doesn't agree with the direction (%s) of the loop.\n", diff --git a/src/tests/JIT/Directed/Arrays/LoopCloning.cs b/src/tests/JIT/Directed/Arrays/LoopCloning.cs new file mode 100644 index 00000000000000..ccc5e45c6e4c67 --- /dev/null +++ b/src/tests/JIT/Directed/Arrays/LoopCloning.cs @@ -0,0 +1,34 @@ +// 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 System.Runtime.Intrinsics; + +public class Program +{ + public static unsafe int Main() + { + int result = 0; + try { + test_up_big(new int[10], 5, 2); + } catch (IndexOutOfRangeException) { + result = 100; + } + + return result; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + public static int test_up_big(int[] a, int s, int x) + { + int r = 0; + int i; + for (i = 1; i < s; i += 2147483647) + { + r += a[i]; + } + return r; + } +} \ No newline at end of file diff --git a/src/tests/JIT/Directed/Arrays/LoopCloning.csproj b/src/tests/JIT/Directed/Arrays/LoopCloning.csproj new file mode 100644 index 00000000000000..c8c892477bb411 --- /dev/null +++ b/src/tests/JIT/Directed/Arrays/LoopCloning.csproj @@ -0,0 +1,13 @@ + + + Exe + 1 + + + PdbOnly + True + + + + +