Skip to content

Commit bffa7cf

Browse files
authored
Do not promote struct locals with holes (#62645)
* Make sure the combined field size matches the struct size * Fix the condition * Update test and include containHoles condition
1 parent f18f658 commit bffa7cf

File tree

3 files changed

+90
-2
lines changed

3 files changed

+90
-2
lines changed

src/coreclr/jit/lclvars.cpp

+7-2
Original file line numberDiff line numberDiff line change
@@ -2045,8 +2045,13 @@ bool Compiler::StructPromotionHelper::ShouldPromoteStructVar(unsigned lclNum)
20452045
// multiple registers?
20462046
if (compiler->lvaIsMultiregStruct(varDsc, compiler->info.compIsVarArgs))
20472047
{
2048-
if ((structPromotionInfo.fieldCnt != 2) &&
2049-
!((structPromotionInfo.fieldCnt == 1) && varTypeIsSIMD(structPromotionInfo.fields[0].fldType)))
2048+
if (structPromotionInfo.containsHoles && structPromotionInfo.customLayout)
2049+
{
2050+
JITDUMP("Not promoting multi-reg struct local V%02u with holes.\n", lclNum);
2051+
shouldPromote = false;
2052+
}
2053+
else if ((structPromotionInfo.fieldCnt != 2) &&
2054+
!((structPromotionInfo.fieldCnt == 1) && varTypeIsSIMD(structPromotionInfo.fields[0].fldType)))
20502055
{
20512056
JITDUMP("Not promoting multireg struct local V%02u, because lvIsParam is true, #fields != 2 and it's "
20522057
"not a single SIMD.\n",
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
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+
//
4+
// Note: In below test case, we were not honoring the fact that the explicit struct size
5+
// of struct is 32 bytes while the only 2 fields it has is just 2 bytes. In such case,
6+
// we would pass partial struct value.
7+
using System;
8+
using System.Reflection.Emit;
9+
using System.Runtime.CompilerServices;
10+
using System.Runtime.InteropServices;
11+
using System.Text;
12+
13+
[StructLayout(LayoutKind.Explicit, Size = 32)]
14+
public readonly unsafe struct SmallString
15+
{
16+
[FieldOffset(0)] private readonly byte _length;
17+
[FieldOffset(1)] private readonly byte _firstByte;
18+
19+
public SmallString(string value)
20+
{
21+
fixed (char* srcPtr = value)
22+
fixed (byte* destPtr = &_firstByte)
23+
{
24+
Encoding.ASCII.GetBytes(srcPtr, value.Length, destPtr, value.Length);
25+
}
26+
27+
_length = (byte)value.Length;
28+
}
29+
30+
[MethodImpl(MethodImplOptions.NoInlining)]
31+
public byte Dump()
32+
{
33+
fixed (byte* ptr = &_firstByte)
34+
{
35+
byte* next = ptr + 1;
36+
return *next;
37+
}
38+
}
39+
}
40+
41+
public static class Program
42+
{
43+
static int result = 0;
44+
public static int Main()
45+
{
46+
var value = new SmallString("foobar");
47+
48+
TheTest(value);
49+
50+
return result;
51+
}
52+
53+
[MethodImpl(MethodImplOptions.NoInlining)]
54+
public static void TheTest(SmallString foo)
55+
{
56+
Execute(foo);
57+
}
58+
59+
[MethodImpl(MethodImplOptions.NoInlining)]
60+
public static object Execute(SmallString foo)
61+
{
62+
byte value = foo.Dump();
63+
// 111 corresponds to the ASCII code of 2nd characted of string "foobar" i.e. ASCII value of 'o'.
64+
if (value == 111)
65+
{
66+
result = 100;
67+
}
68+
return new StringBuilder();
69+
}
70+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
<Project Sdk="Microsoft.NET.Sdk">
2+
<PropertyGroup>
3+
<OutputType>Exe</OutputType>
4+
<AllowUnsafeBlocks>True</AllowUnsafeBlocks>
5+
</PropertyGroup>
6+
<PropertyGroup>
7+
<DebugType>None</DebugType>
8+
<Optimize>True</Optimize>
9+
</PropertyGroup>
10+
<ItemGroup>
11+
<Compile Include="$(MSBuildProjectName).cs" />
12+
</ItemGroup>
13+
</Project>

0 commit comments

Comments
 (0)