Split NumSucc/GetSucc into two versions

One set takes Compiler*, one doesn't.

It's simpler, and hopefully faster.

Commit migrated from https://github.com/dotnet/coreclr/commit/37e6f0405eda04cdc1bc39682a5d40208a6a0e55

diff --git a/src/coreclr/src/jit/block.cpp b/src/coreclr/src/jit/block.cpp
index 673eaca..8e5dc29 100644 (file)
--- a/src/coreclr/src/jit/block.cpp
+++ b/src/coreclr/src/jit/block.cpp
@@ -903,56 +903,135 @@ bool BasicBlock::bbFallsThrough()
     }
 }
 
-unsigned BasicBlock::NumSucc(Compiler* comp)
+//------------------------------------------------------------------------
+// NumSucc: Returns the count of block successors. See the declaration comment for details.
+//
+// Arguments:
+//    None.
+//
+// Return Value:
+//    Count of block successors.
+//
+unsigned BasicBlock::NumSucc()
 {
-    // As described in the spec comment of NumSucc at its declaration, whether "comp" is null determines
-    // whether NumSucc and GetSucc yield successors of finally blocks.
-
     switch (bbJumpKind)
     {
-
         case BBJ_THROW:
         case BBJ_RETURN:
+        case BBJ_EHFINALLYRET:
+        case BBJ_EHFILTERRET:
             return 0;
 
-        case BBJ_EHFILTERRET:
-            if (comp == nullptr)
+        case BBJ_CALLFINALLY:
+        case BBJ_ALWAYS:
+        case BBJ_EHCATCHRET:
+        case BBJ_LEAVE:
+        case BBJ_NONE:
+            return 1;
+
+        case BBJ_COND:
+            if (bbJumpDest == bbNext)
             {
-                return 0;
+                return 1;
             }
             else
             {
-                return 1;
+                return 2;
+            }
+
+        case BBJ_SWITCH:
+            return bbJumpSwt->bbsCount;
+
+        default:
+            unreached();
+    }
+}
+
+//------------------------------------------------------------------------
+// GetSucc: Returns the requested block successor. See the declaration comment for details.
+//
+// Arguments:
+//    i - index of successor to return. 0 <= i <= NumSucc().
+//
+// Return Value:
+//    Requested successor block
+//
+BasicBlock* BasicBlock::GetSucc(unsigned i)
+{
+    assert(i < NumSucc()); // Index bounds check.
+    switch (bbJumpKind)
+    {
+        case BBJ_CALLFINALLY:
+        case BBJ_ALWAYS:
+        case BBJ_EHCATCHRET:
+        case BBJ_LEAVE:
+            return bbJumpDest;
+
+        case BBJ_NONE:
+            return bbNext;
+
+        case BBJ_COND:
+            if (i == 0)
+            {
+                return bbNext;
+            }
+            else
+            {
+                assert(i == 1);
+                return bbJumpDest;
             }
 
+        case BBJ_SWITCH:
+            return bbJumpSwt->bbsDstTab[i];
+
+        default:
+            unreached();
+    }
+}
+
+//------------------------------------------------------------------------
+// NumSucc: Returns the count of block successors. See the declaration comment for details.
+//
+// Arguments:
+//    comp - Compiler instance
+//
+// Return Value:
+//    Count of block successors.
+//
+unsigned BasicBlock::NumSucc(Compiler* comp)
+{
+    assert(comp != nullptr);
+
+    switch (bbJumpKind)
+    {
+        case BBJ_THROW:
+        case BBJ_RETURN:
+            return 0;
+
         case BBJ_EHFINALLYRET:
         {
-            if (comp == nullptr)
+            // The first block of the handler is labelled with the catch type.
+            BasicBlock* hndBeg = comp->fgFirstBlockOfHandler(this);
+            if (hndBeg->bbCatchTyp == BBCT_FINALLY)
             {
-                return 0;
+                return comp->fgNSuccsOfFinallyRet(this);
             }
             else
             {
-                // The first block of the handler is labelled with the catch type.
-                BasicBlock* hndBeg = comp->fgFirstBlockOfHandler(this);
-                if (hndBeg->bbCatchTyp == BBCT_FINALLY)
-                {
-                    return comp->fgNSuccsOfFinallyRet(this);
-                }
-                else
-                {
-                    assert(hndBeg->bbCatchTyp == BBCT_FAULT); // We can only BBJ_EHFINALLYRET from FINALLY and FAULT.
-                    // A FAULT block has no successors.
-                    return 0;
-                }
+                assert(hndBeg->bbCatchTyp == BBCT_FAULT); // We can only BBJ_EHFINALLYRET from FINALLY and FAULT.
+                // A FAULT block has no successors.
+                return 0;
             }
         }
+
         case BBJ_CALLFINALLY:
         case BBJ_ALWAYS:
         case BBJ_EHCATCHRET:
+        case BBJ_EHFILTERRET:
         case BBJ_LEAVE:
         case BBJ_NONE:
             return 1;
+
         case BBJ_COND:
             if (bbJumpDest == bbNext)
             {
@@ -962,46 +1041,44 @@ unsigned BasicBlock::NumSucc(Compiler* comp)
             {
                 return 2;
             }
+
         case BBJ_SWITCH:
-            if (comp == nullptr)
-            {
-                return bbJumpSwt->bbsCount;
-            }
-            else
-            {
-                Compiler::SwitchUniqueSuccSet sd = comp->GetDescriptorForSwitch(this);
-                return sd.numDistinctSuccs;
-            }
+        {
+            Compiler::SwitchUniqueSuccSet sd = comp->GetDescriptorForSwitch(this);
+            return sd.numDistinctSuccs;
+        }
 
         default:
             unreached();
     }
 }
 
+//------------------------------------------------------------------------
+// GetSucc: Returns the requested block successor. See the declaration comment for details.
+//
+// Arguments:
+//    i - index of successor to return. 0 <= i <= NumSucc(comp).
+//    comp - Compiler instance
+//
+// Return Value:
+//    Requested successor block
+//
 BasicBlock* BasicBlock::GetSucc(unsigned i, Compiler* comp)
 {
-    // As described in the spec comment of GetSucc at its declaration, whether "comp" is null determines
-    // whether NumSucc and GetSucc yield successors of finally blocks.
+    assert(comp != nullptr);
 
     assert(i < NumSucc(comp)); // Index bounds check.
-    // printf("bbjk=%d\n", bbJumpKind);
     switch (bbJumpKind)
     {
-
-        case BBJ_THROW:
-        case BBJ_RETURN:
-            unreached(); // Should have been covered by assert above.
-
         case BBJ_EHFILTERRET:
         {
-            assert(comp != nullptr); // Or else we're not looking for successors.
-            BasicBlock* result = comp->fgFirstBlockOfHandler(this);
-            noway_assert(result == bbJumpDest);
             // Handler is the (sole) normal successor of the filter.
-            return result;
+            assert(comp->fgFirstBlockOfHandler(this) == bbJumpDest);
+            return bbJumpDest;
         }
 
         case BBJ_EHFINALLYRET:
+            // Note: the following call is expensive.
             return comp->fgSuccOfFinallyRet(this, i);
 
         case BBJ_CALLFINALLY:
@@ -1012,6 +1089,7 @@ BasicBlock* BasicBlock::GetSucc(unsigned i, Compiler* comp)
 
         case BBJ_NONE:
             return bbNext;
+
         case BBJ_COND:
             if (i == 0)
             {
@@ -1021,21 +1099,15 @@ BasicBlock* BasicBlock::GetSucc(unsigned i, Compiler* comp)
             {
                 assert(i == 1);
                 return bbJumpDest;
-            };
-        case BBJ_SWITCH:
-            if (comp == nullptr)
-            {
-                assert(i < bbJumpSwt->bbsCount); // Range check.
-                return bbJumpSwt->bbsDstTab[i];
-            }
-            else
-            {
-                // Remove duplicates.
-                Compiler::SwitchUniqueSuccSet sd = comp->GetDescriptorForSwitch(this);
-                assert(i < sd.numDistinctSuccs); // Range check.
-                return sd.nonDuplicates[i];
             }
 
+        case BBJ_SWITCH:
+        {
+            Compiler::SwitchUniqueSuccSet sd = comp->GetDescriptorForSwitch(this);
+            assert(i < sd.numDistinctSuccs); // Range check.
+            return sd.nonDuplicates[i];
+        }
+
         default:
             unreached();
     }
@@ -1293,4 +1365,4 @@ BasicBlock* Compiler::bbNewBasicBlock(BBjumpKinds jumpKind)
     block->bbNatLoopNum = BasicBlock::NOT_IN_LOOP;
 
     return block;
-}
\ No newline at end of file
+}

diff --git a/src/coreclr/src/jit/block.h b/src/coreclr/src/jit/block.h
index 1268a63..d67891d 100644 (file)
--- a/src/coreclr/src/jit/block.h
+++ b/src/coreclr/src/jit/block.h
@@ -705,33 +705,42 @@ struct BasicBlock : private LIR::Range
         BBswtDesc*  bbJumpSwt;  // switch descriptor
     };
 
-    // NumSucc() gives the number of successors, and GetSucc() allows one to iterate over them.
+    // NumSucc() gives the number of successors, and GetSucc() returns a given numbered successor.
     //
-    // The behavior of both for blocks that end in BBJ_EHFINALLYRET (a return from a finally or fault block)
-    // depends on whether "comp" is non-null. If it is null, then the block is considered to have no
-    // successor. If it is non-null, we figure out the actual successors. Some cases will want one behavior,
-    // other cases the other.  For example, IL verification requires that these blocks end in an empty operand
+    // There are two versions of these functions: ones that take a Compiler* and ones that don't. You must
+    // always use a matching set. Thus, if you call NumSucc() without a Compiler*, you must also call
+    // GetSucc() without a Compiler*.
+    //
+    // The behavior of NumSucc()/GetSucc() is different when passed a Compiler* for blocks that end in:
+    // (1) BBJ_EHFINALLYRET (a return from a finally or fault block)
+    // (2) BBJ_EHFILTERRET (a return from EH filter block)
+    // (3) BBJ_SWITCH
+    //
+    // For BBJ_EHFINALLYRET, if no Compiler* is passed, then the block is considered to have no
+    // successor. If Compiler* is passed, we figure out the actual successors. Some cases will want one behavior,
+    // other cases the other. For example, IL verification requires that these blocks end in an empty operand
     // stack, and since the dataflow analysis of IL verification is concerned only with the contents of the
     // operand stack, we can consider the finally block to have no successors. But a more general dataflow
     // analysis that is tracking the contents of local variables might want to consider *all* successors,
     // and would pass the current Compiler object.
     //
-    // Similarly, BBJ_EHFILTERRET blocks are assumed to have no successors if "comp" is null; if non-null,
-    // NumSucc/GetSucc yields the first block of the try blocks handler.
+    // Similarly, BBJ_EHFILTERRET blocks are assumed to have no successors if Compiler* is not passed; if
+    // Compiler* is passed, NumSucc/GetSucc yields the first block of the try block's handler.
     //
-    // Also, the behavior for switches changes depending on the value of "comp". If it is null, then all
-    // switch successors are returned. If it is non-null, then only unique switch successors are returned;
-    // the duplicate successors are omitted.
+    // For BBJ_SWITCH, if Compiler* is not passed, then all switch successors are returned. If Compiler*
+    // is passed, then only unique switch successors are returned; the duplicate successors are omitted.
     //
     // Note that for BBJ_COND, which has two successors (fall through and condition true branch target),
     // only the unique targets are returned. Thus, if both targets are the same, NumSucc() will only return 1
     // instead of 2.
-    //
-    // Returns the number of successors of "this".
-    unsigned NumSucc(Compiler* comp = nullptr);
 
-    // Returns the "i"th successor.  Requires (0 <= i < NumSucc()).
-    BasicBlock* GetSucc(unsigned i, Compiler* comp = nullptr);
+    // NumSucc: Returns the number of successors of "this".
+    unsigned NumSucc();
+    unsigned NumSucc(Compiler* comp);
+
+    // GetSucc: Returns the "i"th successor. Requires (0 <= i < NumSucc()).
+    BasicBlock* GetSucc(unsigned i);
+    BasicBlock* GetSucc(unsigned i, Compiler* comp);
 
     BasicBlock* GetUniquePred(Compiler* comp);