From 31ad45c2fd059da82855226313606d9824bae25e Mon Sep 17 00:00:00 2001
From: Andy Ayers <andya@microsoft.com>
Date: Mon, 30 Jan 2017 08:19:25 -0800
Subject: [PATCH] JIT: fixup finally target flag after finally opts run (#9171)

The finally target flag can't be incrementally maintaned during finally
opts, so flag if any of these opts run and then run a post-opts fixup pass
to properly determine the flag value.

Only impacts arm targets.

Closes #9015.
---
 src/jit/compiler.h    |  3 +++
 src/jit/flowgraph.cpp | 58 +++++++++++++++++++++++++++++++++++++++++++++------
 src/jit/morph.cpp     |  5 ++---
 3 files changed, 57 insertions(+), 9 deletions(-)

diff --git a/src/jit/compiler.h b/src/jit/compiler.h
index 71440c0..82228b6 100644
--- a/src/jit/compiler.h
+++ b/src/jit/compiler.h
@@ -3425,6 +3425,7 @@ public:
     bool fgComputePredsDone; // Have we computed the bbPreds list
     bool fgCheapPredsValid;  // Is the bbCheapPreds list valid?
     bool fgDomsComputed;     // Have we computed the dominator sets?
+    bool fgOptimizedFinally; // Did we optimize any try-finallys?
 
     bool     fgHasSwitch;  // any BBJ_SWITCH jumps?
     bool     fgHasPostfix; // any postfix ++/-- found?
@@ -3503,6 +3504,8 @@ public:
 
     void fgCleanupContinuation(BasicBlock* continuation);
 
+    void fgUpdateFinallyTargetFlags();
+
     GenTreePtr fgGetCritSectOfStaticMethod();
 
 #if !defined(_TARGET_X86_)
diff --git a/src/jit/flowgraph.cpp b/src/jit/flowgraph.cpp
index 1d49d32..d265bde 100644
--- a/src/jit/flowgraph.cpp
+++ b/src/jit/flowgraph.cpp
@@ -22703,6 +22703,7 @@ void Compiler::fgRemoveEmptyFinally()
     if (emptyCount > 0)
     {
         JITDUMP("fgRemoveEmptyFinally() removed %u try-finally clauses from %u finallys\n", emptyCount, finallyCount);
+        fgOptimizedFinally = true;
 
 #ifdef DEBUG
         if (verbose)
@@ -23010,6 +23011,7 @@ void Compiler::fgRemoveEmptyTry()
     if (emptyCount > 0)
     {
         JITDUMP("fgRemoveEmptyTry() optimized %u empty-try try-finally clauses\n", emptyCount);
+        fgOptimizedFinally = true;
 
 #ifdef DEBUG
         if (verbose)
@@ -23575,6 +23577,7 @@ void Compiler::fgCloneFinally()
     if (cloneCount > 0)
     {
         JITDUMP("fgCloneFinally() cloned %u finally handlers\n", cloneCount);
+        fgOptimizedFinally = true;
 
 #ifdef DEBUG
         if (verbose)
@@ -23796,7 +23799,11 @@ void Compiler::fgDebugCheckTryFinallyExits()
 //    a callfinally/always pair.
 //
 //    Used by finally cloning, empty try removal, and empty
-//    finally removal. Resets flags and removes finally artifacts.
+//    finally removal.
+//
+//    BBF_FINALLY_TARGET bbFlag is left unchanged by this method
+//    since it cannot be incrementally updated. Proper updates happen
+//    when fgUpdateFinallyTargetFlags runs after all finally optimizations.
 
 void Compiler::fgCleanupContinuation(BasicBlock* continuation)
 {
@@ -23805,11 +23812,6 @@ void Compiler::fgCleanupContinuation(BasicBlock* continuation)
     // always flag.
     continuation->bbFlags &= ~BBF_KEEP_BBJ_ALWAYS;
 
-#if FEATURE_EH_FUNCLETS && defined(_TARGET_ARM_)
-    // Also, clear the finally target bit for arm
-    fgClearFinallyTargetBit(continuation);
-#endif // FEATURE_EH_FUNCLETS && defined(_TARGET_ARM_)
-
 #if !FEATURE_EH_FUNCLETS
     // Remove the GT_END_LFIN from the continuation,
     // Note we only expect to see one such statement.
@@ -23827,3 +23829,47 @@ void Compiler::fgCleanupContinuation(BasicBlock* continuation)
     assert(foundEndLFin);
 #endif // !FEATURE_EH_FUNCLETS
 }
+
+//------------------------------------------------------------------------
+// fgUpdateFinallyTargetFlags: recompute BBF_FINALLY_TARGET bits for all blocks
+// after finally optimizations have run.
+
+void Compiler::fgUpdateFinallyTargetFlags()
+{
+#if FEATURE_EH_FUNCLETS && defined(_TARGET_ARM_)
+
+    // Any fixup required?
+    if (!fgOptimizedFinally)
+    {
+        JITDUMP("In fgUpdateFinallyTargetFlags - no finally opts, no fixup required\n");
+        return;
+    }
+
+    JITDUMP("In fgUpdateFinallyTargetFlags, updating finally target flag bits\n");
+
+    // Walk all blocks, and reset the target bits.
+    for (BasicBlock* block = fgFirstBB; block != nullptr; block = block->bbNext)
+    {
+        block->bbFlags &= ~BBF_FINALLY_TARGET;
+    }
+
+    // Walk all blocks again, and set the target bits.
+    for (BasicBlock* block = fgFirstBB; block != nullptr; block = block->bbNext)
+    {
+        if (block->isBBCallAlwaysPair())
+        {
+            BasicBlock* const leave        = block->bbNext;
+            BasicBlock* const continuation = leave->bbJumpDest;
+
+            if ((continuation->bbFlags & BBF_FINALLY_TARGET) == 0)
+            {
+                JITDUMP("Found callfinally BB%02u; setting finally target bit on BB%02u\n", block->bbNum,
+                        continuation->bbNum);
+
+                continuation->bbFlags |= BBF_FINALLY_TARGET;
+            }
+        }
+    }
+
+#endif // FEATURE_EH_FUNCLETS && defined(_TARGET_ARM_)
+}
diff --git a/src/jit/morph.cpp b/src/jit/morph.cpp
index 75afb03..b7f8d07 100644
--- a/src/jit/morph.cpp
+++ b/src/jit/morph.cpp
@@ -16915,8 +16915,6 @@ void Compiler::fgMorph()
     fgDebugCheckBBlist(false, false);
 #endif // DEBUG
 
-// RemoveEmptyFinally is disabled on ARM due to github issue #9013
-#ifndef _TARGET_ARM_
     fgRemoveEmptyTry();
 
     EndPhase(PHASE_EMPTY_TRY);
@@ -16928,7 +16926,8 @@ void Compiler::fgMorph()
     fgCloneFinally();
 
     EndPhase(PHASE_CLONE_FINALLY);
-#endif // _TARGET_ARM_
+
+    fgUpdateFinallyTargetFlags();
 
     /* For x64 and ARM64 we need to mark irregular parameters early so that they don't get promoted */
     fgMarkImplicitByRefArgs();
-- 
2.7.4