From 1937982a886703ee69f479badcfffc56bc5a8f57 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sat, 28 Aug 2021 12:59:06 +0300 Subject: [PATCH 1/4] fix fgValueNumberArrIndexVal for wide reads --- src/coreclr/jit/valuenum.cpp | 4 +- .../JitBlue/Runtime_57914/Runtime_57914.cs | 48 +++++++++++++++++++ .../Runtime_57914/Runtime_57914.csproj | 12 +++++ 3 files changed, 62 insertions(+), 2 deletions(-) create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_57914/Runtime_57914.cs create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_57914/Runtime_57914.csproj diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 84a7465e33a7a..6de40642fbdbf 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -4243,10 +4243,10 @@ ValueNum Compiler::fgValueNumberArrIndexVal(GenTree* tree, var_types indType = (tree == nullptr) ? elemTyp : tree->TypeGet(); ValueNum selectedElem; - if (fldSeq == FieldSeqStore::NotAField()) + if ((fldSeq == FieldSeqStore::NotAField()) || (genTypeSize(indType) > genTypeSize(elemTyp))) { // This doesn't represent a proper array access - JITDUMP(" *** NotAField sequence encountered in fgValueNumberArrIndexVal\n"); + JITDUMP(" *** Not a proper arrray access encountered in fgValueNumberArrIndexVal\n"); // a new unique value number selectedElem = vnStore->VNForExpr(compCurBB, elemTyp); diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_57914/Runtime_57914.cs b/src/tests/JIT/Regression/JitBlue/Runtime_57914/Runtime_57914.cs new file mode 100644 index 0000000000000..46b0ef4d43827 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_57914/Runtime_57914.cs @@ -0,0 +1,48 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Runtime.CompilerServices; + +public class Program +{ + public static int Main() + { + return (Test1() && Test2()) ? 100 : 101; + } + + private static bool Test1() + { + byte[] arr = new byte[2]; + arr[0] = 1; + arr[1] = 2; + + short a1 = Unsafe.ReadUnaligned(ref arr[0]); + arr[1] = 42; + short a2 = Unsafe.ReadUnaligned(ref arr[0]); + + return a1 != a2; + } + + private static bool Test2() + { + bool result = true; + byte[] buffer = new byte[4]; + buffer[0] = 0x1; + buffer[1] = 0x2; + buffer[2] = 0x3; + buffer[3] = 0x4; + + if (buffer.Length > 0) + { + int n = Unsafe.ReadUnaligned(ref buffer[0]); + if (n != 0x4030201) + result = false; + Consume(n); + } + return result; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static void Consume(int n) {} +} diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_57914/Runtime_57914.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_57914/Runtime_57914.csproj new file mode 100644 index 0000000000000..f3e1cbd44b404 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_57914/Runtime_57914.csproj @@ -0,0 +1,12 @@ + + + Exe + + + None + True + + + + + From d92ae2ab20572f9dbaf3f8e82ffce898d49d4335 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sat, 28 Aug 2021 13:15:39 +0300 Subject: [PATCH 2/4] Update test --- .../JitBlue/Runtime_57914/Runtime_57914.cs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_57914/Runtime_57914.cs b/src/tests/JIT/Regression/JitBlue/Runtime_57914/Runtime_57914.cs index 46b0ef4d43827..f11dc7d830207 100644 --- a/src/tests/JIT/Regression/JitBlue/Runtime_57914/Runtime_57914.cs +++ b/src/tests/JIT/Regression/JitBlue/Runtime_57914/Runtime_57914.cs @@ -13,15 +13,17 @@ public static int Main() private static bool Test1() { - byte[] arr = new byte[2]; - arr[0] = 1; - arr[1] = 2; + byte[] array = new byte[2]; + array[0] = 1; + array[1] = 2; - short a1 = Unsafe.ReadUnaligned(ref arr[0]); - arr[1] = 42; - short a2 = Unsafe.ReadUnaligned(ref arr[0]); + // a1, a2 and a3 all have different values here + byte a1 = Unsafe.ReadUnaligned(ref array[0]); + short a2 = Unsafe.ReadUnaligned(ref array[0]); + array[1] = 42; + short a3 = Unsafe.ReadUnaligned(ref array[0]); - return a1 != a2; + return a1 != a2 && a1 != a3 && a2 != a3; } private static bool Test2() From 486574b3a7bf28ebed8dafa559e49d148536eb23 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sat, 28 Aug 2021 17:16:19 +0300 Subject: [PATCH 3/4] Fix regressions --- src/coreclr/jit/valuenum.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 6de40642fbdbf..cdd04f1e89602 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -4242,8 +4242,9 @@ ValueNum Compiler::fgValueNumberArrIndexVal(GenTree* tree, var_types elemTyp = DecodeElemType(elemTypeEq); var_types indType = (tree == nullptr) ? elemTyp : tree->TypeGet(); ValueNum selectedElem; + unsigned elemWidth = varTypeIsStruct(elemTyp) ? info.compCompHnd->getClassSize(elemTypeEq) : genTypeSize(elemTyp); - if ((fldSeq == FieldSeqStore::NotAField()) || (genTypeSize(indType) > genTypeSize(elemTyp))) + if ((fldSeq == FieldSeqStore::NotAField()) || (genTypeSize(indType) > elemWidth)) { // This doesn't represent a proper array access JITDUMP(" *** Not a proper arrray access encountered in fgValueNumberArrIndexVal\n"); From d49e741a05ed34b99183de17fabfb7cc34683701 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Sat, 28 Aug 2021 19:25:44 +0300 Subject: [PATCH 4/4] Update valuenum.cpp --- src/coreclr/jit/valuenum.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index cdd04f1e89602..8e302db1abaf3 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -4242,7 +4242,7 @@ ValueNum Compiler::fgValueNumberArrIndexVal(GenTree* tree, var_types elemTyp = DecodeElemType(elemTypeEq); var_types indType = (tree == nullptr) ? elemTyp : tree->TypeGet(); ValueNum selectedElem; - unsigned elemWidth = varTypeIsStruct(elemTyp) ? info.compCompHnd->getClassSize(elemTypeEq) : genTypeSize(elemTyp); + unsigned elemWidth = elemTyp == TYP_STRUCT ? info.compCompHnd->getClassSize(elemTypeEq) : genTypeSize(elemTyp); if ((fldSeq == FieldSeqStore::NotAField()) || (genTypeSize(indType) > elemWidth)) {