From: Jakob Botsch Nielsen Date: Tue, 26 Jun 2018 14:57:47 +0000 (+0200) Subject: Fix value numbering when selecting a constant (dotnet/coreclr#18627) X-Git-Tag: submit/tizen/20210909.063632~11030^2~4513 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=a019f994f6ce2ec09ac7b3abd1e05ba75d64c350;p=platform%2Fupstream%2Fdotnet%2Fruntime.git Fix value numbering when selecting a constant (dotnet/coreclr#18627) When applying selectors, constants were special-cased to not require any type casts. However this is wrong if a narrowing needs to be performed. Fix dotnet/coreclr#18235 Commit migrated from https://github.com/dotnet/coreclr/commit/4e237f058b12403f7ee69d62fd24053081d5fe13 --- diff --git a/src/coreclr/src/jit/valuenum.cpp b/src/coreclr/src/jit/valuenum.cpp index 6bf8429..38ef43f 100644 --- a/src/coreclr/src/jit/valuenum.cpp +++ b/src/coreclr/src/jit/valuenum.cpp @@ -2568,49 +2568,41 @@ ValueNum ValueNumStore::VNApplySelectorsTypeCheck(ValueNum elem, var_types indTy if (indType != elemTyp) { - bool isConstant = IsVNConstant(elem); - if (isConstant && (elemTyp == genActualType(indType))) + // We are trying to read from an 'elem' of type 'elemType' using 'indType' read + + size_t elemTypSize = (elemTyp == TYP_STRUCT) ? elemStructSize : genTypeSize(elemTyp); + size_t indTypeSize = genTypeSize(indType); + + if ((indType == TYP_REF) && (varTypeIsStruct(elemTyp))) { - // (i.e. We recorded a constant of TYP_INT for a TYP_BYTE field) + // indType is TYP_REF and elemTyp is TYP_STRUCT + // + // We have a pointer to a static that is a Boxed Struct + // + return elem; } - else + else if (indTypeSize > elemTypSize) { - // We are trying to read from an 'elem' of type 'elemType' using 'indType' read - - size_t elemTypSize = (elemTyp == TYP_STRUCT) ? elemStructSize : genTypeSize(elemTyp); - size_t indTypeSize = genTypeSize(indType); - - if ((indType == TYP_REF) && (varTypeIsStruct(elemTyp))) - { - // indType is TYP_REF and elemTyp is TYP_STRUCT - // - // We have a pointer to a static that is a Boxed Struct - // - return elem; - } - else if (indTypeSize > elemTypSize) - { - // Reading beyong the end of 'elem' + // Reading beyong the end of 'elem' - // return a new unique value number - elem = VNForExpr(nullptr, indType); - JITDUMP(" *** Mismatched types in VNApplySelectorsTypeCheck (reading beyond the end)\n"); - } - else if (varTypeIsStruct(indType)) - { - // indType is TYP_STRUCT + // return a new unique value number + elem = VNForExpr(nullptr, indType); + JITDUMP(" *** Mismatched types in VNApplySelectorsTypeCheck (reading beyond the end)\n"); + } + else if (varTypeIsStruct(indType)) + { + // indType is TYP_STRUCT - // return a new unique value number - elem = VNForExpr(nullptr, indType); - JITDUMP(" *** Mismatched types in VNApplySelectorsTypeCheck (indType is TYP_STRUCT)\n"); - } - else - { - // We are trying to read an 'elem' of type 'elemType' using 'indType' read + // return a new unique value number + elem = VNForExpr(nullptr, indType); + JITDUMP(" *** Mismatched types in VNApplySelectorsTypeCheck (indType is TYP_STRUCT)\n"); + } + else + { + // We are trying to read an 'elem' of type 'elemType' using 'indType' read - // insert a cast of elem to 'indType' - elem = VNForCast(elem, indType, elemTyp); - } + // insert a cast of elem to 'indType' + elem = VNForCast(elem, indType, elemTyp); } } return elem; diff --git a/src/coreclr/tests/arm/Tests.lst b/src/coreclr/tests/arm/Tests.lst index 5a251c1..9e96a1e 100644 --- a/src/coreclr/tests/arm/Tests.lst +++ b/src/coreclr/tests/arm/Tests.lst @@ -94724,3 +94724,19 @@ MaxAllowedDurationSeconds=600 Categories=EXPECTED_PASS HostStyle=0 +[GitHub_18235_1.cmd_11901] +RelativePath=JIT\Regression\JitBlue\GitHub_18235\GitHub_18235_1\GitHub_18235_1.cmd +WorkingDir=JIT\Regression\JitBlue\GitHub_18235\GitHub_18235_1 +Expected=0 +MaxAllowedDurationSeconds=600 +Categories=EXPECTED_PASS +HostStyle=0 + +[GitHub_18235_2.cmd_11902] +RelativePath=JIT\Regression\JitBlue\GitHub_18235\GitHub_18235_2\GitHub_18235_2.cmd +WorkingDir=JIT\Regression\JitBlue\GitHub_18235\GitHub_18235_2 +Expected=0 +MaxAllowedDurationSeconds=600 +Categories=EXPECTED_PASS +HostStyle=0 + diff --git a/src/coreclr/tests/arm64/Tests.lst b/src/coreclr/tests/arm64/Tests.lst index b0d7cd1..a9a7320 100644 --- a/src/coreclr/tests/arm64/Tests.lst +++ b/src/coreclr/tests/arm64/Tests.lst @@ -94748,3 +94748,18 @@ MaxAllowedDurationSeconds=600 Categories=EXPECTED_PASS HostStyle=0 +[GitHub_18235_1.cmd_12221] +RelativePath=JIT\Regression\JitBlue\GitHub_18235\GitHub_18235_1\GitHub_18235_1.cmd +WorkingDir=JIT\Regression\JitBlue\GitHub_18235\GitHub_18235_1 +Expected=0 +MaxAllowedDurationSeconds=600 +Categories=EXPECTED_PASS +HostStyle=0 + +[GitHub_18235_2.cmd_12222] +RelativePath=JIT\Regression\JitBlue\GitHub_18235\GitHub_18235_2\GitHub_18235_2.cmd +WorkingDir=JIT\Regression\JitBlue\GitHub_18235\GitHub_18235_2 +Expected=0 +MaxAllowedDurationSeconds=600 +Categories=EXPECTED_PASS +HostStyle=0 diff --git a/src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_18235/GitHub_18235_1.cs b/src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_18235/GitHub_18235_1.cs new file mode 100644 index 0000000..190d2d3 --- /dev/null +++ b/src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_18235/GitHub_18235_1.cs @@ -0,0 +1,22 @@ +// 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; + +class C0 +{ + public sbyte F; +} + +public class Program +{ + public static int Main() + { + C0 var0 = new C0 { F = -1 }; + // The JIT was giving (byte)var0.F the same value number as the -1 assigned + // above, which was causing the OR below to be discarded. + ulong var1 = (ulong)(1000 | (byte)var0.F); + return var1 == 1023 ? 100 : 0; + } +} diff --git a/src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_18235/GitHub_18235_1.csproj b/src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_18235/GitHub_18235_1.csproj new file mode 100644 index 0000000..ecc6def --- /dev/null +++ b/src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_18235/GitHub_18235_1.csproj @@ -0,0 +1,34 @@ + + + + + Debug + AnyCPU + $(MSBuildProjectName) + 2.0 + {95DFC527-4DC1-495E-97D7-E94EE1F7140D} + Exe + {786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC} + ..\..\ + + + + + + + False + + + + + True + + + + + + + + + + \ No newline at end of file diff --git a/src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_18235/GitHub_18235_2.cs b/src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_18235/GitHub_18235_2.cs new file mode 100644 index 0000000..d3280d9 --- /dev/null +++ b/src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_18235/GitHub_18235_2.cs @@ -0,0 +1,28 @@ +// 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. + +struct S0 +{ + public uint F0; + public byte F2; + public int F3; + public sbyte F5; + public long F8; +} + +public class Program +{ + static uint s_0; + public static int Main() + { + S0 vr3 = new S0(); + vr3.F0 = 0x10001; + // The JIT was giving the RHS below the same value-number as + // 0x10001 above, which was then constant propagated to + // usages of vr4, even though it should be narrowed. + uint vr4 = (ushort)vr3.F0; + s_0 = vr4; + return vr4 == 1 ? 100 : 0; + } +} diff --git a/src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_18235/GitHub_18235_2.csproj b/src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_18235/GitHub_18235_2.csproj new file mode 100644 index 0000000..ecc6def --- /dev/null +++ b/src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_18235/GitHub_18235_2.csproj @@ -0,0 +1,34 @@ + + + + + Debug + AnyCPU + $(MSBuildProjectName) + 2.0 + {95DFC527-4DC1-495E-97D7-E94EE1F7140D} + Exe + {786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC} + ..\..\ + + + + + + + False + + + + + True + + + + + + + + + + \ No newline at end of file