diff options
| -rw-r--r-- | src/jit/gentree.cpp | 4 | ||||
| -rw-r--r-- | src/jit/lir.cpp | 5 | ||||
| -rw-r--r-- | src/jit/liveness.cpp | 7 | ||||
| -rw-r--r-- | src/jit/lsraarm.cpp | 7 | ||||
| -rw-r--r-- | src/jit/lsraarm64.cpp | 7 | ||||
| -rw-r--r-- | src/jit/lsraxarch.cpp | 7 | ||||
| -rw-r--r-- | tests/src/JIT/Regression/JitBlue/DevDiv_544983/DevDiv_544983.il | 151 | ||||
| -rw-r--r-- | tests/src/JIT/Regression/JitBlue/DevDiv_544983/DevDiv_544983.ilproj | 23 |
8 files changed, 204 insertions, 7 deletions
diff --git a/src/jit/gentree.cpp b/src/jit/gentree.cpp index 718a566206..41733d0de8 100644 --- 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 a0a265d5ae..5a05e23c8e 100644 --- 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 5d52c397fa..7ae9487ecd 100644 --- a/src/jit/liveness.cpp +++ b/src/jit/liveness.cpp @@ -2234,8 +2234,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 fb3df19139..df4a435589 100644 --- 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 1f367915d1..6497ac877a 100644 --- 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 0f802d17fc..2d7e6564e3 100644 --- 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 index 0000000000..8ca630e1d7 --- /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 index 0000000000..5934cf63ac --- /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> |