Fix VSO 359733. (dotnet/coreclr#8803)

* Fix VSO 359733.

This bug was an assertion when extracting side effects that failed when
checking that the presence or absence of value numbers on the head of
the side effect list matched the presence or absence of value numbers on
the node being added to the list. This condition does not hold when
remorphing, as remorphing may create nodes without value numbers. In
this case, the new comma node will have no value numbers for
conservative correctness. The failing assertion was weakened to allow
the presence/absence of value numbers on the head of the list and the
node being added to differ during remorphing.

Commit migrated from https://github.com/dotnet/coreclr/commit/4af7d40e0eeaa961f55ea8abff2eb03fc5dcdd56

diff --git a/src/coreclr/src/jit/gentree.cpp b/src/coreclr/src/jit/gentree.cpp
index 4a6cc74..dcd6038 100644 (file)
--- a/src/coreclr/src/jit/gentree.cpp
+++ b/src/coreclr/src/jit/gentree.cpp
@@ -14411,12 +14411,14 @@ GenTreePtr Compiler::gtBuildCommaList(GenTreePtr list, GenTreePtr expr)
         result->gtFlags |= (list->gtFlags & GTF_ALL_EFFECT);
         result->gtFlags |= (expr->gtFlags & GTF_ALL_EFFECT);
 
-        // 'list' and 'expr' should have valuenumbers defined for both or for neither one
-        noway_assert(list->gtVNPair.BothDefined() == expr->gtVNPair.BothDefined());
+        // 'list' and 'expr' should have valuenumbers defined for both or for neither one (unless we are remorphing,
+        // in which case a prior transform involving either node may have discarded or otherwise invalidated the value
+        // numbers).
+        assert((list->gtVNPair.BothDefined() == expr->gtVNPair.BothDefined()) || !fgGlobalMorph);
 
         // Set the ValueNumber 'gtVNPair' for the new GT_COMMA node
         //
-        if (expr->gtVNPair.BothDefined())
+        if (list->gtVNPair.BothDefined() && expr->gtVNPair.BothDefined())
         {
             // The result of a GT_COMMA node is op2, the normal value number is op2vnp
             // But we also need to include the union of side effects from op1 and op2.

diff --git a/src/coreclr/tests/src/JIT/Regression/JitBlue/DevDiv_359733/DevDiv_359733.il b/src/coreclr/tests/src/JIT/Regression/JitBlue/DevDiv_359733/DevDiv_359733.il
new file mode 100644 (file)
index 0000000..18a712a
--- /dev/null
+++ b/src/coreclr/tests/src/JIT/Regression/JitBlue/DevDiv_359733/DevDiv_359733.il
@@ -0,0 +1,66 @@
+// 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.
+
+// This test originally triggered an assert during remorphing while performing constant propagation when
+// extracting the side effects of a discarded tree (specifically, the tree that corresponds to the xor in
+// method C::M, which transitively takes exception-producing covnersions as arguments). The assert was
+// attempting to ensure that if value numbers were present on the comma node used to hold the side effects,
+// then value numbers were also present on the expression being added to the list. This condition may be violated
+// when remorphing, however, and the assertion was appropiately weakened.
+
+.assembly extern mscorlib {}
+.assembly DevDiv_1359733 {}
+
+.class private C extends [mscorlib]System.Object
+{
+    .method private static int16 M(bool a0, int16 a1, int16 a2, int32 a3) cil managed noinlining
+    {
+        .locals init (int32 l0, int16 l1, int64 l2, int16 l3, int8 l4, float32 l5, int8 l6)
+
+        ldloc l3
+        ldloc l5
+        conv.i8
+        conv.r8
+        neg
+        conv.ovf.u1.un
+        ldloc.s l5
+        conv.ovf.i8
+        ldc.i8 0x4007ACD1
+        ldloc l6
+        shr
+        rem
+        nop
+        ldc.i8 0x21C591BD
+        neg
+        cgt.un
+        xor
+        pop
+        ret
+    }
+
+    .method private static int32 Main() cil managed
+    {
+        .entrypoint
+
+        .try
+        {
+            ldc.i4 0
+            ldc.i4 0
+            ldc.i4 0
+            ldc.i4 0
+            call int16 C::M(bool, int16, int16, int32)
+            pop
+            leave.s done
+        }
+        catch [mscorlib]System.Exception
+        {
+            pop
+            leave.s done
+        }
+
+    done:
+        ldc.i4 100
+        ret
+    }
+}

diff --git a/src/coreclr/tests/src/JIT/Regression/JitBlue/DevDiv_359733/DevDiv_359733.ilproj b/src/coreclr/tests/src/JIT/Regression/JitBlue/DevDiv_359733/DevDiv_359733.ilproj
new file mode 100644 (file)
index 0000000..8f0870d
--- /dev/null
+++ b/src/coreclr/tests/src/JIT/Regression/JitBlue/DevDiv_359733/DevDiv_359733.ilproj
@@ -0,0 +1,41 @@
+<?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>
+    <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  .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>None</DebugType>
+    <Optimize>True</Optimize>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="DevDiv_359733.il" />
+  </ItemGroup>
+  <ItemGroup>
+    <Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" />
+  </ItemGroup>
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
+  <PropertyGroup Condition=" '$(MsBuildProjectDirOverride)' != '' ">
+  </PropertyGroup> 
+</Project>