Skip to content

Commit

Permalink
Fix spill check for struct lclVars (dotnet/coreclr#23570)
Browse files Browse the repository at this point in the history
* Fix spill check for struct lclVars

With the 1st class struct changes, the `SPILL_APPEND` check for the case of an assignment to a lclVar needs to handle block ops as well as lclVar lhs.

Fix dotnet/coreclr#23545


Commit migrated from dotnet/coreclr@f036d23
  • Loading branch information
CarolEidt authored Apr 2, 2019
1 parent 952b410 commit aa45b25
Show file tree
Hide file tree
Showing 4 changed files with 96 additions and 7 deletions.
33 changes: 29 additions & 4 deletions src/coreclr/src/jit/importer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10740,11 +10740,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()))
{
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 src/coreclr/tests/issues.targets
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,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
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;
}
}
}
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>

0 comments on commit aa45b25

Please sign in to comment.