From 86e279f81ce11e96204a60ff8dedb4c943c0abf0 Mon Sep 17 00:00:00 2001 From: Pat Gavlin Date: Wed, 4 Jan 2017 15:30:13 -0800 Subject: [PATCH] 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 --- src/coreclr/src/jit/gentree.cpp | 8 ++- .../JitBlue/DevDiv_359733/DevDiv_359733.il | 66 ++++++++++++++++++++++ .../JitBlue/DevDiv_359733/DevDiv_359733.ilproj | 41 ++++++++++++++ 3 files changed, 112 insertions(+), 3 deletions(-) create mode 100644 src/coreclr/tests/src/JIT/Regression/JitBlue/DevDiv_359733/DevDiv_359733.il create mode 100644 src/coreclr/tests/src/JIT/Regression/JitBlue/DevDiv_359733/DevDiv_359733.ilproj diff --git a/src/coreclr/src/jit/gentree.cpp b/src/coreclr/src/jit/gentree.cpp index 4a6cc74..dcd6038 100644 --- 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 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 index 0000000..8f0870d --- /dev/null +++ b/src/coreclr/tests/src/JIT/Regression/JitBlue/DevDiv_359733/DevDiv_359733.ilproj @@ -0,0 +1,41 @@ + + + + + Debug + AnyCPU + $(MSBuildProjectName) + 2.0 + {95DFC527-4DC1-495E-97D7-E94EE1F7140D} + Exe + Properties + 512 + {786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC} + $(ProgramFiles)\Common Files\microsoft shared\VSTT .0\UITestExtensionPackages + ..\..\ + 7a9bfb7d + + + + + + + + + False + + + + None + True + + + + + + + + + + + -- 2.7.4