Skip to content

Commit 4a08419

Browse files
authored
Adjust stack level while calculating offset for Vector.getElement (#38311)
* Adjust stack level * Reenable NarrowDouble() test * formatting * review comments * do not assert when isEBPbased=true * Add test coverage * updated the comment inside test
1 parent 9d538d7 commit 4a08419

File tree

4 files changed

+102
-3
lines changed

4 files changed

+102
-3
lines changed

src/coreclr/src/jit/simdcodegenxarch.cpp

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2144,6 +2144,17 @@ void CodeGen::genSIMDIntrinsicGetItem(GenTreeSIMD* simdNode)
21442144
bool isEBPbased;
21452145
unsigned varNum = op1->AsLclVarCommon()->GetLclNum();
21462146
offset += compiler->lvaFrameAddress(varNum, &isEBPbased);
2147+
2148+
#if !FEATURE_FIXED_OUT_ARGS
2149+
if (!isEBPbased)
2150+
{
2151+
// Adjust the offset by the amount currently pushed on the CPU stack
2152+
offset += genStackLevel;
2153+
}
2154+
#else
2155+
assert(genStackLevel == 0);
2156+
#endif // !FEATURE_FIXED_OUT_ARGS
2157+
21472158
if (op1->OperGet() == GT_LCL_FLD)
21482159
{
21492160
offset += op1->AsLclFld()->GetLclOffs();
@@ -2192,8 +2203,19 @@ void CodeGen::genSIMDIntrinsicGetItem(GenTreeSIMD* simdNode)
21922203
{
21932204
unsigned simdInitTempVarNum = compiler->lvaSIMDInitTempVarNum;
21942205
noway_assert(simdInitTempVarNum != BAD_VAR_NUM);
2195-
bool isEBPbased;
2196-
unsigned offs = compiler->lvaFrameAddress(simdInitTempVarNum, &isEBPbased);
2206+
bool isEBPbased;
2207+
unsigned offs = compiler->lvaFrameAddress(simdInitTempVarNum, &isEBPbased);
2208+
2209+
#if !FEATURE_FIXED_OUT_ARGS
2210+
if (!isEBPbased)
2211+
{
2212+
// Adjust the offset by the amount currently pushed on the CPU stack
2213+
offs += genStackLevel;
2214+
}
2215+
#else
2216+
assert(genStackLevel == 0);
2217+
#endif // !FEATURE_FIXED_OUT_ARGS
2218+
21972219
regNumber indexReg = op2->GetRegNum();
21982220

21992221
// Store the vector to the temp location.
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
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+
6+
using System;
7+
using System.Numerics;
8+
using System.Runtime.CompilerServices;
9+
using Xunit;
10+
11+
namespace projs
12+
{
13+
public class Program
14+
{
15+
public static int Main(string[] args)
16+
{
17+
Program p = new Program();
18+
p.NarrowDouble();
19+
Console.WriteLine("Passed.");
20+
return 100;
21+
}
22+
23+
[MethodImpl(MethodImplOptions.NoInlining)]
24+
public void NarrowDouble()
25+
{
26+
// GenerateSource1 and GenerateSource2 methods are needed to exercise the bug code path.
27+
double[] source1 = GenerateSource1();
28+
double[] source2 = GenerateSource2();
29+
Vector<double> sourceVec1 = new Vector<double>(source1);
30+
Vector<double> sourceVec2 = new Vector<double>(source2);
31+
Vector<float> dest = Vector.Narrow(sourceVec1, sourceVec2);
32+
33+
for (int i = 0; i < Vector<double>.Count; i++)
34+
{
35+
Assert.Equal(unchecked((float)source1[i]), dest[i]);
36+
}
37+
for (int i = 0; i < Vector<double>.Count; i++)
38+
{
39+
// The test represents a problem on x86 when double-alignment causes access to stack local.
40+
// At that time, the stack level was not included in the offset while trying to fetch the element
41+
// of dest present on stack.
42+
Assert.Equal(unchecked((float)source2[i]), dest[i + Vector<double>.Count]);
43+
}
44+
}
45+
46+
internal static double[] GenerateSource1()
47+
{
48+
return new double[4] { 5.1, 5.2, 5.3, 5.4 };
49+
}
50+
internal static double[] GenerateSource2()
51+
{
52+
return new double[4] { 6.1, 6.2, 6.3, 6.4 };
53+
}
54+
}
55+
}
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
<Project Sdk="Microsoft.NET.Sdk">
2+
<PropertyGroup>
3+
<OutputType>Exe</OutputType>
4+
</PropertyGroup>
5+
<PropertyGroup>
6+
<DebugType />
7+
<Optimize>True</Optimize>
8+
</PropertyGroup>
9+
<ItemGroup>
10+
<Compile Include="$(MSBuildProjectName).cs" />
11+
</ItemGroup>
12+
<PropertyGroup>
13+
<!-- Set JitStreess variables, Linux requires them to be properly capitalized. -->
14+
<CLRTestBatchPreCommands><![CDATA[
15+
$(CLRTestBatchPreCommands)
16+
set COMPlus_JitStressModeNames=STRESS_RANDOM_INLINE STRESS_DBL_ALN
17+
]]></CLRTestBatchPreCommands>
18+
<BashCLRTestPreCommands><![CDATA[
19+
$(BashCLRTestPreCommands)
20+
export COMPlus_JitStressModeNames=STRESS_RANDOM_INLINE STRESS_DBL_ALN
21+
]]></BashCLRTestPreCommands>
22+
</PropertyGroup>
23+
</Project>

src/libraries/System.Numerics.Vectors/tests/GenericVectorTests.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3031,7 +3031,6 @@ public void NarrowInt64()
30313031
}
30323032

30333033
[Fact]
3034-
[SkipOnCoreClr("https://github.com/dotnet/runtime/issues/36614", RuntimeTestModes.JitStress)]
30353034
public void NarrowDouble()
30363035
{
30373036
double[] source1 = GenerateRandomValuesForVector<double>();

0 commit comments

Comments
 (0)