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

Commit 6b07baf

Browse files
briansullbriansull
committed
Reject struct promotion of parameters when -GS checks are enabled
as we could introduce shadow copies of them. Add new test case: GitHub_17329.cs
1 parent cc76b83 commit 6b07baf

File tree

3 files changed

+160
-2
lines changed

3 files changed

+160
-2
lines changed

‎src/jit/lclvars.cpp‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1746,6 +1746,16 @@ void Compiler::lvaCanPromoteStructVar(unsigned lclNum, lvaStructPromotionInfo* S
17461746
// profitably promoted.
17471747
if (varDsc->lvIsUsedInSIMDIntrinsic())
17481748
{
1749+
JITDUMP(" struct promotion of V%02u is disabled because lvIsUsedInSIMDIntrinsic()\n", lclNum);
1750+
StructPromotionInfo->canPromote = false;
1751+
return;
1752+
}
1753+
1754+
// Reject struct promotion of parameters when -GS stack reordering is enabled
1755+
// as we could introduce shadow copies of them.
1756+
if (varDsc->lvIsParam && compGSReorderStackLayout)
1757+
{
1758+
JITDUMP(" struct promotion of V%02u is disabled because lvIsParam and compGSReorderStackLayout\n", lclNum);
17491759
StructPromotionInfo->canPromote = false;
17501760
return;
17511761
}
@@ -1757,22 +1767,23 @@ void Compiler::lvaCanPromoteStructVar(unsigned lclNum, lvaStructPromotionInfo* S
17571767
// TODO-PERF - Allow struct promotion for HFA register arguments
17581768
if (varDsc->lvIsHfaRegArg())
17591769
{
1770+
JITDUMP(" struct promotion of V%02u is disabled because lvIsHfaRegArg()\n", lclNum);
17601771
StructPromotionInfo->canPromote = false;
17611772
return;
17621773
}
17631774

17641775
#if !FEATURE_MULTIREG_STRUCT_PROMOTE
17651776
if (varDsc->lvIsMultiRegArg)
17661777
{
1767-
JITDUMP("Skipping V%02u: marked lvIsMultiRegArg.\n", lclNum);
1778+
JITDUMP(" struct promotion of V%02u is disabled because lvIsMultiRegArg\n", lclNum);
17681779
StructPromotionInfo->canPromote = false;
17691780
return;
17701781
}
17711782
#endif
17721783

17731784
if (varDsc->lvIsMultiRegRet)
17741785
{
1775-
JITDUMP("Skipping V%02u: marked lvIsMultiRegRet.\n", lclNum);
1786+
JITDUMP(" struct promotion of V%02u is disabled because lvIsMultiRegRet\n", lclNum);
17761787
StructPromotionInfo->canPromote = false;
17771788
return;
17781789
}

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

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
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.Runtime.CompilerServices;
7+
8+
// Having an UnsafeValueTypeAttribute on a struct causes incoming arguments to be
9+
// spilled into shadow copies and the struct promotion optimization does not
10+
// currently handle this properly.
11+
//
12+
13+
[UnsafeValueTypeAttribute]
14+
struct DangerousBuffer
15+
{
16+
public long a;
17+
public long b;
18+
public long c;
19+
}
20+
21+
struct Point1
22+
{
23+
long x;
24+
25+
public Point1(long _x)
26+
{
27+
x = _x;
28+
}
29+
30+
public void Increase(ref Point1 s, long amount)
31+
{
32+
x = s.x + amount;
33+
}
34+
35+
[MethodImplAttribute(MethodImplOptions.NoInlining)]
36+
public long Value()
37+
{
38+
return x;
39+
}
40+
}
41+
42+
class TestCase
43+
{
44+
static public long[] arr;
45+
46+
unsafe static long Test(int size, Point1 a, Point1 b, Point1 c)
47+
{
48+
49+
// Mutate the values stored in a, b and c
50+
// So if these have a shadow copy we will notice
51+
//
52+
a.Increase(ref a, arr[0]);
53+
b.Increase(ref b, arr[1]);
54+
c.Increase(ref c, arr[2]);
55+
56+
DangerousBuffer db = new DangerousBuffer();
57+
db.a = -1;
58+
db.b = -2;
59+
db.c = -3;
60+
61+
long* x1 = stackalloc long[size];
62+
63+
long sum = 0;
64+
if (size >= 3)
65+
{
66+
x1[0] = a.Value();
67+
x1[1] = b.Value();
68+
x1[2] = c.Value();
69+
70+
for (int i = 0; i < size; i++)
71+
{
72+
sum += x1[i];
73+
}
74+
}
75+
return sum;
76+
}
77+
78+
static int Main()
79+
{
80+
long testResult = 0;
81+
int mainResult = 0;
82+
83+
Point1 p1 = new Point1(1);
84+
Point1 p2 = new Point1(3);
85+
Point1 p3 = new Point1(5);
86+
87+
arr = new long[3];
88+
arr[0] = 9;
89+
arr[1] = 10;
90+
arr[2] = 11;
91+
92+
93+
testResult = Test(3, p1, p2, p3);
94+
95+
if (testResult != 39)
96+
{
97+
Console.WriteLine("FAILED!");
98+
mainResult = -1;
99+
}
100+
else
101+
{
102+
Console.WriteLine("passed");
103+
mainResult = 100;
104+
}
105+
106+
return mainResult;
107+
}
108+
}

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

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
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>{95DFC527-4DC1-495E-97D7-B8089FFA8D79}</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+
<NuGetPackageImportStamp>7a9bfb7d</NuGetPackageImportStamp>
17+
</PropertyGroup>
18+
<!-- Default configurations to help VS understand the configurations -->
19+
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' "></PropertyGroup>
20+
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' "></PropertyGroup>
21+
<ItemGroup>
22+
<CodeAnalysisDependentAssemblyPaths Condition=" '$(VS100COMNTOOLS)' != '' " Include="$(VS100COMNTOOLS)..\IDE\PrivateAssemblies">
23+
<Visible>False</Visible>
24+
</CodeAnalysisDependentAssemblyPaths>
25+
</ItemGroup>
26+
<PropertyGroup>
27+
<DebugType></DebugType>
28+
<Optimize>True</Optimize>
29+
<AllowUnsafeBlocks>True</AllowUnsafeBlocks>
30+
</PropertyGroup>
31+
<ItemGroup>
32+
<Compile Include="$(MSBuildProjectName).cs" />
33+
</ItemGroup>
34+
<ItemGroup>
35+
<Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" />
36+
</ItemGroup>
37+
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
38+
<PropertyGroup Condition=" '$(MsBuildProjectDirOverride)' != '' "></PropertyGroup>
39+
</Project>

0 commit comments

Comments
 (0)