From bc856a428e4c063dffde9fa52487ecba82b3e36d Mon Sep 17 00:00:00 2001 From: Joseph Tremoulet Date: Tue, 28 Mar 2017 14:56:27 -0400 Subject: [PATCH] Take fields into account checking for self-assign 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 dotnet/coreclr#10481. Commit migrated from https://github.com/dotnet/coreclr/commit/c3610d32da61d995191d77b91735b462b46a516a --- src/coreclr/src/jit/morph.cpp | 5 +- .../JitBlue/GitHub_10481/GitHub_10481.cs | 103 +++++++++++++++++++++ .../JitBlue/GitHub_10481/GitHub_10481.csproj | 42 +++++++++ 3 files changed, 148 insertions(+), 2 deletions(-) create mode 100644 src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_10481/GitHub_10481.cs create mode 100644 src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_10481/GitHub_10481.csproj diff --git a/src/coreclr/src/jit/morph.cpp b/src/coreclr/src/jit/morph.cpp index 48ed943..bb2e7e4 100644 --- a/src/coreclr/src/jit/morph.cpp +++ b/src/coreclr/src/jit/morph.cpp @@ -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/src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_10481/GitHub_10481.cs b/src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_10481/GitHub_10481.cs new file mode 100644 index 0000000..1c7a946 --- /dev/null +++ b/src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_10481/GitHub_10481.cs @@ -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 _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 _first; + + public ArraySegment First => _first; + + public ArraySegmentWrapper(ArraySegment first) + { + _first = first; + } + } + + static class Repro1 + { + [MethodImpl(MethodImplOptions.NoInlining)] + public static int RunTest() + { + var data = new byte[] { 1 }; + var segment = new ArraySegment(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/src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_10481/GitHub_10481.csproj b/src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_10481/GitHub_10481.csproj new file mode 100644 index 0000000..6b95745 --- /dev/null +++ b/src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_10481/GitHub_10481.csproj @@ -0,0 +1,42 @@ + + + + + Debug + AnyCPU + $(MSBuildProjectName) + 2.0 + {DADEC17C-DA8A-4F7D-9927-47A9033A2E80} + Exe + Properties + 512 + {786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC} + $(ProgramFiles)\Common Files\microsoft shared\VSTT\11.0\UITestExtensionPackages + ..\..\ + + 7a9bfb7d + + + + + + + + + False + + + + + True + + + + + + + + + + + -- 2.7.4