From b9effd47a1010533db0a8ffacb2c9cea2f5d8950 Mon Sep 17 00:00:00 2001
From: Eugene Rozenfeld <erozen@microsoft.com>
Date: Wed, 1 Aug 2018 15:48:41 -0700
Subject: [PATCH] Fix value number update in fgMorphCast.
 (dotnet/coreclr#19226)

removing GT_CAST nodes. It caused a problem for cases
where GT_CAST operand is a constant (e.g., GT_CNS_INT). In these
cases the value number shouldn't change since there is an
assumption that constant nodes have known constant value numbers.

The bug was found by ILGEN, I created a corresponding C# repro.
Without the fix this assert fires: https://github.com/dotnet/coreclr/blob/dotnet/coreclr@1f28125ad1f9975fbe68dd6839908aa6e63fc43b#gitext://gotocommit/dotnet/coreclr@1f28125ad1f9975fbe68dd6839908aa6e63fc43b/src/jit/assertionprop.cpp#L2687

The fix is to update value numbers only when we changed the operand of GT_CAST and
value number wasn't updated otherwise (e.g., in optNarrowTree).

I verified no x64 diffs in
jit-diff diff  --pmi --tests --frameworks
(with pri0 and pri1 tests).

Commit migrated from https://github.com/dotnet/coreclr/commit/fd4bd60a0b405126f0d7954861bbbc2504192bd4
---
 src/coreclr/src/jit/morph.cpp                      |  6 ++-
 .../JitBlue/DevDiv_653853/DevDiv_653853.cs         | 43 ++++++++++++++++++++++
 .../JitBlue/DevDiv_653853/DevDiv_653853.csproj     | 34 +++++++++++++++++
 3 files changed, 81 insertions(+), 2 deletions(-)
 create mode 100644 src/coreclr/tests/src/JIT/Regression/JitBlue/DevDiv_653853/DevDiv_653853.cs
 create mode 100644 src/coreclr/tests/src/JIT/Regression/JitBlue/DevDiv_653853/DevDiv_653853.csproj

diff --git a/src/coreclr/src/jit/morph.cpp b/src/coreclr/src/jit/morph.cpp
index 9b642c2..438123a 100644
--- a/src/coreclr/src/jit/morph.cpp
+++ b/src/coreclr/src/jit/morph.cpp
@@ -600,6 +600,10 @@ OPTIMIZECAST:
                             case GT_LCL_FLD:
                             case GT_ARR_ELEM:
                                 oper->gtType = dstType;
+                                // We're changing the type here so we need to update the VN;
+                                // in other cases we discard the cast without modifying oper
+                                // so the VN doesn't change.
+                                oper->SetVNsFromNode(tree);
                                 goto REMOVE_CAST;
                             default:
                                 break;
@@ -753,8 +757,6 @@ OPTIMIZECAST:
     return tree;
 
 REMOVE_CAST:
-    oper->SetVNsFromNode(tree);
-
     /* Here we've eliminated the cast, so just return it's operand */
     assert(!gtIsActiveCSE_Candidate(tree)); // tree cannot be a CSE candidate
 
diff --git a/src/coreclr/tests/src/JIT/Regression/JitBlue/DevDiv_653853/DevDiv_653853.cs b/src/coreclr/tests/src/JIT/Regression/JitBlue/DevDiv_653853/DevDiv_653853.cs
new file mode 100644
index 0000000..423acaa
--- /dev/null
+++ b/src/coreclr/tests/src/JIT/Regression/JitBlue/DevDiv_653853/DevDiv_653853.cs
@@ -0,0 +1,43 @@
+// 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;
+
+public static class Test
+{
+    public static int Main()
+    {        
+        if (RunTest(0) == 5)
+        {
+            Console.WriteLine("SUCCESS");
+            return 100;
+        }
+        else
+        {
+            Console.WriteLine("FAILURE");
+            return -1;
+        }
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    static int RunTest(int arg)
+    {
+        float f = 0.0F;
+        // The bug was that after removing the cast and
+        // replacing the right-hand side of the comparison
+        // with 0, its value number was changed to the value number
+        // of the initial right-hand side tree. That broke the
+        // assumption that constant nodes have known constant value numbers.
+        if (arg != (sbyte)f)
+        {
+            return 2*arg;
+        }
+        else
+        {
+            return arg + 5;
+        }
+    }
+}
diff --git a/src/coreclr/tests/src/JIT/Regression/JitBlue/DevDiv_653853/DevDiv_653853.csproj b/src/coreclr/tests/src/JIT/Regression/JitBlue/DevDiv_653853/DevDiv_653853.csproj
new file mode 100644
index 0000000..556ba33
--- /dev/null
+++ b/src/coreclr/tests/src/JIT/Regression/JitBlue/DevDiv_653853/DevDiv_653853.csproj
@@ -0,0 +1,34 @@
+<?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>{ADEEA3D1-B67B-456E-8F2B-6DCCACC2D34C}</ProjectGuid>
+    <OutputType>Exe</OutputType>
+    <ProjectTypeGuids>{786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids>
+    <SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\..\</SolutionDir>
+  </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>None</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>
-- 
2.7.4