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.

2 files changed, 15 insertions, 1 deletions

diff --git a/src/jit/gentree.h b/src/jit/gentree.h
index ee80ce5d64..9756dcbd6a 100644
--- a/src/jit/gentree.h
+++ b/src/jit/gentree.h
@@ -846,6 +846,7 @@ public:
     #define GTF_RELOP_JMP_USED  0x40000000  // GT_<relop> -- result of compare used for jump or ?:
     #define GTF_RELOP_QMARK     0x20000000  // GT_<relop> -- the node is the condition for ?:
     #define GTF_RELOP_SMALL     0x10000000  // GT_<relop> -- We should use a byte or short sized compare (op1->gtType is the small type)
+    #define GTF_RELOP_ZTT       0x08000000  // GT_<relop> -- 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 ff22902e55..67c963fb92 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));