Fix embedded temporary creation

src/jit/lower.cpp

@@ -864,6 +864,78 @@ GenTreePtr Lowering::CreateLocalTempAsg(GenTreePtr rhs,
     return store;
 }
 
+//-----------------------------------------------------------------------------------------------
+// CreateTemporary: Store the result of the given tree in a newly created temporary local
+//                  variable and replace the original use of the tree with the temporary.
+//
+// Arguments:
+//    ppTree - a pointer to the tree use to replace.
+//
+// Return Value:
+//    The newly created store statement.
+//
+// Assumptions:
+//    This may only be called during tree lowering. The callee must ensure that the tree has already
+//    been lowered and is part of compCurStmt and that compCurStmt is in compCurBB.
+//
+// Notes:
+//    The newly created statement is usually an embedded statement but it can also be a top-level
+//    statement if the tree to be replaced extends to the begining of the current statement. If
+//    a top-level statement is created any embedded statements contained in the tree move to the
+//    the new top-level statement, before the current statement. Such embedded statements need to 
+//    be lowered here because the normal lowering code path won't reach them anymore.
+//
+// TODO-Cleanup: 
+//    Some uses of fgInsertEmbeddedFormTemp in lowering could be replaced with this to avoid 
+//    duplication, see LowerArrElem for example.
+
+GenTreeStmt* Lowering::CreateTemporary(GenTree** ppTree)
+{
+    GenTreeStmt* newStmt = comp->fgInsertEmbeddedFormTemp(ppTree);
+
+    // The tree is assumed to be already lowered so the newly created statement 
+    // should not be lowered again.
+    newStmt->gtFlags |= GTF_STMT_SKIP_LOWER;
+
+    assert(newStmt->gtStmtExpr->OperIsLocalStore());
+
+    // If the newly created statement is top-level then we need to manually lower its embedded 
+    // statements, the tree is lowered but some of its embedded statements are yet to be lowered.
+    if (newStmt->gtStmtIsTopLevel())
+    {
+        GenTree* curStmt = comp->compCurStmt;
+
+        for (GenTree* nextEmbeddedStmt = newStmt->gtStmtNextIfEmbedded();
+             nextEmbeddedStmt != nullptr;
+             nextEmbeddedStmt = nextEmbeddedStmt->gtStmt.gtStmtNextIfEmbedded())
+        {
+            // A previous call to CreateTemporary could have created embedded statements
+            // from the tree and those are already lowered.
+            if ((nextEmbeddedStmt->gtFlags & GTF_STMT_SKIP_LOWER) != 0)
+                continue;
+
+#ifdef DEBUG
+            if (comp->verbose)
+            {
+                printf("Lowering BB%02u, stmt id %u\n", currBlock->bbNum, nextEmbeddedStmt->gtTreeID);
+            }
+#endif
+            comp->compCurStmt = nextEmbeddedStmt;
+            comp->fgWalkTreePost(&nextEmbeddedStmt->gtStmt.gtStmtExpr, &Lowering::LowerNodeHelper, this, true);
+            nextEmbeddedStmt->gtFlags |= GTF_STMT_SKIP_LOWER;
+
+            // Lowering can remove the statement and set compCurStmt to another suitable statement.
+            // Currently only switch lowering does this and since embedded statements can't contain 
+            // a GT_SWITCH this case should never be hit here.
+            assert(comp->compCurStmt == nextEmbeddedStmt);
+        }
+
+        comp->compCurStmt = curStmt;
+    }
+
+    return newStmt;
+}
+
 // This is the main entry point for Lowering.  
 
 // In addition to that, LowerNode is also responsible for initializing the
@@ -3871,8 +3943,7 @@ void Lowering::LowerSignedDivOrMod(GenTreePtr* ppTree, Compiler::fgWalkData* dat
         {
             // We need to use the dividend node multiple times so its value needs to be
             // computed once and stored in a temp variable.
-            GenTreeStmt* newStmt = comp->fgInsertEmbeddedFormTemp(&(divMod->gtOp.gtOp1));
-            newStmt->gtFlags |= GTF_STMT_SKIP_LOWER;
+            CreateTemporary(&(divMod->gtOp.gtOp1));
             dividend = divMod->gtGetOp1();
 
             GenTreeStmt* curStmt = comp->compCurStmt->AsStmt();
@@ -4242,9 +4313,6 @@ void Lowering::DoPhase()
     comp->lvaPromoteLongVars();
 #endif // !defined(_TARGET_64BIT_)
 
-#ifdef DEBUG
-    unsigned stmtNum = 0;
-#endif
     for (BasicBlock* block = comp->fgFirstBB; block; block = block->bbNext)
     {
         GenTreePtr      stmt;
@@ -4264,21 +4332,16 @@ void Lowering::DoPhase()
                 continue;
             }
 #ifdef DEBUG
-            ++stmtNum;
             if (comp->verbose)
             {
-                // This is a useful location for a conditional breakpoint in Visual Studio (i.e. when stmtNum == 15)
-                printf("Decomposing BB%02u, stmt %u\n", block->bbNum, stmtNum);
+                printf("Decomposing BB%02u, stmt id %u\n", block->bbNum, stmt->gtTreeID);
             }
 #endif
             comp->compCurStmt = stmt;
             comp->fgWalkTreePost(&stmt->gtStmt.gtStmtExpr, &Lowering::DecompNodeHelper, this, true);
         }
 #endif //!_TARGET_64BIT_
 
-#ifdef DEBUG
-        stmtNum = 0;
-#endif
         // Walk the statement trees in this basic block 
         for (stmt = block->bbTreeList; stmt; stmt = stmt->gtNext)
         {
@@ -4287,11 +4350,9 @@ void Lowering::DoPhase()
                 continue;
             }
 #ifdef DEBUG
-            ++stmtNum;
-            if  (comp->verbose)
+            if (comp->verbose)
             {
-                // This is a useful location for a conditional breakpoint in Visual Studio (i.e. when stmtNum == 15)
-                printf("Lowering BB%02u, stmt %u\n", block->bbNum, stmtNum); 
+                printf("Lowering BB%02u, stmt id %u\n", block->bbNum, stmt->gtTreeID);
             }
 #endif
             comp->compCurStmt = stmt;

src/jit/lower.h

@@ -203,7 +203,7 @@ class Lowering : public Phase
     static bool NodesAreEquivalentLeaves       (GenTreePtr candidate, GenTreePtr storeInd);
 
     GenTreePtr  CreateLocalTempAsg      (GenTreePtr rhs, unsigned refCount, GenTreePtr *ppLclVar = nullptr);
-
+    GenTreeStmt* CreateTemporary        (GenTree** ppTree);
     bool AreSourcesPossiblyModified     (GenTree* use, GenTree* src1, GenTree *src2);
     void ReplaceNode                    (GenTree** ppTreeLocation,
                                          GenTree* replacementNode,