Skip to content

Commit

Permalink
Fix slow tailcalls to VSD (dotnet/coreclr#27363)
Browse files Browse the repository at this point in the history
* Fix slow tailcalls to VSDs

This was broken when fgCanFastTailCall was changed to call
fgInitArgInfo. fgInitArgInfo has side effects and will in some cases add
arguments to the arg list. Specifically for calls to VSD, the VSD arg is
added, however this case is treated specially for slow tailcalls and it
does not expect the arg to be here.

This targeted fix just removes this arg from the arg list.

* Reenable more_tailcalls on x64 Windows

* Disable more_tailcalls on Unix, arm32 and arm64

* Address feedback

* Disable STRESS_UNSAFE_BUFFER_CHECKS for more_tailcalls


Commit migrated from dotnet/coreclr@f4a8863
  • Loading branch information
jakobbotsch authored and Jarret Shook committed Oct 31, 2019
1 parent 492b317 commit f119bd9
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 7 deletions.
36 changes: 36 additions & 0 deletions src/coreclr/src/jit/morph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7412,6 +7412,42 @@ void Compiler::fgMorphTailCallViaHelper(GenTreeCall* call, void* pfnCopyArgs)
if (call->IsVirtualStub())
{
call->gtFlags |= GTF_CALL_NULLCHECK;

#if defined(_TARGET_AMD64_)
// If we already inited arg info here then we will have added the VSD
// arg on AMD64. So we remove it here as we will handle this case
// specially below.
fgArgInfo* argInfo = call->fgArgInfo;
assert(argInfo != nullptr);

GenTreeCall::Use* vsdArg = nullptr;

for (unsigned index = 0; index < argInfo->ArgCount(); ++index)
{
fgArgTabEntry* arg = argInfo->GetArgEntry(index, false);
if (arg->isNonStandard)
{
// The only supported nonstandard arg for slow tailcalls is
// VSD arg.
assert(vsdArg == nullptr);
vsdArg = arg->use;
#ifndef DEBUG
break;
#endif
}
}

assert(vsdArg != nullptr);
// Find the arg in the linked list keeping track of the pointer to it so
// we can unlink it.
GenTreeCall::Use** ptr = &call->gtCallArgs;
while ((*ptr) != vsdArg)
{
ptr = &(*ptr)->NextRef();
}

*ptr = vsdArg->GetNext();
#endif
}

#if defined(_TARGET_ARM_)
Expand Down
17 changes: 10 additions & 7 deletions src/coreclr/tests/issues.targets
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,9 @@
<ExcludeList Include="$(XunitTestBinBase)/JIT/Methodical/tailcall_v4/hijacking/*">
<Issue>Unix does not support tailcall helper</Issue>
</ExcludeList>
<ExcludeList Include="$(XunitTestBinBase)/JIT/Directed/tailcall/more_tailcalls/*">
<Issue>Unix does not support tailcall helper</Issue>
</ExcludeList>
</ItemGroup>

<!-- Arm32 All OS -->
Expand Down Expand Up @@ -233,7 +236,10 @@
<Issue>26105</Issue>
</ExcludeList>
<ExcludeList Include="$(XunitTestBinBase)/JIT/Methodical/tailcall_v4/hijacking/*">
<Issue>arm32 does not support tailcall helper</Issue>
<Issue>Unix arm32 does not support tailcall helper</Issue>
</ExcludeList>
<ExcludeList Include="$(XunitTestBinBase)/JIT/Directed/tailcall/more_tailcalls/*">
<Issue>Unix arm32 does not support tailcall helper</Issue>
</ExcludeList>
</ItemGroup>

Expand Down Expand Up @@ -281,6 +287,9 @@
<ExcludeList Include="$(XunitTestBinBase)/JIT/Methodical/tailcall_v4/hijacking/*">
<Issue>arm64 does not support tailcall helper</Issue>
</ExcludeList>
<ExcludeList Include="$(XunitTestBinBase)/JIT/Directed/tailcall/more_tailcalls/*">
<Issue>arm64 does not support tailcall helper</Issue>
</ExcludeList>
<ExcludeList Include="$(XunitTestBinBase)/JIT/Regression/JitBlue/GitHub_26491/**/*">
<Issue>26491</Issue>
</ExcludeList>
Expand Down Expand Up @@ -897,12 +906,6 @@
</ExcludeList>
</ItemGroup>

<!-- Tests that run only on x86 Windows -->
<ItemGroup Condition="'$(XunitTestBinBase)' != '' and ('$(BuildArch)' != 'x86' or '$(TargetsWindows)' != 'true')">
<ExcludeList Include="$(XunitTestBinBase)/JIT/Directed/tailcall/more_tailcalls/*">
<Issue>x64 Windows has bugs with helper-based tailcall to VSD (#26311); rest of platforms do not support it</Issue>
</ExcludeList>
</ItemGroup>
<!-- runtest.proj finds all the *.cmd/*.sh scripts in a test folder and creates corresponding test methods.
Exclude these scripts to avoid creating such methods for the superpmicollect dependent test projects
and running them separately from superpmicollect test. -->
Expand Down
10 changes: 10 additions & 0 deletions src/coreclr/tests/src/JIT/Directed/tailcall/more_tailcalls.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 f119bd9

Please sign in to comment.