Fix spill check for struct lclVars (#23570)

* 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 #23545

diff --git a/src/jit/importer.cpp b/src/jit/importer.cpp
index 244605c..95de830 100644 (file)
--- a/src/jit/importer.cpp
+++ b/src/jit/importer.cpp
@@ -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 */

diff --git a/tests/issues.targets b/tests/issues.targets
index 2e6a4be..ec060ad 100644 (file)
--- a/tests/issues.targets
+++ b/tests/issues.targets
@@ -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 -->

diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_23545/GitHub_23545.cs b/tests/src/JIT/Regression/JitBlue/GitHub_23545/GitHub_23545.cs
new file mode 100644 (file)
index 0000000..3ea3d97
--- /dev/null
+++ b/tests/src/JIT/Regression/JitBlue/GitHub_23545/GitHub_23545.cs
@@ -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;
+        }
+    }
+}

diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_23545/GitHub_23545.csproj b/tests/src/JIT/Regression/JitBlue/GitHub_23545/GitHub_23545.csproj
new file mode 100644 (file)
index 0000000..c24f74b
--- /dev/null
+++ b/tests/src/JIT/Regression/JitBlue/GitHub_23545/GitHub_23545.csproj
@@ -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>