diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 351e49636dfde..4e1c25b53520d 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -4985,6 +4985,12 @@ class Compiler BlockSet fgEnterBlks; // Set of blocks which have a special transfer of control; the "entry" blocks plus EH handler // begin blocks. +#if defined(FEATURE_EH_FUNCLETS) && defined(TARGET_ARM) + BlockSet fgAlwaysBlks; // Set of blocks which are BBJ_ALWAYS part of BBJ_CALLFINALLY/BBJ_ALWAYS pair that should + // never be removed due to a requirement to use the BBJ_ALWAYS for generating code and + // not have "retless" blocks. + +#endif // defined(FEATURE_EH_FUNCLETS) && defined(TARGET_ARM) #ifdef DEBUG bool fgReachabilitySetsValid; // Are the bbReach sets valid? diff --git a/src/coreclr/jit/fgbasic.cpp b/src/coreclr/jit/fgbasic.cpp index f3d48c8d33869..5dbf989ff86fa 100644 --- a/src/coreclr/jit/fgbasic.cpp +++ b/src/coreclr/jit/fgbasic.cpp @@ -119,6 +119,10 @@ void Compiler::fgInit() /* This is set by fgComputeReachability */ fgEnterBlks = BlockSetOps::UninitVal(); +#if defined(FEATURE_EH_FUNCLETS) && defined(TARGET_ARM) + fgAlwaysBlks = BlockSetOps::UninitVal(); +#endif // defined(FEATURE_EH_FUNCLETS) && defined(TARGET_ARM) + #ifdef DEBUG fgEnterBlksSetValid = false; #endif // DEBUG diff --git a/src/coreclr/jit/fgopt.cpp b/src/coreclr/jit/fgopt.cpp index c6f6bc8dd89cd..71daf460ca69f 100644 --- a/src/coreclr/jit/fgopt.cpp +++ b/src/coreclr/jit/fgopt.cpp @@ -297,6 +297,10 @@ void Compiler::fgComputeEnterBlocksSet() fgEnterBlks = BlockSetOps::MakeEmpty(this); +#if defined(FEATURE_EH_FUNCLETS) && defined(TARGET_ARM) + fgAlwaysBlks = BlockSetOps::MakeEmpty(this); +#endif // defined(FEATURE_EH_FUNCLETS) && defined(TARGET_ARM) + /* Now set the entry basic block */ BlockSetOps::AddElemD(this, fgEnterBlks, fgFirstBB->bbNum); assert(fgFirstBB->bbNum == 1); @@ -315,19 +319,15 @@ void Compiler::fgComputeEnterBlocksSet() } #if defined(FEATURE_EH_FUNCLETS) && defined(TARGET_ARM) - // TODO-ARM-Cleanup: The ARM code here to prevent creating retless calls by adding the BBJ_ALWAYS - // to the enter blocks is a bit of a compromise, because sometimes the blocks are already reachable, - // and it messes up DFS ordering to have them marked as enter block. We should prevent the - // creation of retless calls some other way. + // For ARM code, prevent creating retless calls by adding the BBJ_ALWAYS to the "fgAlwaysBlks" list. for (BasicBlock* const block : Blocks()) { if (block->bbJumpKind == BBJ_CALLFINALLY) { assert(block->isBBCallAlwaysPair()); - // Don't remove the BBJ_ALWAYS block that is only here for the unwinder. It might be dead - // if the finally is no-return, so mark it as an entry point. - BlockSetOps::AddElemD(this, fgEnterBlks, block->bbNext->bbNum); + // Don't remove the BBJ_ALWAYS block that is only here for the unwinder. + BlockSetOps::AddElemD(this, fgAlwaysBlks, block->bbNext->bbNum); } } #endif // defined(FEATURE_EH_FUNCLETS) && defined(TARGET_ARM) @@ -401,6 +401,13 @@ bool Compiler::fgRemoveUnreachableBlocks() { goto SKIP_BLOCK; } + +#if defined(FEATURE_EH_FUNCLETS) && defined(TARGET_ARM) + if (!BlockSetOps::IsEmptyIntersection(this, fgAlwaysBlks, block->bbReach)) + { + goto SKIP_BLOCK; + } +#endif // defined(FEATURE_EH_FUNCLETS) && defined(TARGET_ARM) } // Remove all the code for the block @@ -636,23 +643,7 @@ void Compiler::fgDfsInvPostOrder() // an incoming edge into the block). assert(fgEnterBlksSetValid); -#if defined(FEATURE_EH_FUNCLETS) && defined(TARGET_ARM) - // - // BlockSetOps::UnionD(this, startNodes, fgEnterBlks); - // - // This causes problems on ARM, because we for BBJ_CALLFINALLY/BBJ_ALWAYS pairs, we add the BBJ_ALWAYS - // to the enter blocks set to prevent flow graph optimizations from removing it and creating retless call finallies - // (BBF_RETLESS_CALL). This leads to an incorrect DFS ordering in some cases, because we start the recursive walk - // from the BBJ_ALWAYS, which is reachable from other blocks. A better solution would be to change ARM to avoid - // creating retless calls in a different way, not by adding BBJ_ALWAYS to fgEnterBlks. - // - // So, let us make sure at least fgFirstBB is still there, even if it participates in a loop. - BlockSetOps::AddElemD(this, startNodes, 1); - assert(fgFirstBB->bbNum == 1); -#else BlockSetOps::UnionD(this, startNodes, fgEnterBlks); -#endif - assert(BlockSetOps::IsMember(this, startNodes, fgFirstBB->bbNum)); // Call the flowgraph DFS traversal helper. diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_59298/Runtime_59298.cs b/src/tests/JIT/Regression/JitBlue/Runtime_59298/Runtime_59298.cs new file mode 100644 index 0000000000000..1458bd4dfe47f --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_59298/Runtime_59298.cs @@ -0,0 +1,66 @@ +using System.Runtime.CompilerServices; +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// +// Note: In below test case, we do not iterate over BBJ_ALWAYS blocks while computing the +// reachability leading to assert +public class Runtime_59298 +{ + public struct S2 + { + public struct S2_D1_F1 + { + public double double_1; + } + } + static int s_int_6 = -2; + static S2 s_s2_16 = new S2(); + int int_6 = -2; + + [MethodImpl(MethodImplOptions.NoInlining)] + public int LeafMethod6() + { + return 1; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + public S2 Method0(out short p_short_1) + { + long long_7 = -1; + p_short_1 = 0; + switch (long_7) + { + case -5: + { + do + { + try + { + int_6 ^= int_6; + } + finally + { + // The expression doesn't matter, it just has to be long enough + // to have few extra blocks which we don't walk when doing inverse + // post order while computing dominance information. + long_7 &= long_7; + int_6 &= (int_6 /= (int_6 -= LeafMethod6() - int_6) + 69) / ((int_6 << (int_6 - int_6)) + (int_6 |= LeafMethod6()) + (LeafMethod6() >> s_int_6) + 62); + } + } + while (long_7 == 8); + break; + } + default: + { + break; + } + } + return s_s2_16; + } + + public static int Main(string[] args) + { + new Runtime_59298().Method0(out short s); + return s + 100; + } +} \ No newline at end of file diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_59298/Runtime_59298.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_59298/Runtime_59298.csproj new file mode 100644 index 0000000000000..f3e1cbd44b404 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_59298/Runtime_59298.csproj @@ -0,0 +1,12 @@ + + + Exe + + + None + True + + + + +