JIT: Allow forward sub to reorder trees throwing the same single exception (#85647)

Allow forward sub to move a tree past another tree that throws an exception provided that they both throw the same exception.

diff --git a/src/coreclr/jit/forwardsub.cpp b/src/coreclr/jit/forwardsub.cpp
index 7be0613..f186520 100644 (file)
--- a/src/coreclr/jit/forwardsub.cpp
+++ b/src/coreclr/jit/forwardsub.cpp
@@ -190,16 +190,7 @@ public:
     };
 
     ForwardSubVisitor(Compiler* compiler, unsigned lclNum, bool livenessBased)
-        : GenTreeVisitor(compiler)
-        , m_use(nullptr)
-        , m_node(nullptr)
-        , m_parentNode(nullptr)
-        , m_lclNum(lclNum)
-        , m_parentLclNum(BAD_VAR_NUM)
-        , m_useFlags(GTF_EMPTY)
-        , m_accumulatedFlags(GTF_EMPTY)
-        , m_treeSize(0)
-        , m_livenessBased(livenessBased)
+        : GenTreeVisitor(compiler), m_lclNum(lclNum), m_livenessBased(livenessBased)
     {
         LclVarDsc* dsc = compiler->lvaGetDesc(m_lclNum);
         if (dsc->lvIsStructField)
@@ -238,10 +229,11 @@ public:
 
                 if (!isDef && !isCallTarget && IsLastUse(node->AsLclVar()))
                 {
-                    m_node       = node;
-                    m_use        = use;
-                    m_useFlags   = m_accumulatedFlags;
-                    m_parentNode = parent;
+                    m_node          = node;
+                    m_use           = use;
+                    m_useFlags      = m_accumulatedFlags;
+                    m_useExceptions = m_accumulatedExceptions;
+                    m_parentNode    = parent;
                 }
             }
         }
@@ -274,6 +266,20 @@ public:
         }
 
         m_accumulatedFlags |= (node->gtFlags & GTF_GLOB_EFFECT);
+        if ((node->gtFlags & GTF_CALL) != 0)
+        {
+            m_accumulatedExceptions = ExceptionSetFlags::All;
+        }
+        else if ((node->gtFlags & GTF_EXCEPT) != 0)
+        {
+            // We can never reorder in the face of different exception types,
+            // so stop calling 'OperExceptions' once we've seen more than one
+            // different exception type.
+            if (genCountBits(static_cast<uint32_t>(m_accumulatedExceptions)) <= 1)
+            {
+                m_accumulatedExceptions |= node->OperExceptions(m_compiler);
+            }
+        }
 
         return fgWalkResult::WALK_CONTINUE;
     }
@@ -305,6 +311,23 @@ public:
         return m_useFlags;
     }
 
+    //------------------------------------------------------------------------
+    // GetExceptions: Get precise exceptions thrown by the trees executed
+    // before the use.
+    //
+    // Returns:
+    //   Exception set.
+    //
+    // Remarks:
+    //   The visitor stops tracking precise exceptions once it finds that 2 or
+    //   more different exceptions can be thrown, so this set cannot be used
+    //   for determining the precise different exceptions thrown in that case.
+    //
+    ExceptionSetFlags GetExceptions() const
+    {
+        return m_useExceptions;
+    }
+
     bool IsCallArg() const
     {
         return m_parentNode->IsCall();
@@ -366,18 +389,23 @@ public:
     }
 
 private:
-    GenTree** m_use;
-    GenTree*  m_node;
-    GenTree*  m_parentNode;
+    GenTree** m_use        = nullptr;
+    GenTree*  m_node       = nullptr;
+    GenTree*  m_parentNode = nullptr;
     unsigned  m_lclNum;
-    unsigned  m_parentLclNum;
+    unsigned  m_parentLclNum = BAD_VAR_NUM;
 #ifdef DEBUG
     unsigned m_useCount = 0;
 #endif
-    GenTreeFlags m_useFlags;
-    GenTreeFlags m_accumulatedFlags;
-    unsigned     m_treeSize;
-    bool         m_livenessBased;
+    GenTreeFlags m_useFlags         = GTF_EMPTY;
+    GenTreeFlags m_accumulatedFlags = GTF_EMPTY;
+    // Precise exceptions thrown by the nodes that were visited so far. Note
+    // that we stop updating this field once we find that two or more separate
+    // exceptions.
+    ExceptionSetFlags m_accumulatedExceptions = ExceptionSetFlags::None;
+    ExceptionSetFlags m_useExceptions         = ExceptionSetFlags::None;
+    unsigned          m_treeSize              = 0;
+    bool              m_livenessBased;
 };
 
 //------------------------------------------------------------------------
@@ -689,15 +717,48 @@ bool Compiler::fgForwardSubStatement(Statement* stmt)
     //
     // if the next tree can't change the value of fwdSubNode or be impacted by fwdSubNode effects
     //
-    const bool fwdSubNodeInvariant   = ((fwdSubNode->gtFlags & GTF_ALL_EFFECT) == 0);
-    const bool nextTreeIsPureUpToUse = ((fsv.GetFlags() & (GTF_EXCEPT | GTF_GLOB_REF | GTF_CALL)) == 0);
-    if (!fwdSubNodeInvariant && !nextTreeIsPureUpToUse)
+    if (((fwdSubNode->gtFlags & GTF_CALL) != 0) && ((fsv.GetFlags() & GTF_ALL_EFFECT) != 0))
     {
-        // Fwd sub may impact global values and or reorder exceptions...
-        //
-        JITDUMP(" potentially interacting effects\n");
+        JITDUMP(" cannot reorder call with any side effect\n");
+        return false;
+    }
+    if (((fwdSubNode->gtFlags & GTF_GLOB_REF) != 0) && ((fsv.GetFlags() & GTF_PERSISTENT_SIDE_EFFECTS) != 0))
+    {
+        JITDUMP(" cannot reorder global reference with persistent side effects\n");
         return false;
     }
+    if (((fwdSubNode->gtFlags & GTF_ORDER_SIDEEFF) != 0) &&
+        ((fsv.GetFlags() & (GTF_GLOB_REF | GTF_ORDER_SIDEEFF)) != 0))
+    {
+        JITDUMP(" cannot reorder ordering side effect with global reference/ordering side effect\n");
+        return false;
+    }
+    if ((fwdSubNode->gtFlags & GTF_EXCEPT) != 0)
+    {
+        if ((fsv.GetFlags() & GTF_PERSISTENT_SIDE_EFFECTS) != 0)
+        {
+            JITDUMP(" cannot reorder exception with persistent side effect\n");
+            return false;
+        }
+
+        if ((fsv.GetFlags() & GTF_EXCEPT) != 0)
+        {
+            assert(fsv.GetExceptions() != ExceptionSetFlags::None);
+            if (genCountBits(static_cast<uint32_t>(fsv.GetExceptions())) > 1)
+            {
+                JITDUMP(" cannot reorder different thrown exceptions\n");
+                return false;
+            }
+
+            ExceptionSetFlags fwdSubNodeExceptions = gtCollectExceptions(fwdSubNode);
+            assert(fwdSubNodeExceptions != ExceptionSetFlags::None);
+            if (fwdSubNodeExceptions != fsv.GetExceptions())
+            {
+                JITDUMP(" cannot reorder different thrown exceptions\n");
+                return false;
+            }
+        }
+    }
 
     // If we're relying on purity of fwdSubNode for legality of forward sub,
     // do some extra checks for global uses that might not be reflected in the flags.
@@ -705,7 +766,7 @@ bool Compiler::fgForwardSubStatement(Statement* stmt)
     // TODO: remove this once we can trust upstream phases and/or gtUpdateStmtSideEffects
     // to set GTF_GLOB_REF properly.
     //
-    if (fwdSubNodeInvariant && ((fsv.GetFlags() & (GTF_CALL | GTF_ASG)) != 0))
+    if ((fsv.GetFlags() & GTF_PERSISTENT_SIDE_EFFECTS) != 0)
     {
         EffectsVisitor ev(this);
         ev.WalkTree(&fwdSubNode, nullptr);
@@ -874,7 +935,7 @@ bool Compiler::fgForwardSubStatement(Statement* stmt)
         fgForwardSubUpdateLiveness(firstLcl, lhsNode->gtPrev);
     }
 
-    if (!fwdSubNodeInvariant)
+    if ((fwdSubNode->gtFlags & GTF_ALL_EFFECT) != 0)
     {
         gtUpdateStmtSideEffects(nextStmt);
     }