From 6bd3330170961b571bbc736df5c7f2b3bef630e7 Mon Sep 17 00:00:00 2001 From: Sergey Andreenko Date: Fri, 8 Jun 2018 10:30:43 -0700 Subject: [PATCH] Workaround for compiler.hpp (1848) - Assertion failed 'lvRefCnt' (dotnet/coreclr#18292) * add test * clean fgRemoveStmt a bit * fix the issue (more like a workaround). Commit migrated from https://github.com/dotnet/coreclr/commit/16169a8738ab7332a0fc2f2e48937997932798ef --- src/coreclr/src/jit/assertionprop.cpp | 38 +++-- src/coreclr/src/jit/flowgraph.cpp | 27 ++- .../JitBlue/GitHub_18291/GitHub_18291.il | 189 +++++++++++++++++++++ .../JitBlue/GitHub_18291/GitHub_18291.ilproj | 34 ++++ 4 files changed, 256 insertions(+), 32 deletions(-) create mode 100644 src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_18291/GitHub_18291.il create mode 100644 src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_18291/GitHub_18291.ilproj diff --git a/src/coreclr/src/jit/assertionprop.cpp b/src/coreclr/src/jit/assertionprop.cpp index 3d0aa94..c445bdb 100644 --- a/src/coreclr/src/jit/assertionprop.cpp +++ b/src/coreclr/src/jit/assertionprop.cpp @@ -4687,7 +4687,24 @@ GenTree* Compiler::optVNConstantPropOnJTrue(BasicBlock* block, GenTree* stmt, Ge // Prepare the tree for replacement so any side effects can be extracted. GenTree* sideEffList = optPrepareTreeForReplacement(test, nullptr); - while (sideEffList) + // Transform the relop's operands to be both zeroes. + ValueNum vnZero = vnStore->VNZeroForType(TYP_INT); + relop->gtOp.gtOp1 = gtNewIconNode(0); + relop->gtOp.gtOp1->gtVNPair = ValueNumPair(vnZero, vnZero); + relop->gtOp.gtOp2 = gtNewIconNode(0); + relop->gtOp.gtOp2->gtVNPair = ValueNumPair(vnZero, vnZero); + + // Update the oper and restore the value numbers. + ValueNum vnCns = relop->gtVNPair.GetConservative(); + ValueNum vnLib = relop->gtVNPair.GetLiberal(); + bool evalsToTrue = (vnStore->CoercedConstantValue(vnCns) != 0); + relop->SetOper(evalsToTrue ? GT_EQ : GT_NE); + relop->gtVNPair = ValueNumPair(vnLib, vnCns); + + // Insert side effects back after they were removed from the JTrue stmt. + // It is important not to allow duplicates exist in the IR, that why we delete + // these side effects from the JTrue stmt before insert them back here. + while (sideEffList != nullptr) { GenTree* newStmt; if (sideEffList->OperGet() == GT_COMMA) @@ -4700,24 +4717,11 @@ GenTree* Compiler::optVNConstantPropOnJTrue(BasicBlock* block, GenTree* stmt, Ge newStmt = fgInsertStmtNearEnd(block, sideEffList); sideEffList = nullptr; } - + // fgMorphBlockStmt could potentially affect stmts after the current one, + // for example when it decides to fgRemoveRestOfBlock. fgMorphBlockStmt(block, newStmt->AsStmt() DEBUGARG(__FUNCTION__)); } - // Transform the relop's operands to be both zeroes. - ValueNum vnZero = vnStore->VNZeroForType(TYP_INT); - relop->gtOp.gtOp1 = gtNewIconNode(0); - relop->gtOp.gtOp1->gtVNPair = ValueNumPair(vnZero, vnZero); - relop->gtOp.gtOp2 = gtNewIconNode(0); - relop->gtOp.gtOp2->gtVNPair = ValueNumPair(vnZero, vnZero); - - // Update the oper and restore the value numbers. - ValueNum vnCns = relop->gtVNPair.GetConservative(); - ValueNum vnLib = relop->gtVNPair.GetLiberal(); - bool evalsToTrue = vnStore->CoercedConstantValue(vnCns) != 0; - relop->SetOper(evalsToTrue ? GT_EQ : GT_NE); - relop->gtVNPair = ValueNumPair(vnLib, vnCns); - return test; } @@ -4821,6 +4825,8 @@ Compiler::fgWalkResult Compiler::optVNConstantPropCurStmt(BasicBlock* block, Gen // Successful propagation, mark as assertion propagated and skip // sub-tree (with side-effects) visits. + // TODO #18291: at that moment stmt could be already removed from the stmt list. + optAssertionProp_Update(newTree, tree, stmt); JITDUMP("After constant propagation on [%06u]:\n", tree->gtTreeID); diff --git a/src/coreclr/src/jit/flowgraph.cpp b/src/coreclr/src/jit/flowgraph.cpp index 3d5c37b..55601f6 100644 --- a/src/coreclr/src/jit/flowgraph.cpp +++ b/src/coreclr/src/jit/flowgraph.cpp @@ -9918,10 +9918,8 @@ void Compiler::fgRemoveStmt(BasicBlock* block, statement boundaries. Or should we leave a GT_NO_OP in its place? */ } - /* Is it the first statement in the list? */ - GenTreeStmt* firstStmt = block->firstStmt(); - if (firstStmt == stmt) + if (firstStmt == stmt) // Is it the first statement in the list? { if (firstStmt->gtNext == nullptr) { @@ -9935,26 +9933,21 @@ void Compiler::fgRemoveStmt(BasicBlock* block, block->bbTreeList = tree->gtNext; block->bbTreeList->gtPrev = tree->gtPrev; } - goto DONE; } - - /* Is it the last statement in the list? */ - - if (stmt == block->lastStmt()) + else if (stmt == block->lastStmt()) // Is it the last statement in the list? { stmt->gtPrev->gtNext = nullptr; block->bbTreeList->gtPrev = stmt->gtPrev; - goto DONE; } + else // The statement is in the middle. + { + assert(stmt->gtPrevStmt != nullptr && stmt->gtNext != nullptr); - tree = stmt->gtPrevStmt; - noway_assert(tree); - - tree->gtNext = stmt->gtNext; - stmt->gtNext->gtPrev = tree; + tree = stmt->gtPrevStmt; -DONE: - fgStmtRemoved = true; + tree->gtNext = stmt->gtNext; + stmt->gtNext->gtPrev = tree; + } noway_assert(!optValnumCSE_phase); @@ -9966,6 +9959,8 @@ DONE: } } + fgStmtRemoved = true; + #ifdef DEBUG if (verbose) { diff --git a/src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_18291/GitHub_18291.il b/src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_18291/GitHub_18291.il new file mode 100644 index 0000000..d7f1752 --- /dev/null +++ b/src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_18291/GitHub_18291.il @@ -0,0 +1,189 @@ +// 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 test showed an error in `Compiler::optVNConstantPropOnJTrue`, that +// tried to remove the same tree twice. `IL_0083: brtrue` has known value +// and its side effects is extracted in a separate statement that is placed +// before the jump, but if this statement has unconditional throw call, then +// it tries to remove the jump during its optimization. + +.assembly extern System.Runtime +{ +} +.assembly extern System.Runtime.Extensions +{ +} +.assembly GitHub_18291 +{ +} + +.class private auto ansi beforefieldinit GitHub_18291 + extends [System.Runtime]System.Object +{ + .method static uint16 ILGEN_METHOD(native unsigned int, int8, native int) + { + .maxstack 339 + .locals init (char, float64) + IL_0000: ldc.i8 0xfd32d272ad594ff4 + IL_0009: ldc.i8 0x64c364b9c2b819ca + IL_0012: rem + IL_0013: pop + IL_0014: ldc.i8 0x1917394023ae4269 + IL_001d: ldc.i8 0x81bdec3ee0298c86 + IL_0026: ldc.i8 0xa89686c29dae8e55 + IL_002f: add.ovf.un + IL_0030: sub + IL_0031: conv.r8 + IL_0032: ldc.r8 float64(0xfb02a0dfac22a4d8) + IL_003b: pop + IL_003c: neg + IL_003d: ckfinite + IL_003e: neg + IL_003f: ldc.i8 0x8e6b6f2945ce019e + IL_0048: conv.r8 + IL_0049: pop + IL_004a: ldloc.s 0x01 + IL_004c: ldloc 0x0001 + IL_0050: mul + IL_0051: clt.un + IL_0053: ldc.i8 0xfc97bfa772027108 + IL_005c: ldc.i8 0x233cd8b922d142f1 + IL_0065: clt.un + IL_0067: ldloc 0x0001 + IL_006b: ldc.r8 float64(0x895c435fe82a3459) + IL_0074: cgt.un + IL_0076: ldloc 0x0001 + IL_007a: ldloc.s 0x01 + IL_007c: clt.un + IL_007e: cgt + IL_0080: shr.un + IL_0081: mul + IL_0082: not + IL_0083: brtrue + IL_00f7 + IL_0088: ldc.i8 0xb04a6130e573d9b7 + IL_0091: ldc.i8 0x30abe634c9c86493 + IL_009a: div.un + IL_009b: ldc.i8 0x3e2e26d76c796837 + IL_00a4: ldc.i8 0x80df80b810563352 + IL_00ad: cgt + IL_00af: ldarg 0x0000 + IL_00b3: ldloc.s 0x01 + IL_00b5: ldloc 0x0001 + IL_00b9: clt.un + IL_00bb: xor + IL_00bc: shr + IL_00bd: conv.i2 + IL_00be: shr + IL_00bf: not + IL_00c0: ldc.i8 0xc4b53aec3c888995 + IL_00c9: ldc.i8 0xa56d8ec44e7a1509 + IL_00d2: ceq + IL_00d4: neg + IL_00d5: ldc.i8 0x327f25ebf903a46f + IL_00de: conv.ovf.u4 + IL_00df: neg + IL_00e0: ldarg.s 0x01 + IL_00e2: not + IL_00e3: ldarg.s 0x00 + IL_00e5: shr.un + IL_00e6: sub.ovf.un + IL_00e7: mul.ovf.un + IL_00e8: conv.ovf.i8.un + IL_00e9: add.ovf.un + IL_00ea: nop + IL_00eb: ldc.i8 0x76e238de4bd30317 + IL_00f4: div + IL_00f5: conv.r.un + IL_00f6: pop + IL_00f7: ldarg.s 0x00 + IL_00f9: conv.ovf.u1.un + IL_00fa: ldarg.s 0x02 + IL_00fc: ldc.r8 float64(0x2987021098838673) + IL_0105: conv.ovf.u1.un + IL_0106: shl + IL_0107: not + IL_0108: ceq + IL_010a: ldc.i4 0x280ab454 + IL_010f: conv.ovf.i2.un + IL_0110: ldc.i8 0x92a914fa3ddce305 + IL_0119: ldc.i8 0xc0bc06b564a01c98 + IL_0122: and + IL_0123: not + IL_0124: ldloc.s 0x01 + IL_0126: ckfinite + IL_0127: conv.ovf.u8 + IL_0128: cgt + IL_012a: shl + IL_012b: conv.r.un + IL_012c: conv.r8 + IL_012d: ckfinite + IL_012e: conv.r.un + IL_012f: ldc.r8 float64(0x0274f44dfcbb9658) + IL_0138: ldloc 0x0001 + IL_013c: rem + IL_013d: neg + IL_013e: clt + IL_0140: ldarg 0x0001 + IL_0144: neg + IL_0145: ldc.i8 0x576c1f39a5a88241 + IL_014e: conv.u1 + IL_014f: and + IL_0150: pop + IL_0151: shr.un + IL_0152: ret + } + + + .method private hidebysig static int32 + Main(string[] args) cil managed + { + .entrypoint + // Code size 27 (0x1b) + .maxstack 3 + .locals init (int32 V_0) + IL_0000: nop + .try + { + IL_0001: nop + IL_0002: ldc.i4.0 + IL_0003: ldc.i4.2 + IL_0004: ldc.i4.3 + IL_0005: call uint16 GitHub_18291::ILGEN_METHOD(native unsigned int, + int8, + native int) + IL_000a: pop + IL_000b: nop + IL_000c: leave.s IL_0015 + + } // end .try + catch [System.Runtime]System.Object + { + IL_000e: pop + IL_000f: nop + IL_0010: ldc.i4.s 100 + IL_0012: stloc.0 + IL_0013: leave.s IL_0019 + + } // end handler + IL_0015: ldc.i4.m1 + IL_0016: stloc.0 + IL_0017: br.s IL_0019 + + IL_0019: ldloc.0 + IL_001a: ret + } // end of method Program::Main + + .method public hidebysig specialname rtspecialname + instance void .ctor() cil managed + { + // Code size 8 (0x8) + .maxstack 8 + IL_0000: ldarg.0 + IL_0001: call instance void [System.Runtime]System.Object::.ctor() + IL_0006: nop + IL_0007: ret + } // end of method Program::.ctor + +} // end of class GitHub_18291 \ No newline at end of file diff --git a/src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_18291/GitHub_18291.ilproj b/src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_18291/GitHub_18291.ilproj new file mode 100644 index 0000000..11ee89b --- /dev/null +++ b/src/coreclr/tests/src/JIT/Regression/JitBlue/GitHub_18291/GitHub_18291.ilproj @@ -0,0 +1,34 @@ + + + + + Debug + AnyCPU + $(MSBuildProjectName) + 2.0 + {95DFC527-4DC1-495E-97D7-E94EE1F7140D} + Exe + {786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC} + ..\..\ + + + + + + + False + + + + None + True + + + + + + + + + + -- 2.7.4