Mark operands of dead FIELD_LIST as unused

This requires fixing the side-effects check in dead code elimination.
Also, fixes gtSetFlags() to be usable from DCE in the non-legacy case.

diff --git a/src/jit/gentree.cpp b/src/jit/gentree.cpp
index 9b8f88b..937efef 100644 (file)
--- a/src/jit/gentree.cpp
+++ b/src/jit/gentree.cpp
@@ -8699,7 +8699,7 @@ bool GenTree::gtSetFlags() const
     //
     // Precondition we have a GTK_SMPOP
     //
-    if (!varTypeIsIntegralOrI(TypeGet()))
+    if (!varTypeIsIntegralOrI(TypeGet()) && (TypeGet() != TYP_VOID))
     {
         return false;
     }
@@ -8722,7 +8722,7 @@ bool GenTree::gtSetFlags() const
     }
 #else // !(defined(LEGACY_BACKEND) && !FEATURE_SET_FLAGS && defined(_TARGET_XARCH_))
 
-#if FEATURE_SET_FLAGS
+#if FEATURE_SET_FLAGS && defined(LEGACY_BACKEND)
     assert(OperIsSimple());
 #endif
     if (((gtFlags & GTF_SET_FLAGS) != 0) && (gtOper != GT_IND))

diff --git a/src/jit/lir.cpp b/src/jit/lir.cpp
index a0a265d..5a05e23 100644 (file)
--- a/src/jit/lir.cpp
+++ b/src/jit/lir.cpp
@@ -1581,6 +1581,11 @@ bool LIR::Range::CheckLIR(Compiler* compiler, bool checkUnusedValues) const
         // Verify that the node is allowed in LIR.
         assert(node->IsLIR());
 
+        // Some nodes should never be marked unused, as they must be contained in the backend.
+        // These may be marked as unused during dead code elimination traversal, but they *must* be subsequently
+        // removed.
+        assert(!node->IsUnusedValue() || !node->OperIs(GT_FIELD_LIST, GT_LIST, GT_INIT_VAL));
+
         // Verify that the REVERSE_OPS flag is not set. NOTE: if we ever decide to reuse the bit assigned to
         // GTF_REVERSE_OPS for an LIR-only flag we will need to move this check to the points at which we
         // insert nodes into an LIR range.

diff --git a/src/jit/liveness.cpp b/src/jit/liveness.cpp
index dac5a00..67d9e82 100644 (file)
--- a/src/jit/liveness.cpp
+++ b/src/jit/liveness.cpp
@@ -2225,8 +2225,11 @@ void Compiler::fgComputeLifeLIR(VARSET_TP& life, BasicBlock* block, VARSET_VALAR
                 assert(!node->OperIsLocal());
                 if (!node->IsValue() || node->IsUnusedValue())
                 {
-                    unsigned sideEffects = node->gtFlags & (GTF_SIDE_EFFECT | GTF_SET_FLAGS);
-                    if ((sideEffects == 0) || ((sideEffects == GTF_EXCEPT) && !node->OperMayThrow(this)))
+                    // We are only interested in avoiding the removal of nodes with direct side-effects
+                    // (as opposed to side effects of their children).
+                    // This default case should never include calls or assignments.
+                    assert(!node->OperRequiresAsgFlag() && !node->OperIs(GT_CALL));
+                    if (!node->gtSetFlags() && !node->OperMayThrow(this))
                     {
                         JITDUMP("Removing dead node:\n");
                         DISPNODE(node);

diff --git a/src/jit/lsraarm.cpp b/src/jit/lsraarm.cpp
index fb3df19..df4a435 100644 (file)
--- a/src/jit/lsraarm.cpp
+++ b/src/jit/lsraarm.cpp
@@ -373,8 +373,13 @@ void LinearScan::BuildNode(GenTree* tree)
             assert(info->srcCount == 2);
             break;
 
-        case GT_LIST:
         case GT_FIELD_LIST:
+            // These should always be contained. We don't correctly allocate or
+            // generate code for a non-contained GT_FIELD_LIST.
+            noway_assert(!"Non-contained GT_FIELD_LIST");
+            break;
+
+        case GT_LIST:
         case GT_ARGPLACE:
         case GT_NO_OP:
         case GT_START_NONGC:

diff --git a/src/jit/lsraarm64.cpp b/src/jit/lsraarm64.cpp
index 1f36791..6497ac8 100644 (file)
--- a/src/jit/lsraarm64.cpp
+++ b/src/jit/lsraarm64.cpp
@@ -87,8 +87,13 @@ void LinearScan::BuildNode(GenTree* tree)
             BuildStoreLoc(tree->AsLclVarCommon());
             break;
 
-        case GT_LIST:
         case GT_FIELD_LIST:
+            // These should always be contained. We don't correctly allocate or
+            // generate code for a non-contained GT_FIELD_LIST.
+            noway_assert(!"Non-contained GT_FIELD_LIST");
+            break;
+
+        case GT_LIST:
         case GT_ARGPLACE:
         case GT_NO_OP:
         case GT_START_NONGC:

diff --git a/src/jit/lsraxarch.cpp b/src/jit/lsraxarch.cpp
index b0e95ae..5abdb4b 100644 (file)
--- a/src/jit/lsraxarch.cpp
+++ b/src/jit/lsraxarch.cpp
@@ -111,8 +111,13 @@ void LinearScan::BuildNode(GenTree* tree)
             BuildStoreLoc(tree->AsLclVarCommon());
             break;
 
-        case GT_LIST:
         case GT_FIELD_LIST:
+            // These should always be contained. We don't correctly allocate or
+            // generate code for a non-contained GT_FIELD_LIST.
+            noway_assert(!"Non-contained GT_FIELD_LIST");
+            break;
+
+        case GT_LIST:
         case GT_ARGPLACE:
         case GT_NO_OP:
         case GT_START_NONGC:

diff --git a/tests/src/JIT/Regression/JitBlue/DevDiv_544983/DevDiv_544983.il b/tests/src/JIT/Regression/JitBlue/DevDiv_544983/DevDiv_544983.il
new file mode 100644 (file)
index 0000000..8ca630e
--- /dev/null
+++ b/tests/src/JIT/Regression/JitBlue/DevDiv_544983/DevDiv_544983.il
@@ -0,0 +1,151 @@
+// 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 bug that this test captures was a case where a call to a divide helper
+// was being removed as dead code, but the GT_FIELD_LIST node for the long
+// argument was left. Such nodes must always be contained, and if they become
+// dead, all of their children must be marked as unused.
+
+.assembly extern mscorlib { auto }
+.assembly extern System.Runtime { auto }
+.assembly extern System.Console { auto }
+
+.assembly DevDiv_544983 { }
+
+.class public auto ansi beforefieldinit DevDiv_544983
+       extends [System.Runtime]System.Object
+{
+  .method public hidebysig static int32 
+          Test(int32 i) cil managed noinlining
+  {
+    .locals init ([0] int32 dummy)
+        ldloc.0
+        not         
+        ldc.r8       -4.5508785095998289e-253
+        conv.r8     
+        conv.ovf.u4.un
+        conv.ovf.u8.un
+        conv.r.un   
+        ldarg        0x0
+        conv.r.un   
+        dup         
+        cgt         
+        conv.r.un   
+        div         
+        conv.u1     
+        conv.ovf.i8 
+        pop  
+        ret
+  } // end of method DevDiv_544983:Test
+
+  .method public hidebysig static int16
+          Test2(uint16 a, float32 b, int32 c, uint16 d, uint8 e, int16 f, int32 g, int64 h)
+  {
+    .locals init ([0] int64 i)
+
+        ldc.i8       2
+        stloc.0
+        ldarg.s      0x2
+        ldarg.s      0x5
+        div         
+        conv.ovf.u2.un
+        conv.ovf.u2.un
+        ldarg        0x1
+        conv.ovf.i4 
+        conv.ovf.i8.un
+        ldc.i4       0x669FC58A
+        neg         
+        shl         
+        conv.r.un   
+        ldloc.s      0x0
+        conv.ovf.i8.un
+        conv.r.un   
+        ldloc        0x0
+        conv.r.un   
+        div         
+        ceq         
+        cgt.un      
+        conv.r8     
+        conv.r.un
+        pop         
+        ldloc        0x0
+        neg         
+        not         
+        conv.i2
+        ret 
+   }
+
+  .method public hidebysig static int32  Main() cil managed
+  {
+    .entrypoint
+    .locals init ([0] int32 retVal, [1] int32 testResult)
+
+         ldc.i4.s   100
+         stloc.0
+         ldc.i4.1
+         call       int32 DevDiv_544983::Test(int32)
+         stloc.1
+         ldloc.1
+         ldc.i4.m1
+         ceq
+         brtrue.s  L2
+
+         ldstr      "Test Result = "
+         call       void [System.Console]System.Console::Write(string)
+         ldloc.1
+         call       void [System.Console]System.Console::WriteLine(int32)
+         ldstr      "FAIL"
+         call       void [System.Console]System.Console::WriteLine(string)
+         ldc.i4.m1
+         stloc.0
+
+    L2:  ldc.i4.1
+         ldc.r4     2.0
+         ldc.i4.3
+         ldc.i4.4
+         ldc.i4     5
+         ldc.i4.6
+         ldc.i4.7
+         ldc.i8     8
+         call       int16 DevDiv_544983::Test2(uint16, float32, int32, uint16, uint8, int16, int32, int64)
+         stloc.1
+         ldloc.1
+         ldc.i4.1
+         ceq
+         brtrue.s  L3
+
+         ldstr      "Test2 Result = "
+         call       void [System.Console]System.Console::Write(string)
+         ldloc.1
+         call       void [System.Console]System.Console::WriteLine(int32)
+         ldstr      "FAIL"
+         call       void [System.Console]System.Console::WriteLine(string)
+         ldc.i4.m1
+         stloc.0
+         br         L4
+
+    L3:  ldloc.0
+         ldc.i4.s   100
+         ceq
+         brfalse.s  L4
+
+         ldstr      "PASS"
+         call       void [System.Console]System.Console::WriteLine(string)
+
+    L4:  ldloc.0
+         ret
+  } // end of method DevDiv_544983:Main
+
+  .method public hidebysig specialname rtspecialname 
+          instance void  .ctor() cil managed
+  {
+    // Code size       8 (0x8)
+    .maxstack  8
+         ldarg.0
+         call       instance void [System.Runtime]System.Object::.ctor()
+         ret
+  } // end of method DevDiv_544983:.ctor
+
+} // end of class DevDiv_544983
+

diff --git a/tests/src/JIT/Regression/JitBlue/DevDiv_544983/DevDiv_544983.ilproj b/tests/src/JIT/Regression/JitBlue/DevDiv_544983/DevDiv_544983.ilproj
new file mode 100644 (file)
index 0000000..5934cf6
--- /dev/null
+++ b/tests/src/JIT/Regression/JitBlue/DevDiv_544983/DevDiv_544983.ilproj
@@ -0,0 +1,23 @@
+<?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>
+    <ProjectGuid>{95DFC527-4DC1-495E-97D7-E94EE1F7140D}</ProjectGuid>
+    <OutputType>Exe</OutputType>
+    <SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\..\</SolutionDir>
+  </PropertyGroup>
+  <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' "></PropertyGroup>
+  <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' "></PropertyGroup>
+  <PropertyGroup>
+    <DebugType>None</DebugType>
+    <Optimize>True</Optimize>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).il" />
+  </ItemGroup>
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
+  <PropertyGroup Condition=" '$(MsBuildProjectDirOverride)' != '' "></PropertyGroup>
+</Project>