Skip to content

Commit c6af44d

Browse files
[clr-interp] Add logic to fix tail-call issues (#122618)
- Detect the case where tail calls are not immediately followed by ret, and fail the tail-call. - Detect the attempted usage of tail-calls from within synchronized methods or from within reverse p/invoke methods - Use the jit interface canTailCall api when a tail-call is requested - Enable tail-calls for calli instructions Update the TailcallVerifyWithPrefix test to skip tests which depend on the jit performing implicit tail-calls when optimizing when running under the interpreter This fixes the TailcallVerifyWithPrefix and more_tailcalls test
1 parent 25fc25e commit c6af44d

File tree

4 files changed

+50
-3
lines changed

4 files changed

+50
-3
lines changed

src/coreclr/interpreter/compiler.cpp

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4004,7 +4004,7 @@ void InterpCompiler::EmitPushLdvirtftn(int thisVar, CORINFO_RESOLVED_TOKEN* pRes
40044004
m_pLastNewIns->info.pCallInfo->pCallArgs = callArgs;
40054005
}
40064006

4007-
static bool DisallowTailCall(CORINFO_SIG_INFO* callerSig, CORINFO_SIG_INFO* calleeSig)
4007+
bool InterpCompiler::DisallowTailCall(CORINFO_SIG_INFO* callerSig, CORINFO_SIG_INFO* calleeSig)
40084008
{
40094009
// We allow only the return value types to differ between caller and callee as long as their stack types are the same.
40104010
// In principle we could allow more differences (e.g. I8 coercion to I4, or O to I) but for now we keep it simple.
@@ -4024,6 +4024,16 @@ static bool DisallowTailCall(CORINFO_SIG_INFO* callerSig, CORINFO_SIG_INFO* call
40244024
// Disallow tail calls from async methods for now
40254025
return true;
40264026
}
4027+
if (m_isSynchronized)
4028+
{
4029+
// Disallow tail calls from synchronized methods
4030+
return true;
4031+
}
4032+
if (m_corJitFlags.IsSet(CORJIT_FLAGS::CORJIT_FLAG_REVERSE_PINVOKE))
4033+
{
4034+
// Disallow tail calls from reverse pinvoke methods
4035+
return true;
4036+
}
40274037
return false;
40284038
}
40294039

@@ -4488,7 +4498,14 @@ void InterpCompiler::EmitCall(CORINFO_RESOLVED_TOKEN* pConstrainedToken, bool re
44884498
#endif
44894499
}
44904500

4491-
if (tailcall && DisallowTailCall(&m_methodInfo->args, &callInfo.sig))
4501+
if (tailcall && (
4502+
DisallowTailCall(&m_methodInfo->args, &callInfo.sig) // Disallow tail-calls for code gen reasons
4503+
|| !m_compHnd->canTailCall(m_methodHnd, // Disallow tail calls due to rules specified by the VM
4504+
isCalli ? (CORINFO_METHOD_HANDLE)NULL : callInfo.hMethod, // The method we are attempting to call logically
4505+
isCalli ? (CORINFO_METHOD_HANDLE)NULL : (callInfo.kind == CORINFO_CALL ? callInfo.hMethod : (CORINFO_METHOD_HANDLE)NULL),
4506+
true) // The method we are calling exactly. We only know this if it's a non-virtual call
4507+
|| (!isJmp && *m_ip != CEE_RET) // Disallow tailcalls that are not immediately before a ret
4508+
))
44924509
{
44934510
if (isJmp)
44944511
{
@@ -9043,7 +9060,7 @@ void InterpCompiler::GenerateCode(CORINFO_METHOD_INFO* methodInfo)
90439060
tailcall = false;
90449061
break;
90459062
case CEE_CALLI:
9046-
EmitCall(NULL /*pConstrainedToken*/, false /* readonly*/, false /* tailcall*/, false /*newObj*/, true /*isCalli*/);
9063+
EmitCall(NULL /*pConstrainedToken*/, false /* readonly*/, tailcall /* tailcall*/, false /*newObj*/, true /*isCalli*/);
90479064
pConstrainedToken = NULL;
90489065
readonly = false;
90499066
tailcall = false;

src/coreclr/interpreter/compiler.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -872,6 +872,7 @@ class InterpCompiler
872872
void PushInterpType(InterpType interpType, CORINFO_CLASS_HANDLE clsHnd);
873873
void PushTypeVT(CORINFO_CLASS_HANDLE clsHnd, int size);
874874
void ConvertFloatingPointStackEntryToStackType(StackInfo* entry, StackType type);
875+
bool DisallowTailCall(CORINFO_SIG_INFO* callerSig, CORINFO_SIG_INFO* calleeSig);
875876

876877
// Opcode peeps
877878
bool FindAndApplyPeep(OpcodePeep* Peeps[]);

src/tests/JIT/opt/Tailcall/TailcallVerifyWithPrefix.il

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,10 @@
2828
.publickeytoken = (B0 3F 5F 7F 11 D5 0A 3A )
2929
.ver 4:0:0:0
3030
}
31+
.assembly extern TestLibrary
32+
{
33+
.ver 0:0:0:0
34+
}
3135
.assembly TailcallVerifyWithPrefix
3236
{
3337
.custom instance void [mscorlib]System.Reflection.AssemblyDescriptionAttribute::.ctor(string) = ( 01 00 00 00 00 )
@@ -3341,6 +3345,14 @@
33413345
+ "one - ReturnType: void; Callee<string>: Arguments: None - ReturnType: v"
33423346
+ "oid"
33433347
IL_0005: call void [System.Console]System.Console::WriteLine(string)
3348+
// This test assumes that regular calls will be converted to tail-calls when possible
3349+
call bool [TestLibrary]TestLibrary.Utilities::get_IsCoreClrInterpreter()
3350+
ldc.i4.0
3351+
beq IL_000a
3352+
ldstr "Skipping test on interpreter, as it doesn't support converting regular calls into tail-calls"
3353+
call void [System.Console]System.Console::WriteLine(string)
3354+
ldc.i4.s 100
3355+
ret
33443356
IL_000a: ldc.i4.s 100
33453357
IL_000c: stsfld int32 TailcallVerify.Condition10::Result
33463358
.try
@@ -3457,6 +3469,14 @@
34573469
+ "alueType3Bytes>: Caller: Arguments: None - ReturnType: void; Callee: Ar"
34583470
+ "guments: None - ReturnType: void"
34593471
IL_0023: call void [System.Console]System.Console::WriteLine(string)
3472+
// This test assumes that regular calls will be converted to tail-calls when possible
3473+
call bool [TestLibrary]TestLibrary.Utilities::get_IsCoreClrInterpreter()
3474+
ldc.i4.0
3475+
beq IL_0028
3476+
ldstr "Skipping test on interpreter, as it doesn't support converting regular calls into tail-calls"
3477+
call void [System.Console]System.Console::WriteLine(string)
3478+
ldc.i4.s 100
3479+
ret
34603480
IL_0028: ldc.i4.s 100
34613481
IL_002a: stsfld int32 TailcallVerify.Condition10::Result
34623482
.try
@@ -3536,6 +3556,14 @@
35363556
+ "alueType3Bytes>: Caller: Arguments: string, ValueType3Bytes - ReturnTyp"
35373557
+ "e: void; Callee: Arguments: string - ReturnType: void"
35383558
IL_0023: call void [System.Console]System.Console::WriteLine(string)
3559+
// This test assumes that regular calls will be converted to tail-calls when possible
3560+
call bool [TestLibrary]TestLibrary.Utilities::get_IsCoreClrInterpreter()
3561+
ldc.i4.0
3562+
beq IL_0028
3563+
ldstr "Skipping test on interpreter, as it doesn't support converting regular calls into tail-calls"
3564+
call void [System.Console]System.Console::WriteLine(string)
3565+
ldc.i4.s 100
3566+
ret
35393567
IL_0028: ldc.i4.s 100
35403568
IL_002a: stsfld int32 TailcallVerify.Condition10::Result
35413569
.try

src/tests/JIT/opt/Tailcall/TailcallVerifyWithPrefix.ilproj

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,5 +15,6 @@
1515
<Compile Include="TailcallVerifyWithPrefix.il" />
1616
<Compile Include="TailcallVerifyTransparentLibraryWithPrefix.il" />
1717
<Compile Include="TailcallVerifyVerifiableLibraryWithPrefix.il" />
18+
<ProjectReference Include="$(TestLibraryProjectPath)" />
1819
</ItemGroup>
1920
</Project>

0 commit comments

Comments
 (0)