Fix optimization of CAST(int)(LSH(long)<<CNS_INT 32+ with side effects). (#19899)
authorSergey Andreenko <seandree@microsoft.com>
Wed, 12 Sep 2018 03:32:35 +0000 (20:32 -0700)
committerGitHub <noreply@github.com>
Wed, 12 Sep 2018 03:32:35 +0000 (20:32 -0700)
* add a repro test

* Fix

src/jit/morph.cpp
tests/src/JIT/Regression/JitBlue/GitHub_19272/GitHub_19272.cs [new file with mode: 0644]
tests/src/JIT/Regression/JitBlue/GitHub_19272/GitHub_19272.csproj [new file with mode: 0644]

index 4265da2..4ddd711 100644 (file)
@@ -453,29 +453,23 @@ GenTree* Compiler::fgMorphCast(GenTree* tree)
                 {
                     const ssize_t shiftAmountValue = shiftAmount->AsIntCon()->IconValue();
 
-                    if (shiftAmountValue >= 64)
+                    if ((shiftAmountValue >= 64) || (shiftAmountValue < 0))
                     {
-                        // Shift amount is large enough that result is undefined.
-                        // Don't try and optimize.
+                        // Shift amount is large enough or negative so result is undefined.
+                        // Don't try to optimize.
                         assert(!canPushCast);
                     }
-                    else if (shiftAmountValue >= 32)
+                    else if ((shiftAmountValue >= 32) && ((tree->gtFlags & GTF_ALL_EFFECT) == 0))
                     {
                         // Result of the shift is zero.
                         DEBUG_DESTROY_NODE(tree);
                         GenTree* zero = gtNewZeroConNode(TYP_INT);
                         return fgMorphTree(zero);
                     }
-                    else if (shiftAmountValue >= 0)
-                    {
-                        // Shift amount is small enough that we can push the cast through.
-                        canPushCast = true;
-                    }
                     else
                     {
-                        // Shift amount is negative and so result is undefined.
-                        // Don't try and optimize.
-                        assert(!canPushCast);
+                        // Shift amount is positive and small enough that we can push the cast through.
+                        canPushCast = true;
                     }
                 }
                 else
@@ -489,14 +483,14 @@ GenTree* Compiler::fgMorphCast(GenTree* tree)
             {
                 DEBUG_DESTROY_NODE(tree);
 
-                // Insert narrowing casts for op1 and op2
+                // Insert narrowing casts for op1 and op2.
                 oper->gtOp.gtOp1 = gtNewCastNode(TYP_INT, oper->gtOp.gtOp1, false, dstType);
                 if (oper->gtOp.gtOp2 != nullptr)
                 {
                     oper->gtOp.gtOp2 = gtNewCastNode(TYP_INT, oper->gtOp.gtOp2, false, dstType);
                 }
 
-                // Clear the GT_MUL_64RSLT if it is set
+                // Clear the GT_MUL_64RSLT if it is set.
                 if (oper->gtOper == GT_MUL && (oper->gtFlags & GTF_MUL_64RSLT))
                 {
                     oper->gtFlags &= ~GTF_MUL_64RSLT;
diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_19272/GitHub_19272.cs b/tests/src/JIT/Regression/JitBlue/GitHub_19272/GitHub_19272.cs
new file mode 100644 (file)
index 0000000..dc46508
--- /dev/null
@@ -0,0 +1,44 @@
+// 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.
+
+// The test showed an incorrect optimization of (int)(long<<32+) when the const 32+ tree
+// had side effects.
+
+struct S0
+{
+    public ulong F0;
+    public sbyte F6;
+    public S0(sbyte f6): this() { F6 = f6; }
+}
+
+public class Program
+{
+    static S0 s_1 = new S0(127);
+    static int result = -1;
+
+    static void SetResult(ulong res)
+    {
+        result = 100;
+    }
+
+    static byte M1()
+    {
+        SetResult(s_1.F0);
+        return 0;
+    }
+
+    public static int Main()
+    {
+        int vr0 = (int)((ulong)M1() << 33) / s_1.F6;
+        if (result == 100)
+        {
+            System.Console.WriteLine("Pass");
+        }
+        else
+        {
+            System.Console.WriteLine("Failed");
+        }
+        return result;
+    }
+}
diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_19272/GitHub_19272.csproj b/tests/src/JIT/Regression/JitBlue/GitHub_19272/GitHub_19272.csproj
new file mode 100644 (file)
index 0000000..7e73aa0
--- /dev/null
@@ -0,0 +1,35 @@
+<?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>Full</DebugType>
+    <Optimize>True</Optimize>
+    <AllowUnsafeBlocks>True</AllowUnsafeBlocks>
+  </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>