Fixes for bugs in fgMorphCast and optNarrowTree. (dotnet/coreclr#18816)

The fix under NARROW_IND prevents transformation of, e.g.,
CAST      int <- ushort <- int
    CLS_VAR byte

into

CLS_VAR byte.

With the fix the CAST is not removed.

The fix under GT_CAST prevents transformation of, e.g.,

CAST      int <- ushort <- long
    CAST      long <- int
       expr short

into

expt short.

With the fix it gets transformed into

CAST      int <- ushort <- int
    expr short

Block cast optimizations in fgMorphCast if the cast expression is an
active CSE candidate.

Update cast expression value numbers when a cast is removed.

Fixes dotnet/coreclr#18238, dotnet/coreclr#18850.

No diffs in frameworks and tests (pmi and crossgen, x64 and x86), except for the added test cases.

Commit migrated from https://github.com/dotnet/coreclr/commit/4929ebab22bcef4f6ff39acbcdb333ffeb98f24a

diff --git a/src/coreclr/src/jit/morph.cpp b/src/coreclr/src/jit/morph.cpp
index 7717e3d..2e756ad 100644 (file)
--- a/src/coreclr/src/jit/morph.cpp
+++ b/src/coreclr/src/jit/morph.cpp
@@ -532,7 +532,7 @@ OPTIMIZECAST:
     /* Just in case new side effects were introduced */
     tree->gtFlags |= (oper->gtFlags & GTF_ALL_EFFECT);
 
-    if (!gtIsActiveCSE_Candidate(tree)) // tree cannot be a CSE candidate
+    if (!gtIsActiveCSE_Candidate(tree) && !gtIsActiveCSE_Candidate(oper))
     {
         srcType = oper->TypeGet();
 
@@ -753,6 +753,7 @@ 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/src/jit/optimizer.cpp b/src/coreclr/src/jit/optimizer.cpp
index 6b7c3b1..c99703c 100644 (file)
--- a/src/coreclr/src/jit/optimizer.cpp
+++ b/src/coreclr/src/jit/optimizer.cpp
@@ -5782,6 +5782,13 @@ bool Compiler::optNarrowTree(GenTree* tree, var_types srct, var_types dstt, Valu
             case GT_IND:
 
             NARROW_IND:
+
+                if ((dstSize > genTypeSize(tree->gtType)) &&
+                    (varTypeIsUnsigned(dstt) && !varTypeIsUnsigned(tree->gtType)))
+                {
+                    return false;
+                }
+
                 /* Simply change the type of the tree */
 
                 if (doit && (dstSize <= genTypeSize(tree->gtType)))
@@ -5839,9 +5846,11 @@ bool Compiler::optNarrowTree(GenTree* tree, var_types srct, var_types dstt, Valu
                     {
                         dstt = genSignedType(dstt);
 
-                        if (oprSize == dstSize)
+                        if ((oprSize == dstSize) &&
+                            ((varTypeIsUnsigned(dstt) == varTypeIsUnsigned(oprt)) || !varTypeIsSmall(dstt)))
                         {
-                            // Same size: change the CAST into a NOP
+                            // Same size and there is no signedness mismatch for small types: change the CAST
+                            // into a NOP
                             tree->ChangeOper(GT_NOP);
                             tree->gtType     = dstt;
                             tree->gtOp.gtOp2 = nullptr;
@@ -5849,8 +5858,7 @@ bool Compiler::optNarrowTree(GenTree* tree, var_types srct, var_types dstt, Valu
                         }
                         else
                         {
-                            // oprSize is smaller
-                            assert(oprSize < dstSize);
+                            // oprSize is smaller or there is a signedness mismatch for small types
 
                             // Change the CastToType in the GT_CAST node
                             tree->CastToType() = dstt;

diff --git a/src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_18238/GitHub_18238.cs b/src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_18238/GitHub_18238.cs
new file mode 100644 (file)
index 0000000..ac8e21b
--- /dev/null
+++ b/src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_18238/GitHub_18238.cs
@@ -0,0 +1,141 @@
+// 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;
+
+// Regression tests for bugs in fgMorphCast and optNarrowTree.
+
+struct S0
+{
+    public byte F0;
+    public sbyte F1;
+    public sbyte F2;
+    public S0 (byte f0, sbyte f1, sbyte f2)
+    {
+        F0 = f0;
+        F1 = f1;
+        F2 = f2;
+    }
+}
+
+struct S1
+{
+    public bool F0;
+    public short F1;
+    public S1 (short f1) : this ()
+    {
+        F1 = f1;
+        F0 = true;
+    }
+}
+
+static class GitHub_18238
+{
+    static int Main()
+    {
+        bool passed = true;
+
+        ulong result1 = Test1.Run();
+        ulong expectedResult1 = 64537;
+        if (result1 != expectedResult1)
+        {
+            passed = false;
+            Console.WriteLine(String.Format("Failed Test1: expected = {0}, actual = {1}", expectedResult1, result1));
+        }
+
+        S0 vr45 = new S0(0, 0, 0);
+        int result2 = Test2.Run(vr45);
+        int expectedResult2 = 65487;
+        if (result2 != expectedResult2) {
+            passed = false;
+            Console.WriteLine(String.Format("Failed Test2: expected = {0}, actual = {1}", expectedResult2, result2));
+        }
+
+        int result3 = Test3.Run();
+        int expectedResult3 = 65535;
+        if (result3 != expectedResult3) {
+            passed = false;
+            Console.WriteLine(String.Format("Failed Test3: expected = {0}, actual = {1}", expectedResult3, result3));
+        }
+
+        uint result4 = Test4.Run ();
+        uint expectedResult4 = 32779;
+        if (result4 != expectedResult4) {
+            passed = false;
+            Console.WriteLine (String.Format ("Failed Test4: expected = {0}, actual = {1}", expectedResult4, result4));
+        }
+
+        if (passed)
+        {
+            Console.WriteLine("PASSED");
+            return 100;
+        }
+        else
+        {
+            Console.WriteLine("FAILED");
+            return -1;
+        }
+    }
+}
+
+static class Test1
+{
+    static short s_2 = -1000;
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static ulong Run()
+    {
+        ulong var1 = (ushort)(1U ^ s_2);
+        return var1;
+    }
+}
+
+static class Test2
+{
+    static bool [] [] s_9 = new bool [] [] { new bool [] { true } };
+
+    [MethodImpl (MethodImplOptions.NoInlining)]
+    public static int Run(S0 arg1)
+    {
+        arg1 = new S0 (0, -50, 0);
+        char var0 = (char)(1U ^ arg1.F1);
+        return s_9 [0] [0] ? (int)var0 : 0;
+    }
+}
+
+static class Test3
+{
+    static int s_1 = 0xff;
+
+    [MethodImpl (MethodImplOptions.NoInlining)]
+    public static int Run()
+    {
+        int vr14 = (ushort)(sbyte)s_1;
+        return (vr14);
+    }
+}
+
+static class Test4
+{
+    static S1 s_1;
+
+    [MethodImpl (MethodImplOptions.NoInlining)]
+    public static uint Run()
+    {
+        char var0 = default(char);
+        s_1 = new S1(-32767);
+        var vr6 = s_1.F0;
+        uint result = Run1(var0, 0, (ushort)(10L | s_1.F1), vr6, s_1.F1);
+        return result;
+    }
+
+
+    [MethodImpl (MethodImplOptions.NoInlining)]
+    static uint Run1(char arg0, long arg1, uint arg2, bool arg3, short arg4)
+    {
+        return arg2;
+    }
+}
+

diff --git a/src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_18238/GitHub_18238.csproj b/src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_18238/GitHub_18238.csproj
new file mode 100644 (file)
index 0000000..e191e01
--- /dev/null
+++ b/src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_18238/GitHub_18238.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>{95DFC527-4DC1-495E-97D7-E94EE1F7140D}</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>