From 85ed8096b302cb1790071f6cec79543fc979d6e1 Mon Sep 17 00:00:00 2001 From: Bruce Forstall Date: Wed, 22 Feb 2023 11:43:09 -0700 Subject: [PATCH] Fix issues with EH in unexecuted loops (#82329) Fix two problems with code in loops that are determined to not execute that have exception handling clauses: 1. Be more specific in loop unrolling to avoid unrolling even zero-count loops (which causes the loop to be removed) if there is a block in the loop with a different handler region than the handler region of the "top" block. This should only kick in for x86, where handlers are not extracted as funclets. 2. In Value Numbering, before actually value numbering nodes, `optComputeLoopSideEffectsOfBlock()` walks all the blocks of all top-level loops collecting data. Unfortunately, it sets the Value Number of certain nodes as a way to pass information "up the tree" during processing. Presumably these "fake" value numbers actually get replaced during actual value numbering for the tree. However, when we go to actually value number the IR, we only walk reachable blocks. Thus, in some cases, we would end up with a top-level "fake" value number on an unreachable statement root, thus leading `fgDebugCheckExceptionSets()` to check the tree and assert. To fix this, at the end of the `optComputeLoopSideEffectsOfBlock()`, clear the VN in case it was previously set. Fixes #81675 --- src/coreclr/jit/optimizer.cpp | 14 +++- .../JitBlue/Runtime_81675/Runtime_81675.cs | 70 +++++++++++++++++++ .../Runtime_81675/Runtime_81675.csproj | 9 +++ 3 files changed, 90 insertions(+), 3 deletions(-) create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_81675/Runtime_81675.cs create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_81675/Runtime_81675.csproj diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp index 8dde3b2807b0f..657aae1bbb65d 100644 --- a/src/coreclr/jit/optimizer.cpp +++ b/src/coreclr/jit/optimizer.cpp @@ -4304,17 +4304,19 @@ PhaseStatus Compiler::optUnrollLoops() // Heuristic: Estimated cost in code size of the unrolled loop. { - ClrSafeInt loopCostSz; // Cost is size of one iteration - auto tryIndex = loop.lpTop->bbTryIndex; + ClrSafeInt loopCostSz; // Cost is size of one iteration + const BasicBlock* const top = loop.lpTop; // Besides calculating the loop cost, also ensure that all loop blocks are within the same EH // region, and count the number of BBJ_RETURN blocks in the loop. loopRetCount = 0; for (BasicBlock* const block : loop.LoopBlocks()) { - if (block->bbTryIndex != tryIndex) + if (!BasicBlock::sameEHRegion(block, top)) { // Unrolling would require cloning EH regions + // Note that only non-funclet model (x86) could actually have a loop including a handler + // but not it's corresponding `try`, if its `try` was moved due to being marked "rare". JITDUMP("Failed to unroll loop " FMT_LP ": EH constraint\n", lnum); goto DONE_LOOP; } @@ -8806,6 +8808,12 @@ bool Compiler::optComputeLoopSideEffectsOfBlock(BasicBlock* blk) } } } + + // Clear the Value Number from the statement root node, if it was set above. This is to + // ensure that for those blocks that are unreachable, which we still handle in this loop + // but not during Value Numbering, fgDebugCheckExceptionSets() will skip the trees. Ideally, + // we wouldn't be touching the gtVNPair at all (here) before actual Value Numbering. + stmt->GetRootNode()->gtVNPair.SetBoth(ValueNumStore::NoVN); } if (memoryHavoc != emptyMemoryKindSet) diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_81675/Runtime_81675.cs b/src/tests/JIT/Regression/JitBlue/Runtime_81675/Runtime_81675.cs new file mode 100644 index 0000000000000..183aa1378f6c2 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_81675/Runtime_81675.cs @@ -0,0 +1,70 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +// Generated by Fuzzlyn v1.5 on 2023-02-06 00:41:34 +// Run on X86 Windows +// Seed: 12611629827253727687 +// Reduced from 444.9 KiB to 1.1 KiB in 00:13:23 +// Hits JIT assert in Release: +// Assertion failed '!"Jump into the middle of handler region"' in 'Program:Main(Fuzzlyn.ExecutionServer.IRuntime)' during 'Unroll loops' (IL size 131; hash 0xade6b36b; FullOpts) +// +// File: D:\a\_work\1\s\src\coreclr\jit\fgdiagnostic.cpp Line: 2609 +// +public interface I0 +{ +} + +public class C0 : I0 +{ + public sbyte F3; +} + +public class Runtime_81675 +{ + public static ushort s_11; + public static bool[][] s_18; + public static C0 s_26; + public static short[] s_42; + public static int Main() + { + short vr7 = default(short); + for (int vr8 = 0; vr8 < 0; vr8++) + { + try + { + System.Console.WriteLine(32767); + } + finally + { + I0 vr9 = new C0(); + } + } + + for (int vr10 = 0; vr10 < 0; vr10++) + { + if (s_18[0][0]) + { + var vr11 = s_26.F3; + } + else + { + System.Console.WriteLine(0); + } + + if (!((ushort)(-s_11++) >= 0)) + { + vr7 = vr7; + try + { + vr7 = 0; + } + finally + { + vr7 = s_42[0]; + } + } + } + + return 100; + } +} diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_81675/Runtime_81675.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_81675/Runtime_81675.csproj new file mode 100644 index 0000000000000..c0a5f3d0484e3 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_81675/Runtime_81675.csproj @@ -0,0 +1,9 @@ + + + Exe + True + + + + + \ No newline at end of file