From 4ca8649ac193504c078ef62787ee323919eec0ef Mon Sep 17 00:00:00 2001 From: Sergey Date: Mon, 9 Aug 2021 17:17:45 -0700 Subject: [PATCH 01/10] Add/return tests. --- .../ArrayBufferWriterTests.T.cs | 1 - .../JitBlue/Runtime_54102/Runtime_54102.cs | 52 ++++++++++++++++ .../Runtime_54102/Runtime_54102.csproj | 12 ++++ .../JitBlue/Runtime_56980/Runtime_56980.cs | 61 +++++++++++++++++++ .../Runtime_56980/Runtime_56980.csproj | 12 ++++ src/tests/JIT/opt/Structs/structcopies.csproj | 2 - 6 files changed, 137 insertions(+), 3 deletions(-) create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_54102/Runtime_54102.cs create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_54102/Runtime_54102.csproj create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_56980/Runtime_56980.cs create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_56980/Runtime_56980.csproj diff --git a/src/libraries/System.Memory/tests/ArrayBufferWriter/ArrayBufferWriterTests.T.cs b/src/libraries/System.Memory/tests/ArrayBufferWriter/ArrayBufferWriterTests.T.cs index 656918e5fe6e7..b1a6c293aef32 100644 --- a/src/libraries/System.Memory/tests/ArrayBufferWriter/ArrayBufferWriterTests.T.cs +++ b/src/libraries/System.Memory/tests/ArrayBufferWriter/ArrayBufferWriterTests.T.cs @@ -65,7 +65,6 @@ public void Clear() } [Fact] - [SkipOnCoreClr("https://github.com/dotnet/runtime/issues/42517", RuntimeConfiguration.Checked)] public void Advance() { { diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_54102/Runtime_54102.cs b/src/tests/JIT/Regression/JitBlue/Runtime_54102/Runtime_54102.cs new file mode 100644 index 0000000000000..8afef4323c2fb --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_54102/Runtime_54102.cs @@ -0,0 +1,52 @@ +// 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; + +// Generated by Fuzzlyn v1.1 on 2021-06-12 11:06:35 +// Seed: 8276490119048877745 +// Reduced from 962.5 KiB to 0.6 KiB in 00:05:32 +// Debug: Outputs 0 +// Release: Outputs 1 + +struct S0 +{ + public ushort F1; + public uint F2; + public byte F3; + public int F5; + public S0(byte f3): this() + { + F3 = f3; + } +} + +struct S1 +{ + public S0 F0; + public S0 F4; + public S1(S0 f0): this() + { + F0 = f0; + } +} + +struct S2 +{ + public S1 F0; + public S2(S1 f0): this() + { + F0 = f0; + } +} + +public class Program +{ + public static int Main() + { + S2 vr0 = new S2(new S1(new S0(100))); + int ret = vr0.F0.F0.F3 + vr0.F0.F4.F1; + return ret; + } +} diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_54102/Runtime_54102.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_54102/Runtime_54102.csproj new file mode 100644 index 0000000000000..f3e1cbd44b404 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_54102/Runtime_54102.csproj @@ -0,0 +1,12 @@ + + + Exe + + + None + True + + + + + diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_56980/Runtime_56980.cs b/src/tests/JIT/Regression/JitBlue/Runtime_56980/Runtime_56980.cs new file mode 100644 index 0000000000000..1ff1c7611e84f --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_56980/Runtime_56980.cs @@ -0,0 +1,61 @@ +// 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; + +// Generated by Fuzzlyn v1.2 on 2021-07-06 09:46:44 +// Seed: 16635934940619066544 +// Reduced from 447.5 KiB to 0.6 KiB in 00:02:24 +// Debug: Runs successfully +// Release: Throws 'System.NullReferenceException' +struct S0 +{ + public uint F1; + public byte F3; + public long F4; + public uint F5; + public S0(long f4): this() + { + F4 = f4; + } +} + +class C0 +{ + public S0 F4; +} + +struct S1 +{ + public C0 F2; + public S0 F8; + public S1(C0 f2, S0 f8): this() + { + F2 = f2; + F8 = f8; + } +} + +struct S2 +{ + public S1 F0; + public S2(S1 f0): this() + { + F0 = f0; + } +} + +public class Program +{ + public static int Main() + { + S2 vr0 = new S2(new S1(new C0(), new S0(0))); + M17(ref vr0.F0.F2.F4.F1); + return 100; + } + + static void M17(ref uint arg2) + { + } +} \ No newline at end of file diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_56980/Runtime_56980.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_56980/Runtime_56980.csproj new file mode 100644 index 0000000000000..f3e1cbd44b404 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_56980/Runtime_56980.csproj @@ -0,0 +1,12 @@ + + + Exe + + + None + True + + + + + diff --git a/src/tests/JIT/opt/Structs/structcopies.csproj b/src/tests/JIT/opt/Structs/structcopies.csproj index c85bacc9218e5..f3e1cbd44b404 100644 --- a/src/tests/JIT/opt/Structs/structcopies.csproj +++ b/src/tests/JIT/opt/Structs/structcopies.csproj @@ -5,8 +5,6 @@ None True - - True From df7d3868159b9140ff01184ecc8ff5792a6f9a4a Mon Sep 17 00:00:00 2001 From: Sergey Date: Thu, 12 Aug 2021 04:28:02 -0700 Subject: [PATCH 02/10] improve the test naming. --- src/tests/JIT/opt/Structs/structcopies.cs | 88 +++++++++++------------ 1 file changed, 44 insertions(+), 44 deletions(-) diff --git a/src/tests/JIT/opt/Structs/structcopies.cs b/src/tests/JIT/opt/Structs/structcopies.cs index e1331e38f0061..61d8a6f424f0f 100644 --- a/src/tests/JIT/opt/Structs/structcopies.cs +++ b/src/tests/JIT/opt/Structs/structcopies.cs @@ -36,7 +36,7 @@ struct S4W struct S4WW { - public S4W s4; + public S4W s4W; } struct S4Copy @@ -142,18 +142,18 @@ static int TestS4_WW1() { S4 s1 = new S4(); S4WW s2 = new S4WW(); - s2.s4.s4.i = 1; + s2.s4W.s4.i = 1; if (s1.i != 0) { return 101; } - s1 = s2.s4.s4; - s2.s4.s4.i = 2; + s1 = s2.s4W.s4; + s2.s4W.s4.i = 2; if (s1.i != 1) { return 101; } - if (s2.s4.s4.i != 2) + if (s2.s4W.s4.i != 2) { return 101; } @@ -166,13 +166,13 @@ static int TestS4_WW2() S4WW s1 = new S4WW(); S4 s2 = new S4(); s2.i = 1; - if (s1.s4.s4.i != 0) + if (s1.s4W.s4.i != 0) { return 101; } - s1.s4.s4 = s2; + s1.s4W.s4 = s2; s2.i = 2; - if (s1.s4.s4.i != 1) + if (s1.s4W.s4.i != 1) { return 101; } @@ -462,7 +462,7 @@ struct S8W struct S8WW { - public S8W s8; + public S8W s8W; } struct S8Copy @@ -605,8 +605,8 @@ static int TestS8_WW1() { S8 s1 = new S8(); S8WW s2 = new S8WW(); - s2.s8.s8.i1 = 1; - s2.s8.s8.i2 = 2; + s2.s8W.s8.i1 = 1; + s2.s8W.s8.i2 = 2; if (s1.i1 != 0) { return 101; @@ -615,9 +615,9 @@ static int TestS8_WW1() { return 101; } - s1 = s2.s8.s8; - s2.s8.s8.i1 = 3; - s2.s8.s8.i2 = 4; + s1 = s2.s8W.s8; + s2.s8W.s8.i1 = 3; + s2.s8W.s8.i2 = 4; if (s1.i1 != 1) { return 101; @@ -626,11 +626,11 @@ static int TestS8_WW1() { return 101; } - if (s2.s8.s8.i1 != 3) + if (s2.s8W.s8.i1 != 3) { return 101; } - if (s2.s8.s8.i2 != 4) + if (s2.s8W.s8.i2 != 4) { return 101; } @@ -644,22 +644,22 @@ static int TestS8_WW2() S8 s2 = new S8(); s2.i1 = 1; s2.i2 = 2; - if (s1.s8.s8.i1 != 0) + if (s1.s8W.s8.i1 != 0) { return 101; } - if (s1.s8.s8.i2 != 0) + if (s1.s8W.s8.i2 != 0) { return 101; } - s1.s8.s8 = s2; + s1.s8W.s8 = s2; s2.i1 = 3; s2.i2 = 4; - if (s1.s8.s8.i1 != 1) + if (s1.s8W.s8.i1 != 1) { return 101; } - if (s1.s8.s8.i2 != 2) + if (s1.s8W.s8.i2 != 2) { return 101; } @@ -989,7 +989,7 @@ struct S16W struct S16WW { - public S16W s16; + public S16W s16W; } struct S16Copy @@ -1215,10 +1215,10 @@ static int TestS16_WW1() { S16 s1 = new S16(); S16WW s2 = new S16WW(); - s2.s16.s16.i1 = 1; - s2.s16.s16.i2 = 2; - s2.s16.s16.i3 = 3; - s2.s16.s16.i4 = 4; + s2.s16W.s16.i1 = 1; + s2.s16W.s16.i2 = 2; + s2.s16W.s16.i3 = 3; + s2.s16W.s16.i4 = 4; if (s1.i1 != 0) { return 101; @@ -1235,11 +1235,11 @@ static int TestS16_WW1() { return 101; } - s1 = s2.s16.s16; - s2.s16.s16.i1 = 5; - s2.s16.s16.i2 = 6; - s2.s16.s16.i3 = 7; - s2.s16.s16.i4 = 8; + s1 = s2.s16W.s16; + s2.s16W.s16.i1 = 5; + s2.s16W.s16.i2 = 6; + s2.s16W.s16.i3 = 7; + s2.s16W.s16.i4 = 8; if (s1.i1 != 1) { @@ -1258,19 +1258,19 @@ static int TestS16_WW1() return 101; } - if (s2.s16.s16.i1 != 5) + if (s2.s16W.s16.i1 != 5) { return 101; } - if (s2.s16.s16.i2 != 6) + if (s2.s16W.s16.i2 != 6) { return 101; } - if (s2.s16.s16.i3 != 7) + if (s2.s16W.s16.i3 != 7) { return 101; } - if (s2.s16.s16.i4 != 8) + if (s2.s16W.s16.i4 != 8) { return 101; } @@ -1286,41 +1286,41 @@ static int TestS16_WW2() s2.i2 = 2; s2.i3 = 3; s2.i4 = 4; - if (s1.s16.s16.i1 != 0) + if (s1.s16W.s16.i1 != 0) { return 101; } - if (s1.s16.s16.i2 != 0) + if (s1.s16W.s16.i2 != 0) { return 101; } - if (s1.s16.s16.i3 != 0) + if (s1.s16W.s16.i3 != 0) { return 101; } - if (s1.s16.s16.i4 != 0) + if (s1.s16W.s16.i4 != 0) { return 101; } - s1.s16.s16 = s2; + s1.s16W.s16 = s2; s2.i1 = 5; s2.i2 = 6; s2.i3 = 7; s2.i4 = 8; - if (s1.s16.s16.i1 != 1) + if (s1.s16W.s16.i1 != 1) { return 101; } - if (s1.s16.s16.i2 != 2) + if (s1.s16W.s16.i2 != 2) { return 101; } - if (s1.s16.s16.i3 != 3) + if (s1.s16W.s16.i3 != 3) { return 101; } - if (s1.s16.s16.i4 != 4) + if (s1.s16W.s16.i4 != 4) { return 101; } From d58048aa0197d2aafd1973eadfee0ee0d1f40a86 Mon Sep 17 00:00:00 2001 From: Sergey Date: Thu, 12 Aug 2021 02:54:50 -0700 Subject: [PATCH 03/10] Add a new JitEE method. with a naive implementation so far. --- .../superpmi/superpmi-shared/lwmlist.h | 1 + .../superpmi-shared/methodcontext.cpp | 34 +++++++++++++++++++ .../superpmi/superpmi-shared/methodcontext.h | 6 +++- .../superpmi-shim-collector/icorjitinfo.cpp | 10 ++++++ .../superpmi-shim-counter/icorjitinfo.cpp | 8 +++++ .../superpmi-shim-simple/icorjitinfo.cpp | 7 ++++ .../ToolBox/superpmi/superpmi/icorjitinfo.cpp | 7 ++++ src/coreclr/inc/corjit.h | 6 ++++ src/coreclr/inc/icorjitinfoimpl_generated.h | 4 +++ src/coreclr/inc/jiteeversionguid.h | 12 +++---- src/coreclr/jit/ICorJitInfo_API_names.h | 1 + src/coreclr/jit/ICorJitInfo_API_wrapper.hpp | 10 ++++++ .../tools/Common/JitInterface/CorInfoBase.cs | 18 +++++++++- .../tools/Common/JitInterface/CorInfoImpl.cs | 6 ++++ .../ThunkGenerator/ThunkInput.txt | 2 +- .../tools/aot/jitinterface/jitinterface.h | 11 ++++++ src/coreclr/vm/jitinterface.cpp | 29 ++++++++++++++++ src/coreclr/vm/jitinterface.h | 2 ++ 18 files changed, 165 insertions(+), 9 deletions(-) diff --git a/src/coreclr/ToolBox/superpmi/superpmi-shared/lwmlist.h b/src/coreclr/ToolBox/superpmi/superpmi-shared/lwmlist.h index 0f17ce7290e71..4bffe52ecb4f6 100644 --- a/src/coreclr/ToolBox/superpmi/superpmi-shared/lwmlist.h +++ b/src/coreclr/ToolBox/superpmi/superpmi-shared/lwmlist.h @@ -157,6 +157,7 @@ LWM(TryResolveToken, Agnostic_CORINFO_RESOLVED_TOKENin, TryResolveTokenValue) LWM(SatisfiesClassConstraints, DWORDLONG, DWORD) LWM(SatisfiesMethodConstraints, DLDL, DWORD) LWM(GetUnmanagedCallConv, MethodOrSigInfoValue, DD) +LWM(DoesFieldBelongToClass, DLDL, DWORD) DENSELWM(SigInstHandleMap, DWORDLONG) #undef LWM diff --git a/src/coreclr/ToolBox/superpmi/superpmi-shared/methodcontext.cpp b/src/coreclr/ToolBox/superpmi/superpmi-shared/methodcontext.cpp index ae87cb1edde82..7d4a191c51cf6 100644 --- a/src/coreclr/ToolBox/superpmi/superpmi-shared/methodcontext.cpp +++ b/src/coreclr/ToolBox/superpmi/superpmi-shared/methodcontext.cpp @@ -6265,6 +6265,40 @@ DWORD MethodContext::repGetExpectedTargetArchitecture() return value; } +void MethodContext::recDoesFieldBelongToClass(CORINFO_FIELD_HANDLE fld, CORINFO_CLASS_HANDLE cls, bool result) +{ + if (DoesFieldBelongToClass == nullptr) + DoesFieldBelongToClass = new LightWeightMap(); + + DLDL key; + ZeroMemory(&key, sizeof(key)); // Zero key including any struct padding + key.A = CastHandle(fld); + key.B = CastHandle(cls); + + DWORD value = (DWORD)result; + DoesFieldBelongToClass->Add(key, value); + DEBUG_REC(dmpDoesFieldBelongToClass(key, result)); +} + +void MethodContext::dmpDoesFieldBelongToClass(DLDL key, bool value) +{ + printf("DoesFieldBelongToClass key fld=%016llX, cls=%016llx, result=%d", key.A, key.B, value); +} + +bool MethodContext::repDoesFieldBelongToClass(CORINFO_FIELD_HANDLE fld, CORINFO_CLASS_HANDLE cls) +{ + DLDL key; + ZeroMemory(&key, sizeof(key)); // Zero key including any struct padding + key.A = CastHandle(fld); + key.B = CastHandle(cls); + + AssertMapAndKeyExist(DoesFieldBelongToClass, key, ": key %016llX %016llX", key.A, key.B); + + bool value = (bool)DoesFieldBelongToClass->Get(key); + DEBUG_REP(dmpDoesFieldBelongToClass(key, value)); + return value; +} + void MethodContext::recIsValidToken(CORINFO_MODULE_HANDLE module, unsigned metaTOK, bool result) { if (IsValidToken == nullptr) diff --git a/src/coreclr/ToolBox/superpmi/superpmi-shared/methodcontext.h b/src/coreclr/ToolBox/superpmi/superpmi-shared/methodcontext.h index b342996b44631..4abbef3faf135 100644 --- a/src/coreclr/ToolBox/superpmi/superpmi-shared/methodcontext.h +++ b/src/coreclr/ToolBox/superpmi/superpmi-shared/methodcontext.h @@ -769,6 +769,10 @@ class MethodContext void dmpGetExpectedTargetArchitecture(DWORD key, DWORD result); DWORD repGetExpectedTargetArchitecture(); + void recDoesFieldBelongToClass(CORINFO_FIELD_HANDLE fld, CORINFO_CLASS_HANDLE cls, bool result); + void dmpDoesFieldBelongToClass(DLDL key, bool value); + bool repDoesFieldBelongToClass(CORINFO_FIELD_HANDLE fld, CORINFO_CLASS_HANDLE cls); + void recIsValidToken(CORINFO_MODULE_HANDLE module, unsigned metaTOK, bool result); void dmpIsValidToken(DLD key, DWORD value); bool repIsValidToken(CORINFO_MODULE_HANDLE module, unsigned metaTOK); @@ -1063,7 +1067,7 @@ enum mcPackets Packet_TryResolveToken = 158, // Added 4/26/2016 Packet_SatisfiesClassConstraints = 110, Packet_SatisfiesMethodConstraints = 111, - Packet_ShouldEnforceCallvirtRestriction = 112, // Retired 2/18/2020 + Packet_DoesFieldBelongToClass = 112, // Added 8/12/2021 Packet_SigInstHandleMap = 184, Packet_AllocPgoInstrumentationBySchema = 186, // Added 1/4/2021 Packet_GetPgoInstrumentationResults = 187, // Added 1/4/2021 diff --git a/src/coreclr/ToolBox/superpmi/superpmi-shim-collector/icorjitinfo.cpp b/src/coreclr/ToolBox/superpmi/superpmi-shim-collector/icorjitinfo.cpp index 4390113cf0279..2b802b3eacc31 100644 --- a/src/coreclr/ToolBox/superpmi/superpmi-shim-collector/icorjitinfo.cpp +++ b/src/coreclr/ToolBox/superpmi/superpmi-shim-collector/icorjitinfo.cpp @@ -2010,3 +2010,13 @@ bool interceptor_ICJI::notifyInstructionSetUsage(CORINFO_InstructionSet instruct { return original_ICorJitInfo->notifyInstructionSetUsage(instructionSet, supported); } + +bool interceptor_ICJI::doesFieldBelongToClass( + CORINFO_FIELD_HANDLE fldHnd, + CORINFO_CLASS_HANDLE cls) +{ + mc->cr->AddCall("doesFieldBelongToClass"); + bool result = original_ICorJitInfo->doesFieldBelongToClass(fldHnd, cls); + mc->recDoesFieldBelongToClass(fldHnd, cls, result); + return result; +} diff --git a/src/coreclr/ToolBox/superpmi/superpmi-shim-counter/icorjitinfo.cpp b/src/coreclr/ToolBox/superpmi/superpmi-shim-counter/icorjitinfo.cpp index 445cc2a490e5b..e967975ae244a 100644 --- a/src/coreclr/ToolBox/superpmi/superpmi-shim-counter/icorjitinfo.cpp +++ b/src/coreclr/ToolBox/superpmi/superpmi-shim-counter/icorjitinfo.cpp @@ -1389,3 +1389,11 @@ uint32_t interceptor_ICJI::getJitFlags( return original_ICorJitInfo->getJitFlags(flags, sizeInBytes); } +bool interceptor_ICJI::doesFieldBelongToClass( + CORINFO_FIELD_HANDLE fldHnd, + CORINFO_CLASS_HANDLE cls) +{ + mcs->AddCall("doesFieldBelongToClass"); + return original_ICorJitInfo->doesFieldBelongToClass(fldHnd, cls); +} + diff --git a/src/coreclr/ToolBox/superpmi/superpmi-shim-simple/icorjitinfo.cpp b/src/coreclr/ToolBox/superpmi/superpmi-shim-simple/icorjitinfo.cpp index 7b04e139c4d01..8c982934239fe 100644 --- a/src/coreclr/ToolBox/superpmi/superpmi-shim-simple/icorjitinfo.cpp +++ b/src/coreclr/ToolBox/superpmi/superpmi-shim-simple/icorjitinfo.cpp @@ -1217,3 +1217,10 @@ uint32_t interceptor_ICJI::getJitFlags( return original_ICorJitInfo->getJitFlags(flags, sizeInBytes); } +bool interceptor_ICJI::doesFieldBelongToClass( + CORINFO_FIELD_HANDLE fldHnd, + CORINFO_CLASS_HANDLE cls) +{ + return original_ICorJitInfo->doesFieldBelongToClass(fldHnd, cls); +} + diff --git a/src/coreclr/ToolBox/superpmi/superpmi/icorjitinfo.cpp b/src/coreclr/ToolBox/superpmi/superpmi/icorjitinfo.cpp index b2cb35339adcc..cac9ce2b0f871 100644 --- a/src/coreclr/ToolBox/superpmi/superpmi/icorjitinfo.cpp +++ b/src/coreclr/ToolBox/superpmi/superpmi/icorjitinfo.cpp @@ -1574,6 +1574,13 @@ uint32_t MyICJI::getJitFlags(CORJIT_FLAGS* jitFlags, uint32_t sizeInBytes) return ret; } +bool MyICJI::doesFieldBelongToClass(CORINFO_FIELD_HANDLE fldHnd, CORINFO_CLASS_HANDLE cls) +{ + jitInstance->mc->cr->AddCall("doesFieldBelongToClass"); + bool result = jitInstance->mc->repDoesFieldBelongToClass(fldHnd, cls); + return result; +} + // Runs the given function with the given parameter under an error trap // and returns true if the function completes successfully. We fake this // up a bit for SuperPMI and simply catch all exceptions. diff --git a/src/coreclr/inc/corjit.h b/src/coreclr/inc/corjit.h index d4a22f2ee3e4d..d679c99ce9027 100644 --- a/src/coreclr/inc/corjit.h +++ b/src/coreclr/inc/corjit.h @@ -484,6 +484,12 @@ class ICorJitInfo : public ICorDynamicInfo uint32_t sizeInBytes /* IN: The size of the buffer. Note that this is effectively a version number for the CORJIT_FLAGS value. */ ) = 0; + + // Checks if a field belongs to a given class. + virtual bool doesFieldBelongToClass( + CORINFO_FIELD_HANDLE fldHnd, /* IN: the field that we are checking */ + CORINFO_CLASS_HANDLE cls /* IN: the class that we are checking */ + ) = 0; }; /**********************************************************************************/ diff --git a/src/coreclr/inc/icorjitinfoimpl_generated.h b/src/coreclr/inc/icorjitinfoimpl_generated.h index b3b77ece974a8..1c470cf1fdd51 100644 --- a/src/coreclr/inc/icorjitinfoimpl_generated.h +++ b/src/coreclr/inc/icorjitinfoimpl_generated.h @@ -710,6 +710,10 @@ uint32_t getJitFlags( CORJIT_FLAGS* flags, uint32_t sizeInBytes) override; +bool doesFieldBelongToClass( + CORINFO_FIELD_HANDLE fldHnd, + CORINFO_CLASS_HANDLE cls) override; + /**********************************************************************************/ // clang-format on /**********************************************************************************/ diff --git a/src/coreclr/inc/jiteeversionguid.h b/src/coreclr/inc/jiteeversionguid.h index f2d6921bc76cd..f78582151fc69 100644 --- a/src/coreclr/inc/jiteeversionguid.h +++ b/src/coreclr/inc/jiteeversionguid.h @@ -43,12 +43,12 @@ typedef const GUID *LPCGUID; #define GUID_DEFINED #endif // !GUID_DEFINED -constexpr GUID JITEEVersionIdentifier = { /* c2e4a466-795d-4e64-a776-0ae7b2eed65b */ - 0xc2e4a466, - 0x795d, - 0x4e64, - {0xa7, 0x76, 0x0a, 0xe7, 0xb2, 0xee, 0xd6, 0x5b} - }; +constexpr GUID JITEEVersionIdentifier = { /* 5ed35c58-857b-48dd-a818-7c0136dc9f73 */ + 0x5ed35c58, + 0x857b, + 0x48dd, + {0xa8, 0x18, 0x7c, 0x01, 0x36, 0xdc, 0x9f, 0x73} +}; ////////////////////////////////////////////////////////////////////////////////////////////////////////// // diff --git a/src/coreclr/jit/ICorJitInfo_API_names.h b/src/coreclr/jit/ICorJitInfo_API_names.h index f29e37f91baae..fec125b78ae06 100644 --- a/src/coreclr/jit/ICorJitInfo_API_names.h +++ b/src/coreclr/jit/ICorJitInfo_API_names.h @@ -176,5 +176,6 @@ DEF_CLR_API(recordRelocation) DEF_CLR_API(getRelocTypeHint) DEF_CLR_API(getExpectedTargetArchitecture) DEF_CLR_API(getJitFlags) +DEF_CLR_API(doesFieldBelongToClass) #undef DEF_CLR_API diff --git a/src/coreclr/jit/ICorJitInfo_API_wrapper.hpp b/src/coreclr/jit/ICorJitInfo_API_wrapper.hpp index 66292765480a3..f902b3b99ecd6 100644 --- a/src/coreclr/jit/ICorJitInfo_API_wrapper.hpp +++ b/src/coreclr/jit/ICorJitInfo_API_wrapper.hpp @@ -1690,6 +1690,16 @@ uint32_t WrapICorJitInfo::getJitFlags( return temp; } +bool WrapICorJitInfo::doesFieldBelongToClass( + CORINFO_FIELD_HANDLE fldHnd, + CORINFO_CLASS_HANDLE cls) +{ + API_ENTER(doesFieldBelongToClass); + bool temp = wrapHnd->doesFieldBelongToClass(fldHnd, cls); + API_LEAVE(doesFieldBelongToClass); + return temp; +} + /**********************************************************************************/ // clang-format on /**********************************************************************************/ diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoBase.cs b/src/coreclr/tools/Common/JitInterface/CorInfoBase.cs index db9eb3f137a58..6fd7ca33bfb8d 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoBase.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoBase.cs @@ -2549,10 +2549,25 @@ static uint _getJitFlags(IntPtr thisHandle, IntPtr* ppException, CORJIT_FLAGS* f } } + [UnmanagedCallersOnly] + static byte _doesFieldBelongToClass(IntPtr thisHandle, IntPtr* ppException, CORINFO_FIELD_STRUCT_* fldHnd, CORINFO_CLASS_STRUCT_* cls) + { + var _this = GetThis(thisHandle); + try + { + return _this.doesFieldBelongToClass(fldHnd, cls) ? (byte)1 : (byte)0; + } + catch (Exception ex) + { + *ppException = _this.AllocException(ex); + return default; + } + } + static IntPtr GetUnmanagedCallbacks() { - void** callbacks = (void**)Marshal.AllocCoTaskMem(sizeof(IntPtr) * 172); + void** callbacks = (void**)Marshal.AllocCoTaskMem(sizeof(IntPtr) * 173); callbacks[0] = (delegate* unmanaged)&_isJitIntrinsic; callbacks[1] = (delegate* unmanaged)&_getMethodAttribs; @@ -2726,6 +2741,7 @@ static IntPtr GetUnmanagedCallbacks() callbacks[169] = (delegate* unmanaged)&_getRelocTypeHint; callbacks[170] = (delegate* unmanaged)&_getExpectedTargetArchitecture; callbacks[171] = (delegate* unmanaged)&_getJitFlags; + callbacks[172] = (delegate* unmanaged)&_doesFieldBelongToClass; return (IntPtr)callbacks; } diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs index a0ccb4400f075..415764e85fd29 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs @@ -3531,6 +3531,12 @@ private uint getExpectedTargetArchitecture() } } + private bool doesFieldBelongToClass(CORINFO_FIELD_STRUCT_* fld, CORINFO_CLASS_STRUCT_* cls) + { + // we need it to return true for both canon and not-canon classes. + return (HandleToObject(fld).OwningType == HandleToObject(cls)); + } + private bool isMethodDefinedInCoreLib() { TypeDesc owningType = MethodBeingCompiled.OwningType; diff --git a/src/coreclr/tools/Common/JitInterface/ThunkGenerator/ThunkInput.txt b/src/coreclr/tools/Common/JitInterface/ThunkGenerator/ThunkInput.txt index 22e390f52858e..da802c872bb28 100644 --- a/src/coreclr/tools/Common/JitInterface/ThunkGenerator/ThunkInput.txt +++ b/src/coreclr/tools/Common/JitInterface/ThunkGenerator/ThunkInput.txt @@ -325,4 +325,4 @@ FUNCTIONS uint16_t getRelocTypeHint(void* target) uint32_t getExpectedTargetArchitecture() uint32_t getJitFlags(CORJIT_FLAGS* flags, uint32_t sizeInBytes) - + bool doesFieldBelongToClass(CORINFO_FIELD_HANDLE fldHnd, CORINFO_CLASS_HANDLE cls) diff --git a/src/coreclr/tools/aot/jitinterface/jitinterface.h b/src/coreclr/tools/aot/jitinterface/jitinterface.h index 86591df5df32f..68493f0bdc1e1 100644 --- a/src/coreclr/tools/aot/jitinterface/jitinterface.h +++ b/src/coreclr/tools/aot/jitinterface/jitinterface.h @@ -183,6 +183,7 @@ struct JitInterfaceCallbacks uint16_t (* getRelocTypeHint)(void * thisHandle, CorInfoExceptionClass** ppException, void* target); uint32_t (* getExpectedTargetArchitecture)(void * thisHandle, CorInfoExceptionClass** ppException); uint32_t (* getJitFlags)(void * thisHandle, CorInfoExceptionClass** ppException, CORJIT_FLAGS* flags, uint32_t sizeInBytes); + bool (* doesFieldBelongToClass)(void * thisHandle, CorInfoExceptionClass** ppException, CORINFO_FIELD_HANDLE fldHnd, CORINFO_CLASS_HANDLE cls); }; @@ -1857,4 +1858,14 @@ class JitInterfaceWrapper : public ICorJitInfo if (pException != nullptr) throw pException; return temp; } + + virtual bool doesFieldBelongToClass( + CORINFO_FIELD_HANDLE fldHnd, + CORINFO_CLASS_HANDLE cls) +{ + CorInfoExceptionClass* pException = nullptr; + bool temp = _callbacks->doesFieldBelongToClass(_thisHandle, &pException, fldHnd, cls); + if (pException != nullptr) throw pException; + return temp; +} }; diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 692c35eb6dae7..139ca3d54edf0 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -11946,6 +11946,29 @@ uint32_t CEEJitInfo::getExpectedTargetArchitecture() return IMAGE_FILE_MACHINE_NATIVE; } +bool CEEJitInfo::doesFieldBelongToClass(CORINFO_FIELD_HANDLE fldHnd, CORINFO_CLASS_HANDLE cls) +{ + CONTRACTL { + THROWS; + GC_TRIGGERS; + MODE_PREEMPTIVE; + } CONTRACTL_END; + + bool result; + + JIT_TO_EE_TRANSITION(); + + FieldDesc* field = (FieldDesc*) fldHnd; + TypeHandle th(cls); + + MethodTable* pMT = field->GetExactDeclaringType(th.GetMethodTable()); + result = (pMT != nullptr); + + EE_TO_JIT_TRANSITION(); + + return result; +} + void CEEInfo::JitProcessShutdownWork() { LIMITED_METHOD_CONTRACT; @@ -14770,6 +14793,12 @@ uint32_t CEEInfo::getExpectedTargetArchitecture() return IMAGE_FILE_MACHINE_NATIVE; } +bool CEEInfo::doesFieldBelongToClass(CORINFO_FIELD_HANDLE fld, CORINFO_CLASS_HANDLE cls) +{ + LIMITED_METHOD_CONTRACT; + UNREACHABLE_RET(); // only called on derived class. +} + void CEEInfo::setBoundaries(CORINFO_METHOD_HANDLE ftn, ULONG32 cMap, ICorDebugInfo::OffsetMapping *pMap) { diff --git a/src/coreclr/vm/jitinterface.h b/src/coreclr/vm/jitinterface.h index c231cc865ccae..a3378207880aa 100644 --- a/src/coreclr/vm/jitinterface.h +++ b/src/coreclr/vm/jitinterface.h @@ -693,6 +693,8 @@ class CEEJitInfo : public CEEInfo uint32_t getExpectedTargetArchitecture() override final; + bool doesFieldBelongToClass(CORINFO_FIELD_HANDLE fld, CORINFO_CLASS_HANDLE cls) override final; + void ResetForJitRetry() { CONTRACTL { From 2bd4ffd00649206b3a7fd200082ee9904a968f7e Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Fri, 13 Aug 2021 16:42:10 -0700 Subject: [PATCH 04/10] Handle generically dissimilar type concerns as well as handle derived type case in the managed compiler --- .../tools/Common/JitInterface/CorInfoImpl.cs | 34 +++++++++++++++++-- src/coreclr/vm/jitinterface.cpp | 27 +++++++++++++-- 2 files changed, 56 insertions(+), 5 deletions(-) diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs index 415764e85fd29..31825052960f4 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs @@ -3533,8 +3533,38 @@ private uint getExpectedTargetArchitecture() private bool doesFieldBelongToClass(CORINFO_FIELD_STRUCT_* fld, CORINFO_CLASS_STRUCT_* cls) { - // we need it to return true for both canon and not-canon classes. - return (HandleToObject(fld).OwningType == HandleToObject(cls)); + var field = HandleToObject(fld); + var queryType = HandleToObject(cls); + + Debug.Assert(!field.IsStatic); + + // doesFieldBelongToClass implements the predicate of... + // if field is not associated with the class in any way, return false. + // if field is the only FieldDesc that the JIT might see for a given class handle + // and logical field pair then return true. This is needed as the field handle here + // is used as a key into a hashtable mapping writes to fields to value numbers. + // + // In this implmentation this is made more complex as the JIT is exposed to CORINFO_FIELD_STRUCT + // pointers which represent exact instantions, so performing exact matching is the necessary approach + + // BaseType._field, BaseType -> true + // BaseType._field, DerivedType -> true + // BaseType<__Canon>._field, BaseType<__Canon> -> true + // BaseType<__Canon>._field, BaseType -> false + // BaseType<__Canon>._field, BaseType -> false + // BaseType._field, BaseType -> true + // BaseType._field, BaseType -> false + + var fieldOwnerType = field.OwningType; + + while (queryType != null) + { + if (fieldOwnerType == queryType) + return true; + queryType = queryType.BaseType; + } + + return false; } private bool isMethodDefinedInCoreLib() diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 139ca3d54edf0..75d3160dda7b1 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -11950,8 +11950,8 @@ bool CEEJitInfo::doesFieldBelongToClass(CORINFO_FIELD_HANDLE fldHnd, CORINFO_CLA { CONTRACTL { THROWS; - GC_TRIGGERS; - MODE_PREEMPTIVE; + GC_TRIGGERS; + MODE_PREEMPTIVE; } CONTRACTL_END; bool result; @@ -11961,8 +11961,29 @@ bool CEEJitInfo::doesFieldBelongToClass(CORINFO_FIELD_HANDLE fldHnd, CORINFO_CLA FieldDesc* field = (FieldDesc*) fldHnd; TypeHandle th(cls); + _ASSERTE(!field->IsStatic()); + + // doesFieldBelongToClass implements the predicate of... + // if field is not associated with the class in any way, return false. + // if field is the only FieldDesc that the JIT might see for a given class handle + // and logical field pair then return true. This is needed as the field handle here + // is used as a key into a hashtable mapping writes to fields to value numbers. + // + // In the CoreCLR VM implementation, verifying that the canonical MethodTable of + // the field matches the type found via GetExactDeclaringType, as all instance fields + // are only held on the canonical MethodTable. + // This yields a truth table such as + + // BaseType._field, BaseType -> true + // BaseType._field, DerivedType -> true + // BaseType<__Canon>._field, BaseType<__Canon> -> true + // BaseType<__Canon>._field, BaseType -> true + // BaseType<__Canon>._field, BaseType -> true + // BaseType._field, BaseType -> true + // BaseType._field, BaseType -> false + MethodTable* pMT = field->GetExactDeclaringType(th.GetMethodTable()); - result = (pMT != nullptr); + result = (pMT != nullptr) && (pMT->GetCanonicalMethodTable() == field->GetApproxEnclosingMethodTable()->GetCanonicalMethodTable()); EE_TO_JIT_TRANSITION(); From 207365eeb588049d0ba8963e75aaabd325de58b9 Mon Sep 17 00:00:00 2001 From: Sergey Date: Thu, 12 Aug 2021 04:00:22 -0700 Subject: [PATCH 05/10] Fix the field and Unsafe issues. --- src/coreclr/jit/importer.cpp | 39 ------------------- src/coreclr/jit/lclmorph.cpp | 75 ++++++++++++++++++++++++++++++++++-- src/coreclr/jit/valuenum.cpp | 39 ++++++++++++++----- 3 files changed, 100 insertions(+), 53 deletions(-) diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index da396f145bfe0..04e074cf44912 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -17275,45 +17275,6 @@ bool Compiler::impReturnInstruction(int prefixFlags, OPCODE& opcode) #endif } - // If we are inlining a method that returns a struct byref, check whether we are "reinterpreting" the - // struct. - GenTree* effectiveRetVal = op2->gtEffectiveVal(); - if ((returnType == TYP_BYREF) && (info.compRetType == TYP_BYREF) && - (effectiveRetVal->OperGet() == GT_ADDR)) - { - GenTree* addrChild = effectiveRetVal->gtGetOp1(); - if (addrChild->OperGet() == GT_LCL_VAR) - { - LclVarDsc* varDsc = lvaGetDesc(addrChild->AsLclVarCommon()); - - if (varTypeIsStruct(addrChild->TypeGet()) && !isOpaqueSIMDLclVar(varDsc)) - { - CORINFO_CLASS_HANDLE referentClassHandle; - CorInfoType referentType = - info.compCompHnd->getChildType(info.compMethodInfo->args.retTypeClass, - &referentClassHandle); - if (varTypeIsStruct(JITtype2varType(referentType)) && - (varDsc->GetStructHnd() != referentClassHandle)) - { - // We are returning a byref to struct1; the method signature specifies return type as - // byref - // to struct2. struct1 and struct2 are different so we are "reinterpreting" the struct. - // This may happen in, for example, System.Runtime.CompilerServices.Unsafe.As. - // We need to mark the source struct variable as having overlapping fields because its - // fields may be accessed using field handles of a different type, which may confuse - // optimizations, in particular, value numbering. - - JITDUMP("\nSetting lvOverlappingFields to true on V%02u because of struct " - "reinterpretation\n", - addrChild->AsLclVarCommon()->GetLclNum()); - - varDsc->lvOverlappingFields = true; - } - } - } - } - #ifdef DEBUG if (verbose) { diff --git a/src/coreclr/jit/lclmorph.cpp b/src/coreclr/jit/lclmorph.cpp index caa8a1bdc872d..d7f79cacea078 100644 --- a/src/coreclr/jit/lclmorph.cpp +++ b/src/coreclr/jit/lclmorph.cpp @@ -210,7 +210,7 @@ class LocalAddressVisitor final : public GenTreeVisitor // Arguments: // val - the input value // field - the FIELD node that uses the input address value - // fieldSeqStore - the compiler's field sequence store + // compiler - the compiler instance // // Return Value: // `true` if the value was consumed. `false` if the input value @@ -224,8 +224,9 @@ class LocalAddressVisitor final : public GenTreeVisitor // if the offset overflows then location is not representable, must escape // - UNKNOWN => UNKNOWN // - bool Field(Value& val, GenTreeField* field, FieldSeqStore* fieldSeqStore) + bool Field(Value& val, GenTreeField* field, Compiler* compiler) { + assert(!IsLocation() && !IsAddress()); if (val.IsLocation()) @@ -246,14 +247,80 @@ class LocalAddressVisitor final : public GenTreeVisitor m_lclNum = val.m_lclNum; m_offset = newOffset.Value(); + bool haveCorrectFieldForVN; if (field->gtFldMayOverlap) { - m_fieldSeq = FieldSeqStore::NotAField(); + haveCorrectFieldForVN = false; } else { + LclVarDsc* varDsc = compiler->lvaGetDesc(m_lclNum); + if (!varTypeIsStruct(varDsc)) + { + haveCorrectFieldForVN = false; + } + else if (FieldSeqStore::IsPseudoField(field->gtFldHnd)) + { + assert(compiler->compObjectStackAllocation()); + // We use PseudoFields when accessing stack allocated classes. + haveCorrectFieldForVN = false; + } + else if (val.m_fieldSeq == nullptr) + { + + CORINFO_CLASS_HANDLE clsHnd = varDsc->GetStructHnd(); + // If the answer is no we are probably accessing a canon type with a non-canon fldHnd, + // currently it could happen in crossgen2 scenario where VM distinguishes class._field + // from class._field. + haveCorrectFieldForVN = + compiler->info.compCompHnd->doesFieldBelongToClass(field->gtFldHnd, clsHnd); + } + else + { + FieldSeqNode* lastSeqNode = val.m_fieldSeq->GetTail(); + assert(lastSeqNode != nullptr); + if (lastSeqNode->IsPseudoField() || lastSeqNode == FieldSeqStore::NotAField()) + { + haveCorrectFieldForVN = false; + } + else + { + CORINFO_FIELD_HANDLE lastFieldBeforeTheCurrent = lastSeqNode->GetFieldHandle(); + + CORINFO_CLASS_HANDLE clsHnd; + CorInfoType fieldCorType = + compiler->info.compCompHnd->getFieldType(lastFieldBeforeTheCurrent, &clsHnd); + if (fieldCorType != CORINFO_TYPE_VALUECLASS) + { + // For example, System.IntPtr:ToInt64, when inlined, creates trees like + // * FIELD long _value + // \--* ADDR byref + // \--* FIELD long Information + // \--* ADDR byref + // \--* LCL_VAR struct V08 tmp7 + haveCorrectFieldForVN = false; + } + else + { + + haveCorrectFieldForVN = + compiler->info.compCompHnd->doesFieldBelongToClass(field->gtFldHnd, clsHnd); + assert(haveCorrectFieldForVN); + } + } + } + } + + if (haveCorrectFieldForVN) + { + FieldSeqStore* fieldSeqStore = compiler->GetFieldSeqStore(); m_fieldSeq = fieldSeqStore->Append(val.m_fieldSeq, fieldSeqStore->CreateSingleton(field->gtFldHnd)); } + else + { + m_fieldSeq = FieldSeqStore::NotAField(); + JITDUMP("Setting NotAField for [%06u],\n", compiler->dspTreeID(field)); + } } INDEBUG(val.Consume();) @@ -463,7 +530,7 @@ class LocalAddressVisitor final : public GenTreeVisitor assert(TopValue(1).Node() == node); assert(TopValue(0).Node() == node->AsField()->gtFldObj); - if (!TopValue(1).Field(TopValue(0), node->AsField(), m_compiler->GetFieldSeqStore())) + if (!TopValue(1).Field(TopValue(0), node->AsField(), m_compiler)) { // Either the address comes from a location value (e.g. FIELD(IND(...))) // or the field offset has overflowed. diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 0e924073112cd..d8d427cded76c 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -7116,6 +7116,7 @@ void Compiler::fgValueNumberBlockAssignment(GenTree* tree) unsigned lhsLclNum = lclVarTree->GetLclNum(); FieldSeqNode* lhsFldSeq = nullptr; unsigned lclDefSsaNum = GetSsaNumForLocalVarDef(lclVarTree); + LclVarDsc* lhsVarDsc = lvaGetDesc(lhsLclNum); // If it's excluded from SSA, don't need to do anything. if (lvaInSsa(lhsLclNum) && lclDefSsaNum != SsaConfig::RESERVED_SSA_NUM) { @@ -7176,9 +7177,7 @@ void Compiler::fgValueNumberBlockAssignment(GenTree* tree) } else { - rhsVNPair = lvaTable[rhsLclVarTree->GetLclNum()] - .GetPerSsaData(rhsLclVarTree->GetSsaNum()) - ->m_vnPair; + rhsVNPair = rhsVarDsc->GetPerSsaData(rhsLclVarTree->GetSsaNum())->m_vnPair; var_types indType = rhsLclVarTree->TypeGet(); rhsVNPair = vnStore->VNPairApplySelectors(rhsVNPair, rhsFldSeq, indType); @@ -7186,7 +7185,6 @@ void Compiler::fgValueNumberBlockAssignment(GenTree* tree) } else { - rhsVNPair.SetBoth(vnStore->VNForExpr(compCurBB, rhs->TypeGet())); isNewUniq = true; } } @@ -7205,9 +7203,7 @@ void Compiler::fgValueNumberBlockAssignment(GenTree* tree) } else { - rhsVNPair = lvaTable[rhsLclVarTree->GetLclNum()] - .GetPerSsaData(rhsLclVarTree->GetSsaNum()) - ->m_vnPair; + rhsVNPair = rhsVarDsc->GetPerSsaData(rhsLclVarTree->GetSsaNum())->m_vnPair; var_types indType = rhsLclVarTree->TypeGet(); rhsVNPair = vnStore->VNPairApplySelectors(rhsVNPair, rhsFldSeq, indType); @@ -7273,6 +7269,30 @@ void Compiler::fgValueNumberBlockAssignment(GenTree* tree) isNewUniq = true; } + if (!isNewUniq && (rhsVarDsc != nullptr) && (lhsVarDsc != nullptr)) + { + if ((rhsFldSeq == nullptr) && (lhsFldSeq == nullptr)) + { + if (rhsVarDsc->TypeGet() == lhsVarDsc->TypeGet()) + { + assert(rhsVarDsc->TypeGet() == TYP_STRUCT); + if (rhsVarDsc->GetStructHnd() != lhsVarDsc->GetStructHnd()) + { + // This can occur for nested structs or for unsafe casts, when we have IR like + // struct1 = struct2. + // Use an unique value number for the old map, as we don't have information about + // the dst field values using dst FIELD_HANDLE. + // Note that other asignments, like struct1 = IND struct(ADDR(LCL_VAR long)) + // will be handled in `VNPairApplySelectorsAssign`, here we care only about + // `LCL_VAR structX = (*)LCL_VAR structY` cases. + JITDUMP(" *** Different struct handles for Dst/Src of COPYBLK\n"); + assert(ClassLayout::AreCompatible(rhsVarDsc->GetLayout(), lhsVarDsc->GetLayout())); + isNewUniq = true; + } + } + } + } + if (isNewUniq) { rhsVNPair.SetBoth(vnStore->VNForExpr(compCurBB, lclVarTree->TypeGet())); @@ -7293,14 +7313,13 @@ void Compiler::fgValueNumberBlockAssignment(GenTree* tree) } else { - ValueNumPair oldLhsVNPair = - lvaTable[lhsLclNum].GetPerSsaData(lclVarTree->GetSsaNum())->m_vnPair; + ValueNumPair oldLhsVNPair = lhsVarDsc->GetPerSsaData(lclVarTree->GetSsaNum())->m_vnPair; rhsVNPair = vnStore->VNPairApplySelectorsAssign(oldLhsVNPair, lhsFldSeq, rhsVNPair, lclVarTree->TypeGet(), compCurBB); } } - lvaTable[lhsLclNum].GetPerSsaData(lclDefSsaNum)->m_vnPair = vnStore->VNPNormalPair(rhsVNPair); + lhsVarDsc->GetPerSsaData(lclDefSsaNum)->m_vnPair = vnStore->VNPNormalPair(rhsVNPair); #ifdef DEBUG if (verbose) From c4dc8bf978adfd3271f56f48b42ef0edd6e76711 Mon Sep 17 00:00:00 2001 From: Sergey Date: Sun, 15 Aug 2021 19:19:06 -0700 Subject: [PATCH 06/10] fix some failures. It appeared that we have many trees where we generate unique VN for rhs and don't inform `fgValueNumberBlockAssignment` about it. For example, for ``` N005 ( 10, 8) [000033] -A--G---R--- * ASG struct (copy) N004 ( 3, 2) [000031] D------N---- +--* LCL_VAR struct V01 loc1 d:2 N003 ( 6, 5) [000030] n---G------- \--* IND struct N002 ( 3, 3) [000043] ------------ \--* ADDR byref N001 ( 3, 2) [000044] -------N---- \--* LCL_VAR struct V00 loc0 u:6 (last use) ``` we will generate unique rhs but then `fgValueNumberBlockAssignment` will still try to work it as with a non-unique one. --- src/coreclr/jit/valuenum.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index d8d427cded76c..25c98d8af3f7b 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -7286,7 +7286,6 @@ void Compiler::fgValueNumberBlockAssignment(GenTree* tree) // will be handled in `VNPairApplySelectorsAssign`, here we care only about // `LCL_VAR structX = (*)LCL_VAR structY` cases. JITDUMP(" *** Different struct handles for Dst/Src of COPYBLK\n"); - assert(ClassLayout::AreCompatible(rhsVarDsc->GetLayout(), lhsVarDsc->GetLayout())); isNewUniq = true; } } From 8df1e815123c52c4f27d9cda3801a995203fa52a Mon Sep 17 00:00:00 2001 From: Sergey Date: Mon, 16 Aug 2021 01:50:29 -0700 Subject: [PATCH 07/10] Fix an oooold bug in VN. For a tree like: ``` N005 ( 11, 10) [001400] -A------R---- * ASG long N004 ( 6, 5) [001401] *------N----- +--* IND long N003 ( 3, 3) [001402] ------------- | \--* ADDR byref Zero Fseq[_00] N002 ( 3, 2) [001403] U------N----- | \--* LCL_VAR struct V45 tmp38 ud:3->4 N001 ( 1, 1) [001404] ------------- \--* LCL_VAR long V69 tmp62 u:2 (last use) ``` SSA was working correctly and printing that `001403` is using SSA-3 and defining SSA-4. However, this code, for some reason, was writing result as a define for SSA-3. --- src/coreclr/jit/valuenum.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 25c98d8af3f7b..4888fb6187ed6 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -8007,12 +8007,11 @@ void Compiler::fgValueNumberTree(GenTree* tree) rhsVNPair.SetBoth(vnStore->VNForExpr(compCurBB, lclVarTree->TypeGet())); } - unsigned lclDefSsaNum; + unsigned lclDefSsaNum = GetSsaNumForLocalVarDef(lclVarTree); ValueNumPair newLhsVNPair; if (isEntire) { newLhsVNPair = rhsVNPair; - lclDefSsaNum = lclVarTree->GetSsaNum(); } else { @@ -8021,7 +8020,6 @@ void Compiler::fgValueNumberTree(GenTree* tree) ValueNumPair oldLhsVNPair = lvaTable[lclVarTree->GetLclNum()] .GetPerSsaData(lclVarTree->GetSsaNum()) ->m_vnPair; - lclDefSsaNum = GetSsaNumForLocalVarDef(lclVarTree); newLhsVNPair = vnStore->VNPairApplySelectorsAssign(oldLhsVNPair, fieldSeq, rhsVNPair, lhs->TypeGet(), compCurBB); From 064193f2272a06065f0675bfcb734fa3f235f737 Mon Sep 17 00:00:00 2001 From: Sergey Date: Mon, 16 Aug 2021 10:24:36 -0700 Subject: [PATCH 08/10] Add new test cases and a fix for them. --- src/coreclr/jit/valuenum.cpp | 31 +++++++- .../JitBlue/Runtime_57282/Runtime_57282_1.cs | 78 +++++++++++++++++++ .../Runtime_57282/Runtime_57282_1.csproj | 12 +++ .../JitBlue/Runtime_57282/Runtime_57282_2.cs | 78 +++++++++++++++++++ .../Runtime_57282/Runtime_57282_2.csproj | 12 +++ 5 files changed, 208 insertions(+), 3 deletions(-) create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_57282/Runtime_57282_1.cs create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_57282/Runtime_57282_1.csproj create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_57282/Runtime_57282_2.cs create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_57282/Runtime_57282_2.csproj diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 4888fb6187ed6..ff49efd0e5eff 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -7271,12 +7271,37 @@ void Compiler::fgValueNumberBlockAssignment(GenTree* tree) if (!isNewUniq && (rhsVarDsc != nullptr) && (lhsVarDsc != nullptr)) { - if ((rhsFldSeq == nullptr) && (lhsFldSeq == nullptr)) + if (rhsLclVarTree->TypeGet() == TYP_STRUCT) { - if (rhsVarDsc->TypeGet() == lhsVarDsc->TypeGet()) + if (rhsLclVarTree->TypeGet() == lclVarTree->TypeGet()) { assert(rhsVarDsc->TypeGet() == TYP_STRUCT); - if (rhsVarDsc->GetStructHnd() != lhsVarDsc->GetStructHnd()) + assert(lhsVarDsc->TypeGet() == TYP_STRUCT); + + CORINFO_CLASS_HANDLE rhsStructHnd = NO_CLASS_HANDLE; + CORINFO_CLASS_HANDLE lhsStructHnd = NO_CLASS_HANDLE; + + if (rhsFldSeq == nullptr) + { + rhsStructHnd = rhsVarDsc->GetStructHnd(); + } + else + { + CORINFO_FIELD_HANDLE fldHnd = rhsFldSeq->GetTail()->GetFieldHandle(); + rhsStructHnd = info.compCompHnd->getFieldClass(fldHnd); + } + + if (lhsFldSeq == nullptr) + { + lhsStructHnd = lhsVarDsc->GetStructHnd(); + } + else + { + CORINFO_FIELD_HANDLE fldHnd = lhsFldSeq->GetTail()->GetFieldHandle(); + lhsStructHnd = info.compCompHnd->getFieldClass(fldHnd); + } + + if (rhsStructHnd != lhsStructHnd) { // This can occur for nested structs or for unsafe casts, when we have IR like // struct1 = struct2. diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_57282/Runtime_57282_1.cs b/src/tests/JIT/Regression/JitBlue/Runtime_57282/Runtime_57282_1.cs new file mode 100644 index 0000000000000..9a13f069e2906 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_57282/Runtime_57282_1.cs @@ -0,0 +1,78 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +// Generated by Fuzzlyn v1.2 on 2021-08-16 12:59:38 +// Run on .NET 6.0.0-dev on X64 Windows +// Seed: 9053537220764489964 +// Reduced from 104.8 KiB to 0.8 KiB in 00:00:44 +// Debug: Outputs 1, 0 +// Release: Outputs 1, 1 +struct S0 +{ + public short F1; + public ushort F4; + public S0(short f1) : this() + { + F1 = f1; + } +} + +struct S1 +{ + public S0 F0; + public S1(S0 f0) : this() + { + F0 = f0; + } +} + +struct S2 +{ + public S1 F1; + public S0 F2; + public S2(S1 f1) : this() + { + F1 = f1; + } +} + +struct S3 +{ + public S2 F0; + public S3(S2 f0) : this() + { + F0 = f0; + } +} + +struct S4 +{ + public sbyte F4; + public S3 F5; + public S4(S3 f5) : this() + { + F5 = f5; + } +} + +public class Program +{ + public static int Test() + { + S4 vr0 = new S4(new S3(new S2(new S1(new S0(1))))); + System.Console.WriteLine(vr0.F5.F0.F1.F0.F1); + System.Console.WriteLine(vr0.F5.F0.F1.F0.F4); + return vr0.F5.F0.F1.F0.F1 + vr0.F5.F0.F1.F0.F4; + } + + public static int Main() + { + if (Test() == 1) + { + return 100; + } + return 101; + + } + +} \ No newline at end of file diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_57282/Runtime_57282_1.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_57282/Runtime_57282_1.csproj new file mode 100644 index 0000000000000..f3e1cbd44b404 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_57282/Runtime_57282_1.csproj @@ -0,0 +1,12 @@ + + + Exe + + + None + True + + + + + diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_57282/Runtime_57282_2.cs b/src/tests/JIT/Regression/JitBlue/Runtime_57282/Runtime_57282_2.cs new file mode 100644 index 0000000000000..9a13f069e2906 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_57282/Runtime_57282_2.cs @@ -0,0 +1,78 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +// Generated by Fuzzlyn v1.2 on 2021-08-16 12:59:38 +// Run on .NET 6.0.0-dev on X64 Windows +// Seed: 9053537220764489964 +// Reduced from 104.8 KiB to 0.8 KiB in 00:00:44 +// Debug: Outputs 1, 0 +// Release: Outputs 1, 1 +struct S0 +{ + public short F1; + public ushort F4; + public S0(short f1) : this() + { + F1 = f1; + } +} + +struct S1 +{ + public S0 F0; + public S1(S0 f0) : this() + { + F0 = f0; + } +} + +struct S2 +{ + public S1 F1; + public S0 F2; + public S2(S1 f1) : this() + { + F1 = f1; + } +} + +struct S3 +{ + public S2 F0; + public S3(S2 f0) : this() + { + F0 = f0; + } +} + +struct S4 +{ + public sbyte F4; + public S3 F5; + public S4(S3 f5) : this() + { + F5 = f5; + } +} + +public class Program +{ + public static int Test() + { + S4 vr0 = new S4(new S3(new S2(new S1(new S0(1))))); + System.Console.WriteLine(vr0.F5.F0.F1.F0.F1); + System.Console.WriteLine(vr0.F5.F0.F1.F0.F4); + return vr0.F5.F0.F1.F0.F1 + vr0.F5.F0.F1.F0.F4; + } + + public static int Main() + { + if (Test() == 1) + { + return 100; + } + return 101; + + } + +} \ No newline at end of file diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_57282/Runtime_57282_2.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_57282/Runtime_57282_2.csproj new file mode 100644 index 0000000000000..f3e1cbd44b404 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_57282/Runtime_57282_2.csproj @@ -0,0 +1,12 @@ + + + Exe + + + None + True + + + + + From 4f85c882119139e86d9e4d84703502adf2eec357 Mon Sep 17 00:00:00 2001 From: Sergey Date: Mon, 16 Aug 2021 10:37:07 -0700 Subject: [PATCH 09/10] correct a test copy paste --- .../JitBlue/Runtime_57282/Runtime_57282_2.cs | 57 +++++++++---------- 1 file changed, 26 insertions(+), 31 deletions(-) diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_57282/Runtime_57282_2.cs b/src/tests/JIT/Regression/JitBlue/Runtime_57282/Runtime_57282_2.cs index 9a13f069e2906..fc78551f5c677 100644 --- a/src/tests/JIT/Regression/JitBlue/Runtime_57282/Runtime_57282_2.cs +++ b/src/tests/JIT/Regression/JitBlue/Runtime_57282/Runtime_57282_2.cs @@ -1,45 +1,39 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -// Generated by Fuzzlyn v1.2 on 2021-08-16 12:59:38 +// Generated by Fuzzlyn v1.2 on 2021-08-16 12:56:37 // Run on .NET 6.0.0-dev on X64 Windows -// Seed: 9053537220764489964 -// Reduced from 104.8 KiB to 0.8 KiB in 00:00:44 -// Debug: Outputs 1, 0 -// Release: Outputs 1, 1 +// Seed: 10782465293682251646 +// Reduced from 129.7 KiB to 0.8 KiB in 00:01:40 +// Debug: Outputs 0 +// Release: Outputs 1 struct S0 { + public short F0; public short F1; - public ushort F4; - public S0(short f1) : this() + public int F2; + public short F3; + public short F4; + public S0(short f4) : this() { - F1 = f1; + F4 = f4; } } struct S1 { public S0 F0; + public int F3; public S1(S0 f0) : this() { F0 = f0; } } -struct S2 -{ - public S1 F1; - public S0 F2; - public S2(S1 f1) : this() - { - F1 = f1; - } -} - struct S3 { - public S2 F0; - public S3(S2 f0) : this() + public S1 F0; + public S3(S1 f0) : this() { F0 = f0; } @@ -47,32 +41,33 @@ public S3(S2 f0) : this() struct S4 { - public sbyte F4; - public S3 F5; - public S4(S3 f5) : this() + public ushort F3; + public S3 F4; + public S4(S3 f4) : this() { - F5 = f5; + F4 = f4; } } public class Program { + static S0[] s_9 = new S0[] { new S0(0) }; + public static int Test() { - S4 vr0 = new S4(new S3(new S2(new S1(new S0(1))))); - System.Console.WriteLine(vr0.F5.F0.F1.F0.F1); - System.Console.WriteLine(vr0.F5.F0.F1.F0.F4); - return vr0.F5.F0.F1.F0.F1 + vr0.F5.F0.F1.F0.F4; + S0 vr4 = default(S0); + S4 vr5 = new S4(new S3(new S1(new S0(1)))); + s_9[0].F4 = vr5.F4.F0.F0.F4; + System.Console.WriteLine(vr4.F1); + return vr4.F1; } public static int Main() { - if (Test() == 1) + if (Test() == 0) { return 100; } return 101; - } - } \ No newline at end of file From e95814ac4f6cc812dff8f146cda848a4701be005 Mon Sep 17 00:00:00 2001 From: Sergey Date: Sun, 22 Aug 2021 23:01:22 -0700 Subject: [PATCH 10/10] response review. --- src/coreclr/jit/compiler.h | 2 + src/coreclr/jit/lclmorph.cpp | 2 +- src/coreclr/jit/valuenum.cpp | 113 ++++++++++++++++++++--------------- 3 files changed, 69 insertions(+), 48 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index f843a6233e907..b73968f84b269 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -5204,6 +5204,8 @@ class Compiler // Does value-numbering for a block assignment. void fgValueNumberBlockAssignment(GenTree* tree); + bool fgValueNumberIsStructReinterpretation(GenTreeLclVarCommon* lhsLclVarTree, GenTreeLclVarCommon* rhsLclVarTree); + // Does value-numbering for a cast tree. void fgValueNumberCastTree(GenTree* tree); diff --git a/src/coreclr/jit/lclmorph.cpp b/src/coreclr/jit/lclmorph.cpp index d7f79cacea078..0ffd46f47b784 100644 --- a/src/coreclr/jit/lclmorph.cpp +++ b/src/coreclr/jit/lclmorph.cpp @@ -305,7 +305,7 @@ class LocalAddressVisitor final : public GenTreeVisitor haveCorrectFieldForVN = compiler->info.compCompHnd->doesFieldBelongToClass(field->gtFldHnd, clsHnd); - assert(haveCorrectFieldForVN); + noway_assert(haveCorrectFieldForVN); } } } diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index ff49efd0e5eff..84a7465e33a7a 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -7116,7 +7116,7 @@ void Compiler::fgValueNumberBlockAssignment(GenTree* tree) unsigned lhsLclNum = lclVarTree->GetLclNum(); FieldSeqNode* lhsFldSeq = nullptr; unsigned lclDefSsaNum = GetSsaNumForLocalVarDef(lclVarTree); - LclVarDsc* lhsVarDsc = lvaGetDesc(lhsLclNum); + LclVarDsc* lhsVarDsc = lvaGetDesc(lhsLclNum); // If it's excluded from SSA, don't need to do anything. if (lvaInSsa(lhsLclNum) && lclDefSsaNum != SsaConfig::RESERVED_SSA_NUM) { @@ -7269,52 +7269,9 @@ void Compiler::fgValueNumberBlockAssignment(GenTree* tree) isNewUniq = true; } - if (!isNewUniq && (rhsVarDsc != nullptr) && (lhsVarDsc != nullptr)) + if (!isNewUniq && (lclVarTree != nullptr) && (rhsLclVarTree != nullptr)) { - if (rhsLclVarTree->TypeGet() == TYP_STRUCT) - { - if (rhsLclVarTree->TypeGet() == lclVarTree->TypeGet()) - { - assert(rhsVarDsc->TypeGet() == TYP_STRUCT); - assert(lhsVarDsc->TypeGet() == TYP_STRUCT); - - CORINFO_CLASS_HANDLE rhsStructHnd = NO_CLASS_HANDLE; - CORINFO_CLASS_HANDLE lhsStructHnd = NO_CLASS_HANDLE; - - if (rhsFldSeq == nullptr) - { - rhsStructHnd = rhsVarDsc->GetStructHnd(); - } - else - { - CORINFO_FIELD_HANDLE fldHnd = rhsFldSeq->GetTail()->GetFieldHandle(); - rhsStructHnd = info.compCompHnd->getFieldClass(fldHnd); - } - - if (lhsFldSeq == nullptr) - { - lhsStructHnd = lhsVarDsc->GetStructHnd(); - } - else - { - CORINFO_FIELD_HANDLE fldHnd = lhsFldSeq->GetTail()->GetFieldHandle(); - lhsStructHnd = info.compCompHnd->getFieldClass(fldHnd); - } - - if (rhsStructHnd != lhsStructHnd) - { - // This can occur for nested structs or for unsafe casts, when we have IR like - // struct1 = struct2. - // Use an unique value number for the old map, as we don't have information about - // the dst field values using dst FIELD_HANDLE. - // Note that other asignments, like struct1 = IND struct(ADDR(LCL_VAR long)) - // will be handled in `VNPairApplySelectorsAssign`, here we care only about - // `LCL_VAR structX = (*)LCL_VAR structY` cases. - JITDUMP(" *** Different struct handles for Dst/Src of COPYBLK\n"); - isNewUniq = true; - } - } - } + isNewUniq = fgValueNumberIsStructReinterpretation(lclVarTree, rhsLclVarTree); } if (isNewUniq) @@ -7381,6 +7338,67 @@ void Compiler::fgValueNumberBlockAssignment(GenTree* tree) } } +//------------------------------------------------------------------------ +// fgValueNumberIsStructReinterpretation: Checks if there is a struct reinterpretation that prevent VN propagation. +// +// Arguments: +// lhsLclVarTree - a lcl var tree on the lhs of the asg; +// rhsLclVarTree - a lcl var tree on the rhs of the asg; +// +// Return Value: +// True if the locals have different struct types and VN can't use rhs VN for lhs VN. +// False if locals have the same struct type or if this ASG is not a struct ASG. +// +bool Compiler::fgValueNumberIsStructReinterpretation(GenTreeLclVarCommon* lhsLclVarTree, + GenTreeLclVarCommon* rhsLclVarTree) +{ + assert(lhsLclVarTree != nullptr); + assert(rhsLclVarTree != nullptr); + + if (rhsLclVarTree->TypeGet() == TYP_STRUCT) + { + if (rhsLclVarTree->TypeGet() == lhsLclVarTree->TypeGet()) + { + if (lhsLclVarTree->isLclField() || rhsLclVarTree->isLclField()) + { + // Jit does not have a real support for `LCL_FLD struct [FldSeq]` because it can't determinate their + // size + // when the fieldSeq is `NotAField`, but by mistake we could have + // `BLK(ADDR byref(LCL_FLD struct Fseq[]))` nowadays in out IR. + // Generate a unique VN for now, it currently won't match the other side, + // otherwise we would not have 'OBJ struct2 (ADDR(LCL_FLD struct1))` cast. + return true; + } + + assert(lhsLclVarTree->OperIs(GT_LCL_VAR)); + assert(rhsLclVarTree->OperIs(GT_LCL_VAR)); + + const LclVarDsc* lhsVarDsc = lvaGetDesc(lhsLclVarTree); + const LclVarDsc* rhsVarDsc = lvaGetDesc(rhsLclVarTree); + assert(rhsVarDsc->TypeGet() == TYP_STRUCT); + assert(lhsVarDsc->TypeGet() == TYP_STRUCT); + + CORINFO_CLASS_HANDLE rhsStructHnd = rhsVarDsc->GetStructHnd(); + CORINFO_CLASS_HANDLE lhsStructHnd = lhsVarDsc->GetStructHnd(); + + if (rhsStructHnd != lhsStructHnd) + { + // This can occur for nested structs or for unsafe casts, when we have IR like + // struct1 = struct2. + // Use an unique value number for the old map, as we don't have information about + // the dst field values using dst FIELD_HANDLE. + // Note that other asignments, like struct1 = IND struct(ADDR(LCL_VAR long)) + // will be handled in `VNPairApplySelectorsAssign`, here we care only about + // `LCL_VAR structX = (*)LCL_VAR structY` cases. + JITDUMP(" *** Different struct handles for Dst/Src of COPYBLK\n"); + return true; + } + } + } + + return false; +} + void Compiler::fgValueNumberTree(GenTree* tree) { genTreeOps oper = tree->OperGet(); @@ -8032,7 +8050,6 @@ void Compiler::fgValueNumberTree(GenTree* tree) rhsVNPair.SetBoth(vnStore->VNForExpr(compCurBB, lclVarTree->TypeGet())); } - unsigned lclDefSsaNum = GetSsaNumForLocalVarDef(lclVarTree); ValueNumPair newLhsVNPair; if (isEntire) { @@ -8050,6 +8067,8 @@ void Compiler::fgValueNumberTree(GenTree* tree) lhs->TypeGet(), compCurBB); } + unsigned lclDefSsaNum = GetSsaNumForLocalVarDef(lclVarTree); + if (lclDefSsaNum != SsaConfig::RESERVED_SSA_NUM) { lvaTable[lclNum].GetPerSsaData(lclDefSsaNum)->m_vnPair = newLhsVNPair;