Take fields into account checking for self-assign
authorJoseph Tremoulet <jotrem@microsoft.com>
Tue, 28 Mar 2017 18:56:27 +0000 (14:56 -0400)
committerJoseph Tremoulet <jotrem@microsoft.com>
Wed, 29 Mar 2017 18:36:00 +0000 (14:36 -0400)
The code in `fgMorphCopyBlock` that removes self-assigns checks if the LHS
and RHS refer to the same `lclVar`; update it to also check if the LHS and
RHS refer to the same field(s) of that `lclVar`, since otherwise copies
from one field to another of a struct can get lost.

Fixes #10481.

src/jit/morph.cpp
tests/src/JIT/Regression/JitBlue/GitHub_10481/GitHub_10481.cs [new file with mode: 0644]
tests/src/JIT/Regression/JitBlue/GitHub_10481/GitHub_10481.csproj [new file with mode: 0644]

index 48ed943..bb2e7e4 100644 (file)
@@ -9629,6 +9629,7 @@ GenTreePtr Compiler::fgMorphCopyBlock(GenTreePtr tree)
                 assert(dest->gtOper == GT_LCL_FLD);
                 blockWidth = genTypeSize(dest->TypeGet());
                 destAddr   = gtNewOperNode(GT_ADDR, TYP_BYREF, dest);
+                destFldSeq = dest->AsLclFld()->gtFieldSeq;
             }
         }
         else
@@ -9754,9 +9755,9 @@ GenTreePtr Compiler::fgMorphCopyBlock(GenTreePtr tree)
         bool srcSingleLclVarAsg  = false;
         bool destSingleLclVarAsg = false;
 
-        if ((destLclVar != nullptr) && (srcLclVar == destLclVar))
+        if ((destLclVar != nullptr) && (srcLclVar == destLclVar) && (destFldSeq == srcFldSeq))
         {
-            // Beyond perf reasons, it is not prudent to have a copy of a struct to itself.
+            // Self-assign; no effect.
             GenTree* nop = gtNewNothingNode();
             INDEBUG(nop->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED);
             return nop;
diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_10481/GitHub_10481.cs b/tests/src/JIT/Regression/JitBlue/GitHub_10481/GitHub_10481.cs
new file mode 100644 (file)
index 0000000..1c7a946
--- /dev/null
@@ -0,0 +1,103 @@
+// 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.Runtime.CompilerServices;
+
+// Repro case for a bug where copies from one struct-typed field of a local
+// to another were being illegally elided.
+
+namespace N
+{
+    // Original Repro
+    public struct BytesReader2
+    {
+        ArraySegmentWrapper _unreadSegments;
+        ArraySegment<byte> _currentSegment;
+
+        public BytesReader2(ArraySegmentWrapper bytes)
+        {
+            _unreadSegments = bytes;
+            _currentSegment = _unreadSegments.First; // copy gets lost when inlined into RunTest()
+        }
+
+        public bool IsEmpty => _currentSegment.Count == 0;
+    }
+
+    public struct ArraySegmentWrapper
+    {
+        ArraySegment<byte> _first;
+
+        public ArraySegment<byte> First => _first;
+
+        public ArraySegmentWrapper(ArraySegment<byte> first)
+        {
+            _first = first;
+        }
+    }
+
+    static class Repro1
+    {
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        public static int RunTest()
+        {
+            var data = new byte[] { 1 };
+            var segment = new ArraySegment<byte>(data);
+            var wrapper = new ArraySegmentWrapper(segment);
+            var reader2 = new BytesReader2(wrapper);
+            return reader2.IsEmpty ? 50 : 100;
+        }
+    }
+
+    // Simplified repro
+    struct Inner
+    {
+        public long x;
+        public long y;
+    }
+
+    struct Outer
+    {
+        public int z;
+        public Inner i;
+        public Inner j;
+    }
+
+    static class Repro2
+    {
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        static long sum(Outer o)
+        {
+            return o.i.x + o.i.y + o.j.x + o.j.y + (long)o.z;
+        }
+
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        static Inner getInner()
+        {
+            return new Inner() { x = 7, y = 33 };
+        }
+
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        public static int RunTest()
+        {
+            Outer o;
+            o.i = getInner();
+            o.j = o.i;  // Copy gets lost
+            o.z = 20;
+            return (int)sum(o);
+        }
+    }
+
+    static class C
+    {
+        public static int Main(string[] args)
+        {
+            if (Repro1.RunTest() != 100)
+            {
+              return -1;
+            }
+            return Repro2.RunTest();
+        }
+    }
+}
diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_10481/GitHub_10481.csproj b/tests/src/JIT/Regression/JitBlue/GitHub_10481/GitHub_10481.csproj
new file mode 100644 (file)
index 0000000..6b95745
--- /dev/null
@@ -0,0 +1,42 @@
+<?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)' == '' ">Debug</Configuration>
+    <Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
+    <AssemblyName>$(MSBuildProjectName)</AssemblyName>
+    <SchemaVersion>2.0</SchemaVersion>
+    <ProjectGuid>{DADEC17C-DA8A-4F7D-9927-47A9033A2E80}</ProjectGuid>
+    <OutputType>Exe</OutputType>
+    <AppDesignerFolder>Properties</AppDesignerFolder>
+    <FileAlignment>512</FileAlignment>
+    <ProjectTypeGuids>{786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids>
+    <ReferencePath>$(ProgramFiles)\Common Files\microsoft shared\VSTT\11.0\UITestExtensionPackages</ReferencePath>
+    <SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\..\</SolutionDir>
+
+    <NuGetPackageImportStamp>7a9bfb7d</NuGetPackageImportStamp>
+  </PropertyGroup>
+  <!-- Default configurations to help VS understand the configurations -->
+  <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' ">
+  </PropertyGroup>
+  <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' ">
+  </PropertyGroup>
+  <ItemGroup>
+    <CodeAnalysisDependentAssemblyPaths Condition=" '$(VS100COMNTOOLS)' != '' " Include="$(VS100COMNTOOLS)..\IDE\PrivateAssemblies">
+      <Visible>False</Visible>
+    </CodeAnalysisDependentAssemblyPaths>
+  </ItemGroup>
+  <PropertyGroup>
+    <DebugType></DebugType>
+    <Optimize>True</Optimize>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).cs" />
+  </ItemGroup>
+  <ItemGroup>
+    <Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" />
+  </ItemGroup>
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
+  <PropertyGroup Condition=" '$(MsBuildProjectDirOverride)' != '' ">
+  </PropertyGroup>
+</Project>