Skip to content

Commit

Permalink
[release/6.0] Do not promote struct locals with holes (#62738)
Browse files Browse the repository at this point in the history
* Make sure the combined field size matches the struct size

* Fix the condition

* Update test and include containHoles condition

Co-authored-by: Kunal Pathak <Kunal.Pathak@microsoft.com>
  • Loading branch information
github-actions[bot] and kunalspathak authored Jan 3, 2022
1 parent 2282b00 commit 6f1e03f
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 2 deletions.
9 changes: 7 additions & 2 deletions src/coreclr/jit/lclvars.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2103,8 +2103,13 @@ bool Compiler::StructPromotionHelper::ShouldPromoteStructVar(unsigned lclNum)
// multiple registers?
if (compiler->lvaIsMultiregStruct(varDsc, compiler->info.compIsVarArgs))
{
if ((structPromotionInfo.fieldCnt != 2) &&
!((structPromotionInfo.fieldCnt == 1) && varTypeIsSIMD(structPromotionInfo.fields[0].fldType)))
if (structPromotionInfo.containsHoles && structPromotionInfo.customLayout)
{
JITDUMP("Not promoting multi-reg struct local V%02u with holes.\n", lclNum);
shouldPromote = false;
}
else if ((structPromotionInfo.fieldCnt != 2) &&
!((structPromotionInfo.fieldCnt == 1) && varTypeIsSIMD(structPromotionInfo.fields[0].fldType)))
{
JITDUMP("Not promoting multireg struct local V%02u, because lvIsParam is true, #fields != 2 and it's "
"not a single SIMD.\n",
Expand Down
70 changes: 70 additions & 0 deletions src/tests/JIT/Regression/JitBlue/Runtime_62597/Runtime_62597.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
//
// Note: In below test case, we were not honoring the fact that the explicit struct size
// of struct is 32 bytes while the only 2 fields it has is just 2 bytes. In such case,
// we would pass partial struct value.
using System;
using System.Reflection.Emit;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
using System.Text;

[StructLayout(LayoutKind.Explicit, Size = 32)]
public readonly unsafe struct SmallString
{
[FieldOffset(0)] private readonly byte _length;
[FieldOffset(1)] private readonly byte _firstByte;

public SmallString(string value)
{
fixed (char* srcPtr = value)
fixed (byte* destPtr = &_firstByte)
{
Encoding.ASCII.GetBytes(srcPtr, value.Length, destPtr, value.Length);
}

_length = (byte)value.Length;
}

[MethodImpl(MethodImplOptions.NoInlining)]
public byte Dump()
{
fixed (byte* ptr = &_firstByte)
{
byte* next = ptr + 1;
return *next;
}
}
}

public static class Program
{
static int result = 0;
public static int Main()
{
var value = new SmallString("foobar");

TheTest(value);

return result;
}

[MethodImpl(MethodImplOptions.NoInlining)]
public static void TheTest(SmallString foo)
{
Execute(foo);
}

[MethodImpl(MethodImplOptions.NoInlining)]
public static object Execute(SmallString foo)
{
byte value = foo.Dump();
// 111 corresponds to the ASCII code of 2nd characted of string "foobar" i.e. ASCII value of 'o'.
if (value == 111)
{
result = 100;
}
return new StringBuilder();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
<AllowUnsafeBlocks>True</AllowUnsafeBlocks>
</PropertyGroup>
<PropertyGroup>
<DebugType>None</DebugType>
<Optimize>True</Optimize>
</PropertyGroup>
<ItemGroup>
<Compile Include="$(MSBuildProjectName).cs" />
</ItemGroup>
</Project>

0 comments on commit 6f1e03f

Please sign in to comment.