Use conservative VN in CSE candidate const check

The check in CSE is supposed to leave code alone that constant prop
(done by VN-based Assertion Prop) is going to handle, but since that
constant prop code only propagates based on conservative VN, the check
in CSE needs to likewise use conservative VN to determine what to skip,
or else neither phase will eliminate the redundancy.

Fixes dotnet/coreclr#6234.

Commit migrated from https://github.com/dotnet/coreclr/commit/7350750c9eb3842b553d0b7c1ceb38da32263a02

diff --git a/src/coreclr/src/jit/optcse.cpp b/src/coreclr/src/jit/optcse.cpp
index cadb3da..bcb5a4c 100644 (file)
--- a/src/coreclr/src/jit/optcse.cpp
+++ b/src/coreclr/src/jit/optcse.cpp
@@ -724,9 +724,14 @@ unsigned Compiler::optValnumCSE_Locate()
                 }
 
                 // Don't CSE constant values, instead let the Value Number
-                // based Assertion Prop phase handle them.
+                // based Assertion Prop phase handle them.  Here, unlike
+                // the rest of optCSE, we use the conservative value number
+                // rather than the liberal one, since the conservative one
+                // is what the Value Number based Assertion Prop will use
+                // and the point is to avoid optimizing cases that it will
+                // handle.
                 //
-                if (vnStore->IsVNConstant(vnlib))
+                if (vnStore->IsVNConstant(tree->GetVN(VNK_Conservative)))
                 {
                     continue;
                 }

diff --git a/src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_6234/GitHub_6234.cs b/src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_6234/GitHub_6234.cs
new file mode 100644 (file)
index 0000000..75535b5
--- /dev/null
+++ b/src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_6234/GitHub_6234.cs
@@ -0,0 +1,31 @@
+// 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.
+
+// Repro for a failure to hoist an invariant expression due to bad
+// interaction between CSE and constant prop.
+
+using System;
+
+internal class Program
+{
+    public static int ii;
+    private static int Main(string[] args)
+    {
+        int res = 0;
+        ii = 99;
+        int b = ii;
+        int a = ii - 1;
+
+        for (int i = 0; i < b; ++i)
+        {
+            int res1 = (b + a + b * a + a * a + b * b); // large invariant expression with constant liberal VN but non-const conservative VN
+            res += res1;
+        }
+
+        // At this point, res should be 2901096
+        // Since the test needs to return 100 on success,
+        // subtract 2900996 from res.
+        return res - 2900996;
+    }
+}

diff --git a/src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_6234/GitHub_6234.csproj b/src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_6234/GitHub_6234.csproj
new file mode 100644 (file)
index 0000000..d8fc194
--- /dev/null
+++ b/src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_6234/GitHub_6234.csproj
@@ -0,0 +1,42 @@
+<?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>{213B4AD1-637F-4107-9AF5-2E938ACD737C}</ProjectGuid>
+    <OutputType>Exe</OutputType>
+    <AppDesignerFolder>Properties</AppDesignerFolder>
+    <FileAlignment>512</FileAlignment>
+    <ProjectTypeGuids>{786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids>
+    <ReferencePath>$(ProgramFiles)\Common Files\microsoft shared\VSTT\11.0\UITestExtensionPackages</ReferencePath>
+    <SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\..\</SolutionDir>
+
+    <NuGetPackageImportStamp>7a9bfb7d</NuGetPackageImportStamp>
+  </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></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>