Verify no SIL critical edges.

There are multiple reasons this is needed.

1. Most passes do not perform CFG transformations. However, we often
need to split critical edges and remember to invalidate all SIL
analyses at the end of virtually every pass. This is very innefficient
and highly bug prone.

2. Many SIL analysis algorithms needs to reason about CFG
edges. Avoiding critical edges leads to far simpler and more efficient
designs when edges can be identified by blocks.

3. Handling block arguments on conditional branches create complexity
at the lowest level of the SIL interface. This complexity is difficult
to abstract over and bleeds until any algorithm that needs to reason
about phi operands. It's far easier to work with phis if we can easily
recover the phi operand with only a reference to the predecessor
block.

4. Attempting to preserve critical edges in high and mid level IR
blocks optimizations that otherwise have no business optimizing
branches. Branch optimization should always be defered to machine
level IR where the most relevant heuristics are employed to remove
unconditional branches. If code didn't need to be placed on a critical
edges, then a branch optimization can easily remove that code from the
critical edge.

Author: atrick

lib/SIL/Verifier/SILVerifier.cpp

@@ -5188,37 +5188,30 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
 
   void verifyBranches(const SILFunction *F) {
     // Verify no critical edge.
-    auto isCriticalEdgePred = [](const TermInst *T, unsigned EdgeIdx) {
-      assert(T->getSuccessors().size() > EdgeIdx && "Not enough successors");
-
+    auto requireNonCriticalSucc = [this](const TermInst *termInst,
+                                         const Twine &message) {
       // A critical edge has more than one outgoing edges from the source
       // block.
-      auto SrcSuccs = T->getSuccessors();
-      if (SrcSuccs.size() <= 1)
-        return false;
-
-      // And its destination block has more than one predecessor.
-      SILBasicBlock *DestBB = SrcSuccs[EdgeIdx];
-      assert(!DestBB->pred_empty() && "There should be a predecessor");
-      if (DestBB->getSinglePredecessorBlock())
-        return false;
+      auto succBlocks = termInst->getSuccessorBlocks();
+      if (succBlocks.size() <= 1)
+        return;
 
-      return true;
+      for (const SILBasicBlock *destBB : succBlocks) {
+        // And its destination block has more than one predecessor.
+        _require(destBB->getSinglePredecessorBlock(), message);
+      }
     };
 
-    for (auto &BB : *F) {
-      const TermInst *TI = BB.getTerminator();
-      CurInstruction = TI;
+    for (auto &bb : *F) {
+      const TermInst *termInst = bb.getTerminator();
+      CurInstruction = termInst;
 
-      // FIXME: In OSSA, critical edges will never be allowed.
-      // In Lowered SIL, they are allowed on unconditional branches only.
-      //   if (!(isSILOwnershipEnabled() && F->hasOwnership()))
-      if (AllowCriticalEdges && isa<CondBranchInst>(TI))
-        continue;
-
-      for (unsigned Idx = 0, e = BB.getSuccessors().size(); Idx != e; ++Idx) {
-        require(!isCriticalEdgePred(TI, Idx),
-                "non cond_br critical edges not allowed");
+      if (isSILOwnershipEnabled() && F->hasOwnership()) {
+        requireNonCriticalSucc(termInst, "critical edges not allowed in OSSA");
+      }
+      // In Lowered SIL, they are allowed on conditional branches only.
+      if (!AllowCriticalEdges && !isa<CondBranchInst>(termInst)) {
+        requireNonCriticalSucc(termInst, "only cond_br critical edges allowed");
       }
     }
   }