Skip to content

Commit

Permalink
Jitstress fixes for tailcall tests (#1771)
Browse files Browse the repository at this point in the history
Make STRESS_GENERIC_VARN more compatible with methods that make explicit tail
calls. Don't add gc checks for explicit tail calls, and remove the code in
morph that blocks tail calls if gc checks are active.

Update the tailcall test to to disable STRESS_UNSAFE_BUFFER_CHECKS. This can
be reverted when #341 is merged.

Fixes #1752.
  • Loading branch information
AndyAyersMS authored Jan 15, 2020
1 parent 5182dad commit e17f25c
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 18 deletions.
2 changes: 1 addition & 1 deletion src/coreclr/src/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -4021,7 +4021,7 @@ class Compiler
}
void impLoadArg(unsigned ilArgNum, IL_OFFSET offset);
void impLoadLoc(unsigned ilLclNum, IL_OFFSET offset);
bool impReturnInstruction(BasicBlock* block, int prefixFlags, OPCODE& opcode);
bool impReturnInstruction(int prefixFlags, OPCODE& opcode);

#ifdef _TARGET_ARM_
void impMarkLclDstNotPromotable(unsigned tmpNum, GenTree* op, CORINFO_CLASS_HANDLE hClass);
Expand Down
25 changes: 17 additions & 8 deletions src/coreclr/src/jit/importer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11508,7 +11508,7 @@ void Compiler::impImportBlockCode(BasicBlock* block)
case CEE_RET:
prefixFlags &= ~PREFIX_TAILCALL; // ret without call before it
RET:
if (!impReturnInstruction(block, prefixFlags, opcode))
if (!impReturnInstruction(prefixFlags, opcode))
{
return; // abort
}
Expand Down Expand Up @@ -16254,7 +16254,7 @@ GenTree* Compiler::impAssignSmallStructTypeToVar(GenTree* op, CORINFO_CLASS_HAND
// registers return values to suitable temps.
//
// Arguments:
// op -- call returning a struct in a registers
// op -- call returning a struct in registers
// hClass -- class handle for struct
//
// Returns:
Expand All @@ -16278,11 +16278,20 @@ GenTree* Compiler::impAssignMultiRegTypeToVar(GenTree* op, CORINFO_CLASS_HANDLE
}
#endif // FEATURE_MULTIREG_RET

// do import for a return
// returns false if inlining was aborted
// opcode can be ret or call in the case of a tail.call
bool Compiler::impReturnInstruction(BasicBlock* block, int prefixFlags, OPCODE& opcode)
//------------------------------------------------------------------------
// impReturnInstruction: import a return or an explicit tail call
//
// Arguments:
// prefixFlags -- active IL prefixes
// opcode -- [in, out] IL opcode
//
// Returns:
// True if import was successful (may fail for some inlinees)
//
bool Compiler::impReturnInstruction(int prefixFlags, OPCODE& opcode)
{
const bool isTailCall = (prefixFlags & PREFIX_TAILCALL) != 0;

if (tiVerificationNeeded)
{
verVerifyThisPtrInitialised();
Expand Down Expand Up @@ -16334,7 +16343,7 @@ bool Compiler::impReturnInstruction(BasicBlock* block, int prefixFlags, OPCODE&
(varTypeIsStruct(op2) && varTypeIsStruct(info.compRetType)));

#ifdef DEBUG
if (opts.compGcChecks && info.compRetType == TYP_REF)
if (!isTailCall && opts.compGcChecks && (info.compRetType == TYP_REF))
{
// DDB 3483 : JIT Stress: early termination of GC ref's life time in exception code path
// VSW 440513: Incorrect gcinfo on the return value under COMPlus_JitGCChecks=1 for methods with
Expand Down Expand Up @@ -16741,7 +16750,7 @@ bool Compiler::impReturnInstruction(BasicBlock* block, int prefixFlags, OPCODE&
}

// We must have imported a tailcall and jumped to RET
if (prefixFlags & PREFIX_TAILCALL)
if (isTailCall)
{
#if defined(FEATURE_CORECLR) || !defined(_TARGET_AMD64_)
// Jit64 compat:
Expand Down
9 changes: 0 additions & 9 deletions src/coreclr/src/jit/morph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6975,15 +6975,6 @@ GenTree* Compiler::fgMorphPotentialTailCall(GenTreeCall* call)
}
#endif

#ifdef DEBUG
// DDB 99324: Just disable tailcall under compGcChecks stress mode.
if (opts.compGcChecks)
{
failTailCall("GcChecks");
return nullptr;
}
#endif

// We have to ensure to pass the incoming retValBuf as the
// outgoing one. Using a temp will not do as this function will
// not regain control to do the copy. This can happen when inlining
Expand Down
10 changes: 10 additions & 0 deletions src/coreclr/tests/src/JIT/Directed/tailcall/tailcall.ilproj
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,14 @@
<ItemGroup>
<Compile Include="$(MSBuildProjectName).il" />
</ItemGroup>
<PropertyGroup>
<CLRTestBatchPreCommands><![CDATA[
$(CLRTestBatchPreCommands)
set COMPlus_JitStressModeNamesNot=STRESS_UNSAFE_BUFFER_CHECKS
]]></CLRTestBatchPreCommands>
<BashCLRTestPreCommands><![CDATA[
$(BashCLRTestPreCommands)
export COMPlus_JitStressModeNamesNot=STRESS_UNSAFE_BUFFER_CHECKS
]]></BashCLRTestPreCommands>
</PropertyGroup>
</Project>

0 comments on commit e17f25c

Please sign in to comment.