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

Fix spill check for struct lclVars #23570

Merged
merged 2 commits into from
Apr 2, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 29 additions & 4 deletions src/jit/importer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10624,11 +10624,36 @@ void Compiler::impImportBlockCode(BasicBlock* block)
SPILL_APPEND:

// We need to call impSpillLclRefs() for a struct type lclVar.
// This is done for non-block assignments in the handling of stloc.
if ((op1->OperGet() == GT_ASG) && varTypeIsStruct(op1->gtOp.gtOp1) &&
(op1->gtOp.gtOp1->gtOper == GT_LCL_VAR))
// This is because there may be loads of that lclVar on the evaluation stack, and
// we need to ensure that those loads are completed before we modify it.
if ((op1->OperGet() == GT_ASG) && varTypeIsStruct(op1->gtGetOp1()))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice to also describe why we need to spill in the first place.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, will do.

{
impSpillLclRefs(op1->gtOp.gtOp1->AsLclVarCommon()->gtLclNum);
GenTree* lhs = op1->gtGetOp1();
GenTreeLclVarCommon* lclVar = nullptr;
if (lhs->gtOper == GT_LCL_VAR)
{
lclVar = lhs->AsLclVarCommon();
}
else if (lhs->OperIsBlk())
{
// Note that, prior to morph, we will only see ADDR(LCL_VAR) for any assignment to
// a local struct. We should never see LCL_VAR_ADDR or ADD(ADDR(LCL_VAR) + CNS).
// Other local struct references (e.g. FIELD or more complex pointer arithmetic)
// will cause the stack to be spilled.
GenTree* addr = lhs->AsBlk()->Addr();
if (addr->OperIs(GT_ADDR) && addr->gtGetOp1()->OperIs(GT_LCL_VAR))
{
lclVar = addr->gtGetOp1()->AsLclVarCommon();
}
else
{
assert(addr->IsLocalAddrExpr() == nullptr);
}
}
if (lclVar != nullptr)
{
impSpillLclRefs(lclVar->gtLclNum);
}
}

/* Append 'op1' to the list of statements */
Expand Down
3 changes: 0 additions & 3 deletions tests/issues.targets
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,6 @@
<ExcludeList Include="$(XunitTestBinBase)/tracing/keyword/TwoKeywords/TwoKeywords/*">
<Issue>23224, often fails with timeout in release</Issue>
</ExcludeList>
<ExcludeList Include="$(XunitTestBinBase)/JIT/Methodical/flowgraph/dev10_bug679955/volatileLocal1/*">
<Issue>23545</Issue>
</ExcludeList>
</ItemGroup>

<!-- All Unix targets -->
Expand Down
50 changes: 50 additions & 0 deletions tests/src/JIT/Regression/JitBlue/GitHub_23545/GitHub_23545.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;
using System.Collections.Generic;

namespace GitHub_23545
{
public struct TestStruct
{
public int value1;

public override string ToString()
{
return this.value1.ToString();
}
}

class Test
{
public static Dictionary<TestStruct, TestStruct> StructKeyValue
{
get
{
return new Dictionary<TestStruct, TestStruct>()
{
{
new TestStruct(){value1 = 12}, new TestStruct(){value1 = 15}
}
};
}
}

static int Main()
{
int value = 0;
foreach (var e in StructKeyValue)
{
value += e.Key.value1 + e.Value.value1;
Console.WriteLine(e.Key.ToString() + " " + e.Value.ToString());
}
if (value != 27)
{
return -1;
}
return 100;
}
}
}
17 changes: 17 additions & 0 deletions tests/src/JIT/Regression/JitBlue/GitHub_23545/GitHub_23545.csproj
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<?xml version="1.0" encoding="utf-8"?>
<Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" />
<PropertyGroup>
<Configuration Condition=" '$(Configuration)' == '' ">Release</Configuration>
<Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
<AssemblyName>$(MSBuildProjectName)</AssemblyName>
<OutputType>Exe</OutputType>
<DebugType></DebugType>
<Optimize>True</Optimize>
</PropertyGroup>
<ItemGroup>
<Compile Include="$(MSBuildProjectName).cs" />
</ItemGroup>
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
<PropertyGroup Condition=" '$(MsBuildProjectDirOverride)' != '' "></PropertyGroup>
</Project>