Reconcile handling of EH vars (#1884)

* Reconcile handling of EH vars

Modify `BasicBlock::hasEHBoundaryIn()` and `BasicBlock::hasEHBoundaryOut()` to reflect what liveness is doing, and adjust the register allocator code appropriately.

It might make sense to extract the code that's basically the same, except that the register allocator doesn't need the `finallyVars` except as validation in the `DEBUG` case.

Fix #1786

diff --git a/src/coreclr/src/jit/block.cpp b/src/coreclr/src/jit/block.cpp
index c44b262..88e1d2f 100644 (file)
--- a/src/coreclr/src/jit/block.cpp
+++ b/src/coreclr/src/jit/block.cpp
@@ -1379,6 +1379,67 @@ BasicBlock* Compiler::bbNewBasicBlock(BBjumpKinds jumpKind)
 }
 
 //------------------------------------------------------------------------
+// isBBCallAlwaysPair: Determine if this is hte first block of a BBJ_CALLFINALLY/BBJ_ALWAYS pari
+
+// Return Value:
+//    True iff "this" is the first block of a BBJ_CALLFINALLY/BBJ_ALWAYS pair
+//    -- a block corresponding to an exit from the try of a try/finally.
+//
+// Notes:
+//    In the flow graph, this becomes a block that calls the finally, and a second, immediately
+//    following empty block (in the bbNext chain) to which the finally will return, and which
+//    branches unconditionally to the next block to be executed outside the try/finally.
+//    Note that code is often generated differently than this description. For example, on ARM,
+//    the target of the BBJ_ALWAYS is loaded in LR (the return register), and a direct jump is
+//    made to the 'finally'. The effect is that the 'finally' returns directly to the target of
+//    the BBJ_ALWAYS. A "retless" BBJ_CALLFINALLY is one that has no corresponding BBJ_ALWAYS.
+//    This can happen if the finally is known to not return (e.g., it contains a 'throw'). In
+//    that case, the BBJ_CALLFINALLY flags has BBF_RETLESS_CALL set. Note that ARM never has
+//    "retless" BBJ_CALLFINALLY blocks due to a requirement to use the BBJ_ALWAYS for
+//    generating code.
+//
+bool BasicBlock::isBBCallAlwaysPair()
+{
+#if defined(FEATURE_EH_FUNCLETS) && defined(_TARGET_ARM_)
+    if (this->bbJumpKind == BBJ_CALLFINALLY)
+#else
+    if ((this->bbJumpKind == BBJ_CALLFINALLY) && !(this->bbFlags & BBF_RETLESS_CALL))
+#endif
+    {
+#if defined(FEATURE_EH_FUNCLETS) && defined(_TARGET_ARM_)
+        // On ARM, there are no retless BBJ_CALLFINALLY.
+        assert(!(this->bbFlags & BBF_RETLESS_CALL));
+#endif
+        // Some asserts that the next block is a BBJ_ALWAYS of the proper form.
+        assert(this->bbNext != nullptr);
+        assert(this->bbNext->bbJumpKind == BBJ_ALWAYS);
+        assert(this->bbNext->bbFlags & BBF_KEEP_BBJ_ALWAYS);
+        assert(this->bbNext->isEmpty());
+
+        return true;
+    }
+    else
+    {
+        return false;
+    }
+}
+
+//------------------------------------------------------------------------
+// isBBCallAlwaysPairTail: Determine if this is the last block of a BBJ_CALLFINALLY/BBJ_ALWAYS pari
+//
+// Return Value:
+//    True iff "this" is the last block of a BBJ_CALLFINALLY/BBJ_ALWAYS pair
+//    -- a block corresponding to an exit from the try of a try/finally.
+//
+// Notes:
+//    See notes on isBBCallAlwaysPair(), above.
+//
+bool BasicBlock::isBBCallAlwaysPairTail()
+{
+    return (bbPrev != nullptr) && bbPrev->isBBCallAlwaysPair();
+}
+
+//------------------------------------------------------------------------
 // hasEHBoundaryIn: Determine if this block begins at an EH boundary.
 //
 // Return Value:
@@ -1421,14 +1482,6 @@ bool BasicBlock::hasEHBoundaryIn()
 bool BasicBlock::hasEHBoundaryOut()
 {
     bool returnVal = false;
-    // If a block is marked BBF_KEEP_BBJ_ALWAYS, it is always paired with its predecessor which is an
-    // EH boundary block. It must remain empty, and we must not have any live incoming vars in registers,
-    // in particular because we can't perform resolution if there are mismatches across edges.
-    if ((bbFlags & BBF_KEEP_BBJ_ALWAYS) != 0)
-    {
-        returnVal = true;
-    }
-
     if (bbJumpKind == BBJ_EHFILTERRET)
     {
         returnVal = true;

diff --git a/src/coreclr/src/jit/block.h b/src/coreclr/src/jit/block.h
index 79e11ed..d9cf941 100644 (file)
--- a/src/coreclr/src/jit/block.h
+++ b/src/coreclr/src/jit/block.h
@@ -650,43 +650,11 @@ struct BasicBlock : private LIR::Range
     bool isValid();
 
     // Returns "true" iff "this" is the first block of a BBJ_CALLFINALLY/BBJ_ALWAYS pair --
-    // a block corresponding to an exit from the try of a try/finally.  In the flow graph,
-    // this becomes a block that calls the finally, and a second, immediately
-    // following empty block (in the bbNext chain) to which the finally will return, and which
-    // branches unconditionally to the next block to be executed outside the try/finally.
-    // Note that code is often generated differently than this description. For example, on ARM,
-    // the target of the BBJ_ALWAYS is loaded in LR (the return register), and a direct jump is
-    // made to the 'finally'. The effect is that the 'finally' returns directly to the target of
-    // the BBJ_ALWAYS. A "retless" BBJ_CALLFINALLY is one that has no corresponding BBJ_ALWAYS.
-    // This can happen if the finally is known to not return (e.g., it contains a 'throw'). In
-    // that case, the BBJ_CALLFINALLY flags has BBF_RETLESS_CALL set. Note that ARM never has
-    // "retless" BBJ_CALLFINALLY blocks due to a requirement to use the BBJ_ALWAYS for
-    // generating code.
-    bool isBBCallAlwaysPair()
-    {
-#if defined(FEATURE_EH_FUNCLETS) && defined(_TARGET_ARM_)
-        if (this->bbJumpKind == BBJ_CALLFINALLY)
-#else
-        if ((this->bbJumpKind == BBJ_CALLFINALLY) && !(this->bbFlags & BBF_RETLESS_CALL))
-#endif
-        {
-#if defined(FEATURE_EH_FUNCLETS) && defined(_TARGET_ARM_)
-            // On ARM, there are no retless BBJ_CALLFINALLY.
-            assert(!(this->bbFlags & BBF_RETLESS_CALL));
-#endif
-            // Some asserts that the next block is a BBJ_ALWAYS of the proper form.
-            assert(this->bbNext != nullptr);
-            assert(this->bbNext->bbJumpKind == BBJ_ALWAYS);
-            assert(this->bbNext->bbFlags & BBF_KEEP_BBJ_ALWAYS);
-            assert(this->bbNext->isEmpty());
-
-            return true;
-        }
-        else
-        {
-            return false;
-        }
-    }
+    // a block corresponding to an exit from the try of a try/finally.
+    bool isBBCallAlwaysPair();
+    // Returns "true" iff "this" is the last block of a BBJ_CALLFINALLY/BBJ_ALWAYS pair --
+    // a block corresponding to an exit from the try of a try/finally.
+    bool isBBCallAlwaysPairTail();
 
     BBjumpKinds bbJumpKind; // jump (if any) at the end of this block
 

diff --git a/src/coreclr/src/jit/inline.cpp b/src/coreclr/src/jit/inline.cpp
index aecf5b8..064c162 100644 (file)
--- a/src/coreclr/src/jit/inline.cpp
+++ b/src/coreclr/src/jit/inline.cpp
@@ -1679,6 +1679,12 @@ CLRRandom* InlineStrategy::GetRandom()
         if (m_Compiler->compRandomInlineStress())
         {
             externalSeed = getJitStressLevel();
+            // We can set COMPlus_JitStressModeNames without setting COMPlus_JitStress,
+            // but we need external seed to be non-zero.
+            if (externalSeed == 0)
+            {
+                externalSeed = 2;
+            }
         }
 
 #endif // DEBUG

diff --git a/src/coreclr/src/jit/lsra.cpp b/src/coreclr/src/jit/lsra.cpp
index 1a8b046..a92a56e 100644 (file)
--- a/src/coreclr/src/jit/lsra.cpp
+++ b/src/coreclr/src/jit/lsra.cpp
@@ -847,7 +847,7 @@ void LinearScan::setBlockSequence()
                     assert(!"Switch with single successor");
                 }
             }
-            if (((predBlock->bbFlags & BBF_KEEP_BBJ_ALWAYS) != 0) || (hasUniquePred && predBlock->hasEHBoundaryOut()))
+            if (block->isBBCallAlwaysPairTail() || (hasUniquePred && predBlock->hasEHBoundaryOut()))
             {
                 // Treat this as having incoming EH flow, since we can't insert resolution moves into
                 // the ALWAYS block of a BBCallAlwaysPair, and a unique pred with an EH out edge won't