From 11e21dfd7d477627b5c07b9fd7e8c57778166868 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Tue, 12 Apr 2022 13:16:34 -0700 Subject: [PATCH 1/7] Handle following scenarios for loop cloning: - Increasing loops that are incremented by more than 1 like "i += 2" - Decreasing loops like "i--", "i -= 2" --- src/coreclr/jit/loopcloning.cpp | 92 ++++++++++++++++++++++++--------- 1 file changed, 67 insertions(+), 25 deletions(-) diff --git a/src/coreclr/jit/loopcloning.cpp b/src/coreclr/jit/loopcloning.cpp index 8f22a4899d7eee..42e29e156ef5a8 100644 --- a/src/coreclr/jit/loopcloning.cpp +++ b/src/coreclr/jit/loopcloning.cpp @@ -1024,17 +1024,13 @@ bool Compiler::optDeriveLoopCloningConditions(unsigned loopNum, LoopCloneContext LoopDsc* loop = &optLoopTable[loopNum]; JitExpandArrayStack* optInfos = context->GetLoopOptInfo(loopNum); + bool isIncreasingLoop = GenTree::StaticOperIs(loop->lpTestOper(), GT_LT, GT_LE); - 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) - { - JITDUMP("> Stride %d is invalid\n", loop->lpIterConst()); - return false; - } + LC_Ident ident; - // Init conditions + // Init conditions if (loop->lpFlags & LPFLG_CONST_INIT) { // Only allowing non-negative const init at this time. @@ -1045,6 +1041,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 +1058,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 +1082,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 +1098,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 +1133,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 <= start) + // GT_LE loop test: (start <= end) ==> (end < start) + // + // Decreasing loops + // GT_GT loop test: (end > start) ==> (end <= start) + // GT_GE loop test: (end >= start) ==> (end < start) 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 +1636,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 +1644,24 @@ 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)))) + bool isLessThanLimitCheck = GenTree::StaticOperIs(loop.lpTestOper(), GT_LT, GT_LE); + bool isGreaterThanLimitCheck = GenTree::StaticOperIs(loop.lpTestOper(), GT_GT, GT_GE); + + // Increasing loop is the one that has "+=" increament operation and "< or <=" limit check. + bool isIncreasingLoop = (isLessThanLimitCheck && (loop.lpIterOper() == GT_ADD)); + + // Increasing 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 isDecreasingLoop = (isGreaterThanLimitCheck && ((loop.lpIterOper() == GT_SUB) || + ((loop.lpIterOper() == GT_ADD) && (loop.lpIterConst() < 0)))); + + //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". + //bool isOtherLoop = (loop.lpIterOper() == GT_DIV) || (loop.lpIterOper() == GT_MUL); + if (!(isIncreasingLoop || isDecreasingLoop)) { JITDUMP("Loop cloning: rejecting loop " FMT_LP ". Loop test (%s) doesn't agree with the direction (%s) of the loop.\n", From a21bd23e05d3bb5784de25ffd386d3e336a3493b Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Tue, 12 Apr 2022 14:37:15 -0700 Subject: [PATCH 2/7] jit format --- src/coreclr/jit/loopcloning.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/coreclr/jit/loopcloning.cpp b/src/coreclr/jit/loopcloning.cpp index 42e29e156ef5a8..72f7ac029c5516 100644 --- a/src/coreclr/jit/loopcloning.cpp +++ b/src/coreclr/jit/loopcloning.cpp @@ -1022,15 +1022,15 @@ 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 = GenTree::StaticOperIs(loop->lpTestOper(), GT_LT, GT_LE); if (GenTree::StaticOperIs(loop->lpTestOper(), GT_LT, GT_LE, GT_GT, GT_GE)) { LC_Ident ident; - // Init conditions + // Init conditions if (loop->lpFlags & LPFLG_CONST_INIT) { // Only allowing non-negative const init at this time. @@ -1067,7 +1067,7 @@ bool Compiler::optDeriveLoopCloningConditions(unsigned loopNum, LoopCloneContext else { // For decreasing loop, the init value needs to be checked against the array length - ident = LC_Ident(initLcl, LC_Ident::Var); + 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); @@ -1644,7 +1644,7 @@ bool Compiler::optIsLoopClonable(unsigned loopInd) return false; } - bool isLessThanLimitCheck = GenTree::StaticOperIs(loop.lpTestOper(), GT_LT, GT_LE); + bool isLessThanLimitCheck = GenTree::StaticOperIs(loop.lpTestOper(), GT_LT, GT_LE); bool isGreaterThanLimitCheck = GenTree::StaticOperIs(loop.lpTestOper(), GT_GT, GT_GE); // Increasing loop is the one that has "+=" increament operation and "< or <=" limit check. @@ -1655,12 +1655,12 @@ bool Compiler::optIsLoopClonable(unsigned loopInd) bool isDecreasingLoop = (isGreaterThanLimitCheck && ((loop.lpIterOper() == GT_SUB) || ((loop.lpIterOper() == GT_ADD) && (loop.lpIterConst() < 0)))); - //TODO-CQ: Handle other loops like: + // 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". - //bool isOtherLoop = (loop.lpIterOper() == GT_DIV) || (loop.lpIterOper() == GT_MUL); + // bool isOtherLoop = (loop.lpIterOper() == GT_DIV) || (loop.lpIterOper() == GT_MUL); if (!(isIncreasingLoop || isDecreasingLoop)) { JITDUMP("Loop cloning: rejecting loop " FMT_LP From d74d920269c543bab1b4c64a32834f39e01feed9 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Wed, 13 Apr 2022 12:13:32 -0700 Subject: [PATCH 3/7] Make the loop cloning condition tighter --- src/coreclr/jit/loopcloning.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/coreclr/jit/loopcloning.cpp b/src/coreclr/jit/loopcloning.cpp index 72f7ac029c5516..35151584b14fdd 100644 --- a/src/coreclr/jit/loopcloning.cpp +++ b/src/coreclr/jit/loopcloning.cpp @@ -1648,12 +1648,13 @@ bool Compiler::optIsLoopClonable(unsigned loopInd) bool isGreaterThanLimitCheck = GenTree::StaticOperIs(loop.lpTestOper(), GT_GT, GT_GE); // Increasing loop is the one that has "+=" increament operation and "< or <=" limit check. - bool isIncreasingLoop = (isLessThanLimitCheck && (loop.lpIterOper() == GT_ADD)); + bool isIncreasingLoop = (isLessThanLimitCheck && (((loop.lpIterOper() == GT_ADD) && (loop.lpIterConst() > 0)) || + ((loop.lpIterOper() == GT_SUB) && (loop.lpIterConst() < 0)))); // Increasing 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 isDecreasingLoop = (isGreaterThanLimitCheck && ((loop.lpIterOper() == GT_SUB) || - ((loop.lpIterOper() == GT_ADD) && (loop.lpIterConst() < 0)))); + bool isDecreasingLoop = (isGreaterThanLimitCheck && (((loop.lpIterOper() == GT_ADD) && (loop.lpIterConst() < 0)) || + ((loop.lpIterOper() == GT_SUB) && (loop.lpIterConst() > 0)))); // TODO-CQ: Handle other loops like: // - The ones whose limit operator is "==" or "!=" From 6830fb9e21eb86c99200e845aaea8e1de637f320 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Thu, 14 Apr 2022 22:38:16 -0700 Subject: [PATCH 4/7] check for stride --- src/coreclr/jit/loopcloning.cpp | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/loopcloning.cpp b/src/coreclr/jit/loopcloning.cpp index 35151584b14fdd..1693c8ef5a133a 100644 --- a/src/coreclr/jit/loopcloning.cpp +++ b/src/coreclr/jit/loopcloning.cpp @@ -1028,7 +1028,37 @@ bool Compiler::optDeriveLoopCloningConditions(unsigned loopNum, LoopCloneContext if (GenTree::StaticOperIs(loop->lpTestOper(), GT_LT, GT_LE, GT_GT, GT_GE)) { - LC_Ident ident; + + bool isLessThanLimitCheck = GenTree::StaticOperIs(loop->lpTestOper(), GT_LT, GT_LE); + bool isGreaterThanLimitCheck = GenTree::StaticOperIs(loop->lpTestOper(), GT_GT, GT_GE); + + // Increasing loop is the one that has "+=" increament operation and "< or <=" limit check. + bool isIncreasingLoop = + (isLessThanLimitCheck && (((loop->lpIterOper() == GT_ADD) && (loop->lpIterConst() > 0)) || + ((loop->lpIterOper() == GT_SUB) && (loop->lpIterConst() < 0)))); + + // Increasing 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 isDecreasingLoop = + (isGreaterThanLimitCheck && (((loop->lpIterOper() == GT_ADD) && (loop->lpIterConst() < 0)) || + ((loop->lpIterOper() == GT_SUB) && (loop->lpIterConst() > 0)))); + + int stride = loop->lpIterConst(); + if (isIncreasingLoop) + { + stride = stride > 0 ? stride : -stride; + } + else if (isDecreasingLoop) + { + stride = stride < 0 ? -stride : stride; + } + else + { + assert("Should not be here. the loop is not detected increasing or decreasing.\n"); + } + + if (stride > 58) + // Init conditions if (loop->lpFlags & LPFLG_CONST_INIT) From c15f325f9c4a166ced5f7d1c963db0ba3d8bd20b Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Mon, 18 Apr 2022 12:51:29 -0700 Subject: [PATCH 5/7] Add a check for stride edge scenario --- src/coreclr/jit/compiler.h | 4 +++ src/coreclr/jit/compiler.hpp | 21 +++++++++++++ src/coreclr/jit/loopcloning.cpp | 55 +++++++++------------------------ 3 files changed, 39 insertions(+), 41 deletions(-) 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..acb8a9a1aab44d 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 "+=" increament 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 1693c8ef5a133a..0b115e173d6f9f 100644 --- a/src/coreclr/jit/loopcloning.cpp +++ b/src/coreclr/jit/loopcloning.cpp @@ -1025,41 +1025,26 @@ bool Compiler::optDeriveLoopCloningConditions(unsigned loopNum, LoopCloneContext LoopDsc* loop = &optLoopTable[loopNum]; JitExpandArrayStack* optInfos = context->GetLoopOptInfo(loopNum); bool isIncreasingLoop = GenTree::StaticOperIs(loop->lpTestOper(), GT_LT, GT_LE); + assert(loop->lpIsIncreasingLoop() || loop->lpIsDecreasingLoop()); if (GenTree::StaticOperIs(loop->lpTestOper(), GT_LT, GT_LE, GT_GT, GT_GE)) { + // 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()); - bool isLessThanLimitCheck = GenTree::StaticOperIs(loop->lpTestOper(), GT_LT, GT_LE); - bool isGreaterThanLimitCheck = GenTree::StaticOperIs(loop->lpTestOper(), GT_GT, GT_GE); - - // Increasing loop is the one that has "+=" increament operation and "< or <=" limit check. - bool isIncreasingLoop = - (isLessThanLimitCheck && (((loop->lpIterOper() == GT_ADD) && (loop->lpIterConst() > 0)) || - ((loop->lpIterOper() == GT_SUB) && (loop->lpIterConst() < 0)))); - - // Increasing 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 isDecreasingLoop = - (isGreaterThanLimitCheck && (((loop->lpIterOper() == GT_ADD) && (loop->lpIterConst() < 0)) || - ((loop->lpIterOper() == GT_SUB) && (loop->lpIterConst() > 0)))); - - int stride = loop->lpIterConst(); - if (isIncreasingLoop) - { - stride = stride > 0 ? stride : -stride; - } - else if (isDecreasingLoop) - { - stride = stride < 0 ? -stride : stride; - } - else + if (stride > 58) { - assert("Should not be here. the loop is not detected increasing or decreasing.\n"); + // Array.MaxLength can have maximum of 0X7FFFFFC7 elements, so make sure + // the stride increament doesn't overflow or underflow the index. Hence, + // the maximum stride limit is set to + // (int.MaxValue - (Array.MaxLength - 1) + 1), which is + // (0X7FFFFFC7 - 0x7fffffc7 + 2) = 0x3a or 58. + return false; } - if (stride > 58) - - + LC_Ident ident; // Init conditions if (loop->lpFlags & LPFLG_CONST_INIT) { @@ -1674,25 +1659,13 @@ bool Compiler::optIsLoopClonable(unsigned loopInd) return false; } - bool isLessThanLimitCheck = GenTree::StaticOperIs(loop.lpTestOper(), GT_LT, GT_LE); - bool isGreaterThanLimitCheck = GenTree::StaticOperIs(loop.lpTestOper(), GT_GT, GT_GE); - - // Increasing loop is the one that has "+=" increament operation and "< or <=" limit check. - bool isIncreasingLoop = (isLessThanLimitCheck && (((loop.lpIterOper() == GT_ADD) && (loop.lpIterConst() > 0)) || - ((loop.lpIterOper() == GT_SUB) && (loop.lpIterConst() < 0)))); - - // Increasing 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 isDecreasingLoop = (isGreaterThanLimitCheck && (((loop.lpIterOper() == GT_ADD) && (loop.lpIterConst() < 0)) || - ((loop.lpIterOper() == GT_SUB) && (loop.lpIterConst() > 0)))); - // 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". // bool isOtherLoop = (loop.lpIterOper() == GT_DIV) || (loop.lpIterOper() == GT_MUL); - if (!(isIncreasingLoop || isDecreasingLoop)) + 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", From bd47a52370f38a841d353f1aff7b48b5bfcbd861 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Tue, 19 Apr 2022 09:11:21 -0700 Subject: [PATCH 6/7] Add test case --- src/tests/JIT/Directed/Arrays/LoopCloning.cs | 34 +++++++++++++++++++ .../JIT/Directed/Arrays/LoopCloning.csproj | 13 +++++++ 2 files changed, 47 insertions(+) create mode 100644 src/tests/JIT/Directed/Arrays/LoopCloning.cs create mode 100644 src/tests/JIT/Directed/Arrays/LoopCloning.csproj 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 + + + + + From 32e8320be87d9c33a67e82e5f9e9f90ade091484 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Fri, 22 Apr 2022 18:03:08 -0700 Subject: [PATCH 7/7] Review feedback --- src/coreclr/jit/compiler.hpp | 2 +- src/coreclr/jit/loopcloning.cpp | 19 +++++++++---------- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/src/coreclr/jit/compiler.hpp b/src/coreclr/jit/compiler.hpp index acb8a9a1aab44d..916520f78b8e7a 100644 --- a/src/coreclr/jit/compiler.hpp +++ b/src/coreclr/jit/compiler.hpp @@ -3522,7 +3522,7 @@ inline genTreeOps Compiler::LoopDsc::lpTestOper() const inline bool Compiler::LoopDsc::lpIsIncreasingLoop() const { - // Increasing loop is the one that has "+=" increament operation and "< or <=" limit check. + // 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)))); diff --git a/src/coreclr/jit/loopcloning.cpp b/src/coreclr/jit/loopcloning.cpp index 0b115e173d6f9f..a5c4d8ea6ed309 100644 --- a/src/coreclr/jit/loopcloning.cpp +++ b/src/coreclr/jit/loopcloning.cpp @@ -1024,8 +1024,8 @@ bool Compiler::optDeriveLoopCloningConditions(unsigned loopNum, LoopCloneContext LoopDsc* loop = &optLoopTable[loopNum]; JitExpandArrayStack* optInfos = context->GetLoopOptInfo(loopNum); - bool isIncreasingLoop = GenTree::StaticOperIs(loop->lpTestOper(), GT_LT, GT_LE); - assert(loop->lpIsIncreasingLoop() || loop->lpIsDecreasingLoop()); + bool isIncreasingLoop = loop->lpIsIncreasingLoop(); + assert(isIncreasingLoop || loop->lpIsDecreasingLoop()); if (GenTree::StaticOperIs(loop->lpTestOper(), GT_LT, GT_LE, GT_GT, GT_GE)) { @@ -1034,13 +1034,13 @@ bool Compiler::optDeriveLoopCloningConditions(unsigned loopNum, LoopCloneContext // is beyond the limit. int stride = abs(loop->lpIterConst()); - if (stride > 58) + if (stride >= 58) { // Array.MaxLength can have maximum of 0X7FFFFFC7 elements, so make sure - // the stride increament doesn't overflow or underflow the index. Hence, + // 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 - // (0X7FFFFFC7 - 0x7fffffc7 + 2) = 0x3a or 58. + // (0X7fffffff - 0x7fffffc7 + 2) = 0x3a or 58. return false; } @@ -1149,12 +1149,12 @@ bool Compiler::optDeriveLoopCloningConditions(unsigned loopNum, LoopCloneContext } // Increasing loops - // GT_LT loop test: (start < end) ==> (end <= start) - // GT_LE loop test: (start <= end) ==> (end < start) + // 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 <= start) - // GT_GE loop test: (end >= start) ==> (end < start) + // GT_GT loop test: (end > start) ==> (end <= arrLen) + // GT_GE loop test: (end >= start) ==> (end < arrLen) genTreeOps opLimitCondition; switch (loop->lpTestOper()) { @@ -1664,7 +1664,6 @@ bool Compiler::optIsLoopClonable(unsigned loopInd) // - 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". - // bool isOtherLoop = (loop.lpIterOper() == GT_DIV) || (loop.lpIterOper() == GT_MUL); if (!(loop.lpIsIncreasingLoop() || loop.lpIsDecreasingLoop())) { JITDUMP("Loop cloning: rejecting loop " FMT_LP