Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Commit 7d9f7bf

Browse files
joshfreejoshfree
committed
Merge pull request #4012 from RussKeldorph/rc2
Unsuppress same-reg zero-extending mov (x64)
2 parents 1609011 + 64d7b17 commit 7d9f7bf

File tree

6 files changed

+206
-12
lines changed

6 files changed

+206
-12
lines changed

‎src/jit/codegenxarch.cpp‎

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7201,13 +7201,6 @@ void CodeGen::genIntToIntCast(GenTreePtr treeNode)
72017201
// We only need to check for a negative value in sourceReg
72027202
inst_RV_IV(INS_cmp, sourceReg, 0, size);
72037203
genJumpToThrowHlpBlk(EJ_jl, SCK_OVERFLOW);
7204-
if (dstType == TYP_ULONG)
7205-
{
7206-
// cast from TYP_INT to TYP_ULONG
7207-
// The upper bits on sourceReg will already be zero by definition (x64)
7208-
srcType = TYP_ULONG;
7209-
size = EA_8BYTE;
7210-
}
72117204
}
72127205
else
72137206
{
@@ -7259,8 +7252,16 @@ void CodeGen::genIntToIntCast(GenTreePtr treeNode)
72597252
}
72607253
}
72617254

7262-
if (targetReg != sourceReg)
7263-
inst_RV_RV(ins, targetReg, sourceReg, srcType, size);
7255+
if (targetReg != sourceReg
7256+
#ifdef _TARGET_AMD64_
7257+
// On amd64, we can hit this path for a same-register
7258+
// 4-byte to 8-byte widening conversion, and need to
7259+
// emit the instruction to set the high bits correctly.
7260+
|| (EA_ATTR(genTypeSize(dstType)) == EA_8BYTE
7261+
&& EA_ATTR(genTypeSize(srcType)) == EA_4BYTE)
7262+
#endif // _TARGET_AMD64_
7263+
)
7264+
inst_RV_RV(ins, targetReg, sourceReg, srcType, size);
72647265
}
72657266
else // non-overflow checking cast
72667267
{
@@ -7327,7 +7328,14 @@ void CodeGen::genIntToIntCast(GenTreePtr treeNode)
73277328
else if (ins == INS_mov)
73287329
{
73297330
noway_assert(!needAndAfter);
7330-
if (targetReg != sourceReg)
7331+
if (targetReg != sourceReg
7332+
#ifdef _TARGET_AMD64_
7333+
// On amd64, 'mov' is the opcode used to zero-extend from
7334+
// 4 bytes to 8 bytes.
7335+
|| (EA_ATTR(genTypeSize(dstType)) == EA_8BYTE
7336+
&& EA_ATTR(genTypeSize(srcType)) == EA_4BYTE)
7337+
#endif // _TARGET_AMD64_
7338+
)
73317339
{
73327340
inst_RV_RV(ins, targetReg, sourceReg, srcType, size);
73337341
}

‎src/jit/emitxarch.cpp‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3707,10 +3707,16 @@ void emitter::emitIns_R_R (instruction ins,
37073707
regNumber reg1,
37083708
regNumber reg2)
37093709
{
3710+
emitAttr size = EA_SIZE(attr);
3711+
37103712
/* We don't want to generate any useless mov instructions! */
3713+
#ifdef _TARGET_AMD64_
3714+
// Same-reg 4-byte mov can be useful because it performs a
3715+
// zero-extension to 8 bytes.
3716+
assert(ins != INS_mov || reg1 != reg2 || size == EA_4BYTE);
3717+
#else
37113718
assert(ins != INS_mov || reg1 != reg2);
3712-
3713-
emitAttr size = EA_SIZE(attr);
3719+
#endif // _TARGET_AMD64_
37143720

37153721
assert(size <= EA_32BYTE);
37163722
noway_assert(emitVerifyEncodable(ins, size, reg1, reg2));

‎tests/src/JIT/Regression/JitBlue/DevDiv_200492/DevDiv_200492.cs‎

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
using System;
6+
using System.Reflection;
7+
using System.Runtime.CompilerServices;
8+
9+
// Regression test for bug 200492, in which the x64 codegen for
10+
// integer casts would unconditionally suppress same-register
11+
// 'mov's, which is incorrect for 32-bit same-register 'mov's
12+
// that are needed to clear the upper 32 bits of a 64-bit
13+
// register. The top bits aren't guaranteed to be clear across
14+
// function boundaries, and the runtime code that invokes
15+
// custom attribute constructors passes garbage in those bits,
16+
// so this test (like the original code in the bug report)
17+
// uses custom attribute constructor arguments as the sources
18+
// of the casts in question.
19+
internal class Program
20+
{
21+
[AttributeUsage(AttributeTargets.Method)]
22+
class TestDoubleAttribute : System.Attribute
23+
{
24+
public double Field;
25+
26+
[MethodImpl(MethodImplOptions.NoInlining)]
27+
static double PickDouble(double d, int dummy)
28+
{
29+
return d;
30+
}
31+
public TestDoubleAttribute(uint f)
32+
{
33+
// Need to clear any garbage in the top half of f's
34+
// register before converting to double.
35+
this.Field = PickDouble((double)f, 0);
36+
}
37+
}
38+
39+
[MethodImpl(MethodImplOptions.NoInlining)]
40+
[Program.TestDouble(6)]
41+
public static bool DoubleTest()
42+
{
43+
var methodInfo = typeof(Program).GetTypeInfo().GetDeclaredMethod("DoubleTest");
44+
var attribute = methodInfo.GetCustomAttribute<TestDoubleAttribute>();
45+
46+
return (attribute.Field == (double)6u);
47+
}
48+
49+
[AttributeUsage(AttributeTargets.Method)]
50+
class TestUlongAttribute : System.Attribute
51+
{
52+
public ulong Field;
53+
54+
[MethodImpl(MethodImplOptions.NoInlining)]
55+
void Store(ulong l)
56+
{
57+
Field = l;
58+
}
59+
60+
public TestUlongAttribute(int i)
61+
{
62+
checked {
63+
// Need to clear any garbage in the top half
64+
// of i's register before passing to Store.
65+
Store((ulong)i);
66+
}
67+
}
68+
}
69+
70+
[MethodImpl(MethodImplOptions.NoInlining)]
71+
[Program.TestUlong(6)]
72+
public static bool UlongTest()
73+
{
74+
var methodInfo = typeof(Program).GetTypeInfo().GetDeclaredMethod("UlongTest");
75+
var attribute = methodInfo.GetCustomAttribute<TestUlongAttribute>();
76+
77+
return (attribute.Field == (ulong)6);
78+
}
79+
80+
private static int Main()
81+
{
82+
int errors = 0;
83+
84+
if (!Program.DoubleTest()) {
85+
errors += 1;
86+
}
87+
88+
if (!Program.UlongTest()) {
89+
errors += 1;
90+
}
91+
92+
if (errors > 0) {
93+
Console.WriteLine("Fail");
94+
}
95+
else
96+
{
97+
Console.WriteLine("Pass");
98+
}
99+
100+
return 100 + errors;
101+
}
102+
}

‎tests/src/JIT/Regression/JitBlue/DevDiv_200492/DevDiv_200492.csproj‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
<?xml version="1.0" encoding="utf-8"?>
2+
<Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
3+
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" />
4+
<PropertyGroup>
5+
<Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
6+
<Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
7+
<AssemblyName>$(MSBuildProjectName)</AssemblyName>
8+
<SchemaVersion>2.0</SchemaVersion>
9+
<ProjectGuid>{E2C84853-0100-4C5D-88C0-355F70483779}</ProjectGuid>
10+
<OutputType>Exe</OutputType>
11+
<AppDesignerFolder>Properties</AppDesignerFolder>
12+
<FileAlignment>512</FileAlignment>
13+
<ProjectTypeGuids>{786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids>
14+
<ReferencePath>$(ProgramFiles)\Common Files\microsoft shared\VSTT\11.0\UITestExtensionPackages</ReferencePath>
15+
<SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\..\</SolutionDir>
16+
17+
<NuGetPackageImportStamp>7a9bfb7d</NuGetPackageImportStamp>
18+
</PropertyGroup>
19+
<!-- Default configurations to help VS understand the configurations -->
20+
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' ">
21+
</PropertyGroup>
22+
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' ">
23+
</PropertyGroup>
24+
<ItemGroup>
25+
<CodeAnalysisDependentAssemblyPaths Condition=" '$(VS100COMNTOOLS)' != '' " Include="$(VS100COMNTOOLS)..\IDE\PrivateAssemblies">
26+
<Visible>False</Visible>
27+
</CodeAnalysisDependentAssemblyPaths>
28+
</ItemGroup>
29+
<PropertyGroup>
30+
<DebugType></DebugType>
31+
<Optimize>True</Optimize>
32+
</PropertyGroup>
33+
<ItemGroup>
34+
<Compile Include="$(MSBuildProjectName).cs" />
35+
</ItemGroup>
36+
<ItemGroup>
37+
<None Include="project.json" />
38+
<None Include="app.config" />
39+
</ItemGroup>
40+
<ItemGroup>
41+
<Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" />
42+
</ItemGroup>
43+
<PropertyGroup>
44+
<ProjectJson>$(JitPackagesConfigFileDirectory)extra\project.json</ProjectJson>
45+
<ProjectLockJson>$(JitPackagesConfigFileDirectory)extra\project.lock.json</ProjectLockJson>
46+
</PropertyGroup>
47+
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
48+
<PropertyGroup Condition=" '$(MsBuildProjectDirOverride)' != '' ">
49+
</PropertyGroup>
50+
</Project>

‎tests/src/JIT/Regression/JitBlue/DevDiv_200492/app.config‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
<?xml version="1.0" encoding="utf-8"?>
2+
<configuration>
3+
<runtime>
4+
<assemblyBinding xmlns="urn:schemas-microsoft-com:asm.v1">
5+
<dependentAssembly>
6+
<assemblyIdentity name="System.Runtime" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" />
7+
<bindingRedirect oldVersion="0.0.0.0-4.0.20.0" newVersion="4.0.20.0" />
8+
</dependentAssembly>
9+
<dependentAssembly>
10+
<assemblyIdentity name="System.Text.Encoding" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" />
11+
<bindingRedirect oldVersion="0.0.0.0-4.0.10.0" newVersion="4.0.10.0" />
12+
</dependentAssembly>
13+
<dependentAssembly>
14+
<assemblyIdentity name="System.Threading.Tasks" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" />
15+
<bindingRedirect oldVersion="0.0.0.0-4.0.10.0" newVersion="4.0.10.0" />
16+
</dependentAssembly>
17+
<dependentAssembly>
18+
<assemblyIdentity name="System.IO" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" />
19+
<bindingRedirect oldVersion="0.0.0.0-4.0.10.0" newVersion="4.0.10.0" />
20+
</dependentAssembly>
21+
<dependentAssembly>
22+
<assemblyIdentity name="System.Reflection" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" />
23+
<bindingRedirect oldVersion="0.0.0.0-4.0.10.0" newVersion="4.0.10.0" />
24+
</dependentAssembly>
25+
</assemblyBinding>
26+
</runtime>
27+
</configuration>

‎tests/src/JIT/config/extra/project.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
"System.IO": "4.0.11-rc2-23816",
1111
"System.IO.FileSystem": "4.0.0",
1212
"System.Reflection": "4.1.0-rc2-23816",
13+
"System.Reflection.Extensions": "4.0.1-rc2-23816",
1314
"System.Reflection.TypeExtensions": "4.0.0",
1415
"System.Runtime": "4.1.0-rc2-23816",
1516
"System.Runtime.Extensions": "4.0.10",

0 commit comments

Comments
 (0)