Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disallow widening for explicit tailcalls #55989

Merged
merged 3 commits into from
Jul 21, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -4603,7 +4603,8 @@ class Compiler
bool exactContextNeedsRuntimeLookup,
CORINFO_CALL_INFO* callInfo);

bool impTailCallRetTypeCompatible(var_types callerRetType,
bool impTailCallRetTypeCompatible(bool allowWidening,
var_types callerRetType,
CORINFO_CLASS_HANDLE callerRetTypeClass,
CorInfoCallConvExtension callerCallConv,
var_types calleeRetType,
Expand Down
46 changes: 34 additions & 12 deletions src/coreclr/jit/importer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7863,11 +7863,32 @@ void Compiler::impInsertHelperCall(CORINFO_HELPER_DESC* helperInfo)
impAppendTree(callout, (unsigned)CHECK_SPILL_NONE, impCurStmtOffs);
}

// Checks whether the return types of caller and callee are compatible
// so that callee can be tail called. Note that here we don't check
// compatibility in IL Verifier sense, but on the lines of return type
// sizes are equal and get returned in the same return register.
bool Compiler::impTailCallRetTypeCompatible(var_types callerRetType,
//------------------------------------------------------------------------
// impTailCallRetTypeCompatible: Checks whether the return types of caller
// and callee are compatible so that calle can be tail called.
// sizes are not supported integral type sizes return values to temps.
//
// Arguments:
// allowWidening -- whether to allow implicit widening by the callee.
// For instance, allowing int32 -> int16 tailcalls.
// The managed calling convention allows this, but
// we don't want explicit tailcalls to depend on this
// detail of the managed calling convention.
// callerRetType -- the caller's return type
// callerRetTypeClass - the caller's return struct type
// callerCallConv -- calling convention of the caller
// calleeRetType -- the callee's return type
// calleeRetTypeClass - the callee return struct type
// calleeCallConv -- calling convention of the callee
//
// Returns:
// True if the tailcall types are compatible.
//
// Remarks:
// Note that here we don't check compatibility in IL Verifier sense, but on the
// lines of return types getting returned in the same return register.
bool Compiler::impTailCallRetTypeCompatible(bool allowWidening,
var_types callerRetType,
CORINFO_CLASS_HANDLE callerRetTypeClass,
CorInfoCallConvExtension callerCallConv,
var_types calleeRetType,
Expand All @@ -7886,7 +7907,7 @@ bool Compiler::impTailCallRetTypeCompatible(var_types callerRetTy
bool isManaged =
(callerCallConv == CorInfoCallConvExtension::Managed) && (calleeCallConv == CorInfoCallConvExtension::Managed);

if (isManaged && varTypeIsIntegral(callerRetType) && varTypeIsIntegral(calleeRetType) &&
if (allowWidening && isManaged && varTypeIsIntegral(callerRetType) && varTypeIsIntegral(calleeRetType) &&
(genTypeSize(callerRetType) <= 4) && (genTypeSize(calleeRetType) <= genTypeSize(callerRetType)))
{
return true;
Expand Down Expand Up @@ -9096,13 +9117,14 @@ var_types Compiler::impImportCall(OPCODE opcode,
BADCODE("Stack should be empty after tailcall");
}

// Note that we can not relax this condition with genActualType() as
// the calling convention dictates that the caller of a function with
// a small-typed return value is responsible for normalizing the return val

// For opportunistic tailcalls we allow implicit widening, i.e. tailcalls from int32 -> int16, since the
// managed calling convention dictates that the callee widens the value. For explicit tailcalls we don't
// want to require this detail of the calling convention to bubble up to the tailcall helpers
bool allowWidening = isImplicitTailCall;
if (canTailCall &&
!impTailCallRetTypeCompatible(info.compRetType, info.compMethodInfo->args.retTypeClass, info.compCallConv,
callRetTyp, sig->retTypeClass, call->AsCall()->GetUnmanagedCallConv()))
!impTailCallRetTypeCompatible(allowWidening, info.compRetType, info.compMethodInfo->args.retTypeClass,
info.compCallConv, callRetTyp, sig->retTypeClass,
call->AsCall()->GetUnmanagedCallConv()))
{
canTailCall = false;
szCanTailCallFailReason = "Return types are not tail call compatible";
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/morph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6872,7 +6872,7 @@ bool Compiler::fgCanFastTailCall(GenTreeCall* callee, const char** failReason)
if (callee->IsTailPrefixedCall())
{
var_types retType = info.compRetType;
assert(impTailCallRetTypeCompatible(retType, info.compMethodInfo->args.retTypeClass, info.compCallConv,
assert(impTailCallRetTypeCompatible(false, retType, info.compMethodInfo->args.retTypeClass, info.compCallConv,
(var_types)callee->gtReturnType, callee->gtRetClsHnd,
callee->GetUnmanagedCallConv()));
}
Expand Down
21 changes: 21 additions & 0 deletions src/tests/JIT/Regression/JitBlue/Runtime_55253/Runtime_55253.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
class Runtime_55253
jakobbotsch marked this conversation as resolved.
Show resolved Hide resolved
{
static int Main()
{
int errors = 0;
if (AsInt32() != -1)
errors |= 1;
if (AsUInt32() != 255)
errors |= 2;

return 100 + errors;
}

static uint AsUInt32() => AsUInt16();
static uint AsUInt16() => AsUInt8();
static uint AsUInt8() => 255;

static int AsInt32() => AsInt16();
static short AsInt16() => AsInt8();
static sbyte AsInt8() => -1;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
</PropertyGroup>
<PropertyGroup>
<DebugType>None</DebugType>
<Optimize>False</Optimize>
</PropertyGroup>
<ItemGroup>
<Compile Include="$(MSBuildProjectName).cs" />
</ItemGroup>
<PropertyGroup>
<CLRTestBatchPreCommands><![CDATA[
$(CLRTestBatchPreCommands)
set DOTNET_TailcallStress=1
]]></CLRTestBatchPreCommands>
<BashCLRTestPreCommands><![CDATA[
$(BashCLRTestPreCommands)
export DOTNET_TailcallStress=1
]]></BashCLRTestPreCommands>
</PropertyGroup>
</Project>