-
Notifications
You must be signed in to change notification settings - Fork 0
[*64] Allow more fastTailCalls involving structs #3
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -239,9 +239,11 @@ void Compiler::lvaInitTypeRef() | |
// Finally the local variables | ||
//------------------------------------------------------------------------- | ||
|
||
unsigned varNum = varDscInfo.varNum; | ||
LclVarDsc* varDsc = varDscInfo.varDsc; | ||
CORINFO_ARG_LIST_HANDLE localsSig = info.compMethodInfo->locals.args; | ||
unsigned argRegNum = varDscInfo.intRegArgNum; | ||
unsigned otherArgRegNum = varDscInfo.floatRegArgNum; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is confusing at best, and needs a comment. I gather that this handles the case where there may be a mix of int and float regs, so that this is the first of each that is used for this arg? |
||
unsigned varNum = varDscInfo.varNum; | ||
LclVarDsc* varDsc = varDscInfo.varDsc; | ||
CORINFO_ARG_LIST_HANDLE localsSig = info.compMethodInfo->locals.args; | ||
|
||
for (unsigned i = 0; i < info.compMethodInfo->locals.numArgs; | ||
i++, varNum++, varDsc++, localsSig = info.compCompHnd->getArgNext(localsSig)) | ||
|
@@ -262,6 +264,9 @@ void Compiler::lvaInitTypeRef() | |
} | ||
} | ||
|
||
info.compArgRegCount = argRegNum; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't make sense to me. Why are you assigning a register number to a count? |
||
info.compOtherArgRegCount = otherArgRegNum; | ||
|
||
if ( // If there already exist unsafe buffers, don't mark more structs as unsafe | ||
// as that will cause them to be placed along with the real unsafe buffers, | ||
// unnecessarily exposing them to overruns. This can affect GS tests which | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,81 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
// See the LICENSE file in the project root for more information. | ||
|
||
using System; | ||
|
||
// 10 byte struct | ||
public struct A | ||
{ | ||
public int a; | ||
public int b; | ||
public short c; | ||
} | ||
|
||
// 32 byte struct | ||
public struct B | ||
{ | ||
public long a; | ||
public long b; | ||
public long c; | ||
public long d; | ||
} | ||
|
||
public class DoNotFastTailCallWithStructs | ||
{ | ||
static A a; | ||
static B b; | ||
|
||
public static void Foo(B b, int count=1000) | ||
{ | ||
if (count == 100) | ||
{ | ||
return count; | ||
} | ||
|
||
if (count-- % 2 == 0) | ||
{ | ||
return Foo(a, count); | ||
} | ||
|
||
else | ||
{ | ||
return Foo(b, count); | ||
} | ||
} | ||
|
||
public static void Foo(A a, int count=1000) | ||
{ | ||
if (count == 100) | ||
{ | ||
return count; | ||
} | ||
|
||
if (count-- % 2 == 0) | ||
{ | ||
return Foo(a, count); | ||
} | ||
|
||
else | ||
{ | ||
return Foo(b, count); | ||
} | ||
} | ||
|
||
public static int Main() | ||
{ | ||
a = new A(); | ||
b = new B(); | ||
|
||
a.a = 100; | ||
a.b = 1000; | ||
a.c = 10000; | ||
|
||
b.a = 500; | ||
b.b = 5000; | ||
b.c = 50000; | ||
b.d = 500000; | ||
|
||
return Foo(a); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
<?xml version="1.0" encoding="utf-8"?> | ||
<Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003"> | ||
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" /> | ||
<PropertyGroup> | ||
<Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration> | ||
<Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform> | ||
<AssemblyName>$(MSBuildProjectName)</AssemblyName> | ||
<SchemaVersion>2.0</SchemaVersion> | ||
<ProjectGuid>{95DFC527-4DC1-495E-97D7-E94EE1F7140D}</ProjectGuid> | ||
<OutputType>Exe</OutputType> | ||
<ProjectTypeGuids>{786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids> | ||
<SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\..\</SolutionDir> | ||
<IlasmRoundTrip>true</IlasmRoundTrip> | ||
</PropertyGroup> | ||
<!-- Default configurations to help VS understand the configurations --> | ||
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' "> | ||
</PropertyGroup> | ||
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' "> | ||
</PropertyGroup> | ||
<ItemGroup> | ||
<CodeAnalysisDependentAssemblyPaths Condition=" '$(VS100COMNTOOLS)' != '' " Include="$(VS100COMNTOOLS)..\IDE\PrivateAssemblies"> | ||
<Visible>False</Visible> | ||
</CodeAnalysisDependentAssemblyPaths> | ||
</ItemGroup> | ||
<PropertyGroup> | ||
<!-- Set to 'Full' if the Debug? column is marked in the spreadsheet. Leave blank otherwise. --> | ||
<DebugType>None</DebugType> | ||
<NoLogo>True</NoLogo> | ||
<NoStandardLib>True</NoStandardLib> | ||
<Noconfig>True</Noconfig> | ||
<Optimize>True</Optimize> | ||
<JitOptimizationSensitive>True</JitOptimizationSensitive> | ||
<AllowUnsafeBlocks>True</AllowUnsafeBlocks> | ||
<DefineConstants>$(DefineConstants);CORECLR</DefineConstants> | ||
</PropertyGroup> | ||
<ItemGroup> | ||
<Compile Include="DoNotFastTailCallWithStructs.cs" /> | ||
</ItemGroup> | ||
<ItemGroup> | ||
<Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" /> | ||
</ItemGroup> | ||
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" /> | ||
<PropertyGroup Condition=" '$(MsBuildProjectDirOverride)' != '' "> | ||
</PropertyGroup> | ||
</Project> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
// See the LICENSE file in the project root for more information. | ||
using System; | ||
|
||
// 10 byte struct | ||
public struct A | ||
{ | ||
public int a; | ||
public int b; | ||
public short c; | ||
} | ||
|
||
class TailCallStructPassingSimple | ||
{ | ||
// Simple tail call candidate that would be ignored on Arm64 and amd64 Unix | ||
// due to https://github.com/dotnet/coreclr/issues/2666 | ||
public static int ImplicitTailCallTenByteStruct(A a, int count=1000) | ||
{ | ||
if (count-- == 0) | ||
{ | ||
return 100; | ||
} | ||
|
||
return ImplicitTailCallTenByteStruct(a, count); | ||
} | ||
|
||
public static int Main() | ||
{ | ||
A temp = new A(); | ||
temp.a = 50; | ||
temp.b = 500; | ||
temp.c = 62; | ||
|
||
int ret = ImplicitTailCallTenByteStruct(temp); | ||
return ret; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These should be ifdef'd under
FEATURE_MULTIREG_ARGS
. Also, is it the number of integer locals or the number of integer arguments in registers?