Fix for bug 2353 Assertion Failure 'nextStmtExpr->gtOper == GT_RETURN' in RetBufferBu...

The change fixes a bug in importing POP instruction where a copy between a tempvar and the return value of a tail call is misgenerated for Unix with enregisterable struct return values.

The change also fixes a bug of using GenTree::CopyFrom in fMorphInline where the dst node is a child of the src node.

diff --git a/src/jit/flowgraph.cpp b/src/jit/flowgraph.cpp
index bd1f559..12e3b7a 100644 (file)
--- a/src/jit/flowgraph.cpp
+++ b/src/jit/flowgraph.cpp
@@ -21804,7 +21804,7 @@ Compiler::fgWalkResult      Compiler::fgUpdateInlineReturnExpressionPlaceHolder(
             else
             {
                 // Just assign the inlinee to a variable to keep it simple.
-                tree->CopyFrom(comp->fgAssignStructInlineeToVar(tree, retClsHnd), comp);
+                tree = comp->fgAssignStructInlineeToVar(tree, retClsHnd);
             }
         }
 #endif // defined(_TARGET_ARM_) || defined(FEATURE_UNIX_AMD64_STRUCT_PASSING)

diff --git a/src/jit/importer.cpp b/src/jit/importer.cpp
index 6fd86a5..cffa81f 100644 (file)
--- a/src/jit/importer.cpp
+++ b/src/jit/importer.cpp
@@ -10779,6 +10779,10 @@ _CONV:
 
             if  ((op1->gtFlags & GTF_SIDE_EFFECT) || opts.compDbgCode)
             {
+#if FEATURE_MULTIREG_STRUCT_RET
+                bool isStructTailCall = varTypeIsStruct(op1) && (op1->gtOper == GT_CALL) 
+                                        && op1->gtCall.CanTailCall();                
+#endif
                 // Since we are throwing away the value, just normalize
                 // it to its address.  This is more efficient.
 
@@ -10805,7 +10809,15 @@ _CONV:
                     op1 = gtUnusedValNode(op1);
                 }
 
-                /* Append the value to the tree list */
+                // Append the value to the tree list
+#if FEATURE_MULTIREG_STRUCT_RET
+                // On Unix, a call with a structral return value will have either an assignment
+                // copying the return registers to stack or an address taken expression of the 
+                // hidden parameter inserted, depends on the size of the return value. We don't
+                // want to insert any more instructions after the call, since it will screw up
+                // the tail call morphing logic.
+                if (!isStructTailCall)
+#endif
                 goto SPILL_APPEND;
             }
             

diff --git a/src/jit/morph.cpp b/src/jit/morph.cpp
index db4a455..c12ba84 100644 (file)
--- a/src/jit/morph.cpp
+++ b/src/jit/morph.cpp
@@ -6669,17 +6669,10 @@ GenTreePtr          Compiler::fgMorphCall(GenTreeCall* call)
         }
         else if ((stmtExpr->gtOper == GT_ASG) && (fgMorphStmt->gtNext != nullptr))
         {
-            GenTreePtr nextStmtExpr = fgMorphStmt->gtNext->gtStmt.gtStmtExpr;
-            noway_assert(nextStmtExpr->gtOper == GT_RETURN);
-            // In this case we have an assignment of the result of the call, and then a return of the  result of the assignment.
-            // This can occur if impSpillStackEnsure() has introduced an assignment to a temp.
-            noway_assert(stmtExpr->gtGetOp1()->OperIsLocal() &&
-                         nextStmtExpr->OperGet() == GT_RETURN &&
-                         nextStmtExpr->gtGetOp1() != nullptr &&
-                         nextStmtExpr->gtGetOp1()->OperIsLocal() &&
-                         stmtExpr->gtGetOp1()->AsLclVarCommon()->gtLclNum == nextStmtExpr->gtGetOp1()->AsLclVarCommon()->gtLclNum);
+            noway_assert(fgMorphStmt->gtNext->gtStmt.gtStmtExpr->gtOper == GT_RETURN);
             deleteReturn = true;
         }
+
         if (deleteReturn)
         {
             fgRemoveStmt(compCurBB, fgMorphStmt->gtNext);