From 8657824294d106720cef63d872beeca7bc277e1b Mon Sep 17 00:00:00 2001 From: Russ Keldorph Date: Mon, 11 Jan 2016 16:51:49 -0800 Subject: [PATCH] Do loop cloning only if zero trip test can be ensured. The problem is our loop detection logic detects the loop structure, but it doesnt know the code outside the loop structure, esp., nothing about the edge from the "head" into the loop "entry" block. In the bug case, there is no zero-trip test in the "head", so the entrance into the loop is not guarded. Note that the other point of entrance into the "entry" block is from the "top" block which will be guarded by the loop "bottom" test. One way to make sure is when we invert a while loop into a do-while with an explicit compiler cloned zero trip test, in fgOptWhileLoop, i.e., we mark it as good to optimize with a flag. The fix marks the loop as ZTT. The caveat is the JIT doesnt always do loop inversion. This change is more conservative than it needs to be in the interest of managing risk. --- src/jit/gentree.h | 1 + src/jit/optimizer.cpp | 15 ++++++++++- tests/src/JIT/RyuJIT/DoWhileBndChk.cs | 40 +++++++++++++++++++++++++++++ tests/src/JIT/RyuJIT/DoWhileBndChk.csproj | 42 +++++++++++++++++++++++++++++++ tests/src/JIT/RyuJIT/app.config | 27 ++++++++++++++++++++ 5 files changed, 124 insertions(+), 1 deletion(-) create mode 100644 tests/src/JIT/RyuJIT/DoWhileBndChk.cs create mode 100644 tests/src/JIT/RyuJIT/DoWhileBndChk.csproj create mode 100644 tests/src/JIT/RyuJIT/app.config diff --git a/src/jit/gentree.h b/src/jit/gentree.h index ee80ce5..9756dcb 100644 --- a/src/jit/gentree.h +++ b/src/jit/gentree.h @@ -846,6 +846,7 @@ public: #define GTF_RELOP_JMP_USED 0x40000000 // GT_ -- result of compare used for jump or ?: #define GTF_RELOP_QMARK 0x20000000 // GT_ -- the node is the condition for ?: #define GTF_RELOP_SMALL 0x10000000 // GT_ -- We should use a byte or short sized compare (op1->gtType is the small type) + #define GTF_RELOP_ZTT 0x08000000 // GT_ -- Loop test cloned for converting while-loops into do-while with explicit "loop test" in the header block. #define GTF_QMARK_CAST_INSTOF 0x80000000 // GT_QMARK -- Is this a top (not nested) level qmark created for castclass or instanceof? diff --git a/src/jit/optimizer.cpp b/src/jit/optimizer.cpp index ff22902..67c963f 100644 --- a/src/jit/optimizer.cpp +++ b/src/jit/optimizer.cpp @@ -3428,8 +3428,9 @@ void Compiler::fgOptWhileLoop(BasicBlock * block) #ifdef DEBUG if (verbose) { - printf("\nDuplication of loop condition %s, because the cost of duplication (%i) is %s than %i," + printf("\nDuplication of loop condition [%06u] is %s, because the cost of duplication (%i) is %s than %i," "\n loopIterations = %7.3f, countOfHelpers = %d, validProfileWeights = %s\n", + condTree->gtTreeID, costIsTooHigh ? "not done" : "performed", estDupCostSz, costIsTooHigh ? "greater" : "less or equal", @@ -3446,9 +3447,14 @@ void Compiler::fgOptWhileLoop(BasicBlock * block) /* Looks good - duplicate the condition test */ + condTree->gtFlags |= GTF_RELOP_ZTT; + condTree = gtCloneExpr(condTree); gtReverseCond(condTree); + // Make sure clone expr copied the flag + assert(condTree->gtFlags & GTF_RELOP_ZTT); + condTree = gtNewOperNode(GT_JTRUE, TYP_VOID, condTree); /* Create a statement entry out of the condition and @@ -7270,6 +7276,13 @@ bool Compiler::optIdentifyLoopOptInfo(unsigned loopNum, LoopCloneC GenTree::NodeName(pLoop->lpTestOper()), GenTree::NodeName(pLoop->lpIterOper())); return false; } + + if (!(pLoop->lpTestTree->OperKind() & GTK_RELOP) || !(pLoop->lpTestTree->gtFlags & GTF_RELOP_ZTT)) + { + JITDUMP("> Loop inversion NOT present, loop test [%06u] may not protect entry from head.\n", pLoop->lpTestTree->gtTreeID); + return false; + } + #ifdef DEBUG GenTreePtr op1 = pLoop->lpIterator(); noway_assert((op1->gtOper == GT_LCL_VAR) && (op1->gtLclVarCommon.gtLclNum == ivLclNum)); diff --git a/tests/src/JIT/RyuJIT/DoWhileBndChk.cs b/tests/src/JIT/RyuJIT/DoWhileBndChk.cs new file mode 100644 index 0000000..817a51b --- /dev/null +++ b/tests/src/JIT/RyuJIT/DoWhileBndChk.cs @@ -0,0 +1,40 @@ +// Copyright (c) Microsoft. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. +// +// Ensure bounds checks aren't elided for this do-while loop. +// +// Reference: TF Bug 150041 + +using System; +using System.Runtime.ExceptionServices; + +public class Program +{ + [HandleProcessCorruptedStateExceptions] + public static int Main() + { + int ret = 99; + + try + { + int[] a = new int[0]; + int i = 0x1FFFFFFF; + do + { + a[i] = 0; + ++i; + } + while (i < a.Length); + } + catch (Exception e) + { + if (e is IndexOutOfRangeException) + { + ret = 100; + } + } + + return ret; + } +} + diff --git a/tests/src/JIT/RyuJIT/DoWhileBndChk.csproj b/tests/src/JIT/RyuJIT/DoWhileBndChk.csproj new file mode 100644 index 0000000..bd26841 --- /dev/null +++ b/tests/src/JIT/RyuJIT/DoWhileBndChk.csproj @@ -0,0 +1,42 @@ + + + + + Debug + AnyCPU + $(MSBuildProjectName) + 2.0 + {95DFC527-4DC1-495E-97D7-E94EE1F7140D} + Exe + Properties + 512 + {786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC} + $(ProgramFiles)\Common Files\microsoft shared\VSTT\11.0\UITestExtensionPackages + ..\..\ + 7a9bfb7d + true + True + + + + False + + + + + + + + + + + + + + $(JitPackagesConfigFileDirectory)empty\project.json + $(JitPackagesConfigFileDirectory)empty\project.lock.json + + + + + diff --git a/tests/src/JIT/RyuJIT/app.config b/tests/src/JIT/RyuJIT/app.config new file mode 100644 index 0000000..c752cb0 --- /dev/null +++ b/tests/src/JIT/RyuJIT/app.config @@ -0,0 +1,27 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + -- 2.7.4