Skip to content

Commit

Permalink
Delete move to return spill in the implicit tail call in the inlinee …
Browse files Browse the repository at this point in the history
…with several returns. (dotnet#14945)

* add fgNeedReturnSpillTemp
* fix the issue
* add the repro without stress mode
  • Loading branch information
Sergey Andreenko authored Nov 15, 2017
1 parent 97d9d59 commit 431a675
Show file tree
Hide file tree
Showing 6 changed files with 298 additions and 113 deletions.
6 changes: 5 additions & 1 deletion src/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -2500,7 +2500,8 @@ class Compiler
// we will redirect all "ldarg(a) 0" and "starg 0" to this temp.

unsigned lvaInlineeReturnSpillTemp; // The temp to spill the non-VOID return expression
// in case there are multiple BBJ_RETURN blocks in the inlinee.
// in case there are multiple BBJ_RETURN blocks in the inlinee
// or if the inlinee has GC ref locals.

#if FEATURE_FIXED_OUT_ARGS
unsigned lvaOutgoingArgSpaceVar; // dummy TYP_LCLBLK var for fixed outgoing argument space
Expand Down Expand Up @@ -4832,6 +4833,7 @@ class Compiler
private:
GenTreePtr fgMorphField(GenTreePtr tree, MorphAddrContext* mac);
bool fgCanFastTailCall(GenTreeCall* call);
bool fgCheckStmtAfterTailCall();
void fgMorphTailCall(GenTreeCall* call);
void fgMorphRecursiveFastTailCallIntoLoop(BasicBlock* block, GenTreeCall* recursiveTailCall);
GenTreePtr fgAssignRecursiveCallArgToCallerParam(GenTreePtr arg,
Expand Down Expand Up @@ -5033,6 +5035,8 @@ class Compiler
bool fgIsSignedModOptimizable(GenTreePtr divisor);
bool fgIsUnsignedModOptimizable(GenTreePtr divisor);

bool fgNeedReturnSpillTemp();

/*
XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
Expand Down
12 changes: 12 additions & 0 deletions src/jit/flowgraph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25872,3 +25872,15 @@ void Compiler::fgCompDominatedByExceptionalEntryBlocks()
}
}
}

//------------------------------------------------------------------------
// fgNeedReturnSpillTemp: Answers does the inlinee need to spill all returns
// as a temp.
//
// Return Value:
// true if the inlinee has to spill return exprs.
bool Compiler::fgNeedReturnSpillTemp()
{
assert(compIsForInlining());
return (lvaInlineeReturnSpillTemp != BAD_VAR_NUM);
}
14 changes: 7 additions & 7 deletions src/jit/importer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15768,7 +15768,7 @@ bool Compiler::impReturnInstruction(BasicBlock* block, int prefixFlags, OPCODE&

// Below, we are going to set impInlineInfo->retExpr to the tree with the return
// expression. At this point, retExpr could already be set if there are multiple
// return blocks (meaning lvaInlineeReturnSpillTemp != BAD_VAR_NUM) and one of
// return blocks (meaning fgNeedReturnSpillTemp() == true) and one of
// the other blocks already set it. If there is only a single return block,
// retExpr shouldn't be set. However, this is not true if we reimport a block
// with a return. In that case, retExpr will be set, then the block will be
Expand Down Expand Up @@ -15800,7 +15800,7 @@ bool Compiler::impReturnInstruction(BasicBlock* block, int prefixFlags, OPCODE&
}
}

if (lvaInlineeReturnSpillTemp != BAD_VAR_NUM)
if (fgNeedReturnSpillTemp())
{
assert(info.compRetNativeType != TYP_VOID &&
(fgMoreThanOneReturnBlock() || impInlineInfo->HasGcRefLocals()));
Expand All @@ -15809,7 +15809,7 @@ bool Compiler::impReturnInstruction(BasicBlock* block, int prefixFlags, OPCODE&
// If we are inlining a call that returns a struct, where the actual "native" return type is
// not a struct (for example, the struct is composed of exactly one int, and the native
// return type is thus an int), and the inlinee has multiple return blocks (thus,
// lvaInlineeReturnSpillTemp is != BAD_VAR_NUM, and is the index of a local var that is set
// fgNeedReturnSpillTemp() == true, and is the index of a local var that is set
// to the *native* return type), and at least one of the return blocks is the result of
// a call, then we have a problem. The situation is like this (from a failed test case):
//
Expand Down Expand Up @@ -15922,7 +15922,7 @@ bool Compiler::impReturnInstruction(BasicBlock* block, int prefixFlags, OPCODE&
CLANG_FORMAT_COMMENT_ANCHOR;
#endif // defined(FEATURE_UNIX_AMD64_STRUCT_PASSING)

if (lvaInlineeReturnSpillTemp != BAD_VAR_NUM)
if (fgNeedReturnSpillTemp())
{
if (!impInlineInfo->retExpr)
{
Expand Down Expand Up @@ -15950,7 +15950,7 @@ bool Compiler::impReturnInstruction(BasicBlock* block, int prefixFlags, OPCODE&
{
assert(!iciCall->HasRetBufArg());
assert(retRegCount >= 2);
if (lvaInlineeReturnSpillTemp != BAD_VAR_NUM)
if (fgNeedReturnSpillTemp())
{
if (!impInlineInfo->retExpr)
{
Expand All @@ -15970,7 +15970,7 @@ bool Compiler::impReturnInstruction(BasicBlock* block, int prefixFlags, OPCODE&
assert(iciCall->HasRetBufArg());
GenTreePtr dest = gtCloneExpr(iciCall->gtCallArgs->gtOp.gtOp1);
// spill temp only exists if there are multiple return points
if (lvaInlineeReturnSpillTemp != BAD_VAR_NUM)
if (fgNeedReturnSpillTemp())
{
// if this is the first return we have seen set the retExpr
if (!impInlineInfo->retExpr)
Expand Down Expand Up @@ -18519,7 +18519,7 @@ unsigned Compiler::impInlineFetchLocal(unsigned lclNum DEBUGARG(const char* reas
// Since there are gc locals we should have seen them earlier
// and if there was a return value, set up the spill temp.
assert(impInlineInfo->HasGcRefLocals());
assert((info.compRetNativeType == TYP_VOID) || (lvaInlineeReturnSpillTemp != BAD_VAR_NUM));
assert((info.compRetNativeType == TYP_VOID) || fgNeedReturnSpillTemp());
}
else
{
Expand Down
Loading

0 comments on commit 431a675

Please sign in to comment.