Merge pull request #30414 from atrick/fix-escape-unreachable

Fix EscapeAnalysis verification assert at unreachable blocks

Author: atrick

include/swift/SILOptimizer/Utils/BasicBlockOptUtils.h

@@ -33,6 +33,24 @@ class BasicBlockCloner;
 class SILLoop;
 class SILLoopInfo;
 
+/// Compute the set of reachable blocks.
+class ReachableBlocks {
+  SmallPtrSet<SILBasicBlock *, 32> visited;
+
+public:
+  /// Invoke \p visitor for each reachable block in \p f in worklist order (at
+  /// least one predecessor has been visited--defs are always visited before
+  /// uses except for phi-type block args). The \p visitor takes a block
+  /// argument, which is already marked visited, and must return true to
+  /// continue visiting blocks.
+  ///
+  /// Returns true if all reachable blocks were visited.
+  bool visit(SILFunction *f, function_ref<bool(SILBasicBlock *)> visitor);
+
+  /// Return true if \p bb has been visited.
+  bool isVisited(SILBasicBlock *bb) const { return visited.count(bb); }
+};
+
 /// Remove all instructions in the body of \p bb in safe manner by using
 /// undef.
 void clearBlockBody(SILBasicBlock *bb);

lib/SILOptimizer/Analysis/EscapeAnalysis.cpp

@@ -18,6 +18,7 @@
 #include "swift/SILOptimizer/Analysis/ArraySemantic.h"
 #include "swift/SILOptimizer/Analysis/BasicCalleeAnalysis.h"
 #include "swift/SILOptimizer/PassManager/PassManager.h"
+#include "swift/SILOptimizer/Utils/BasicBlockOptUtils.h"
 #include "swift/SILOptimizer/Utils/InstOptUtils.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/GraphWriter.h"
@@ -1585,17 +1586,19 @@ void EscapeAnalysis::ConnectionGraph::verify() const {
   verifyStructure();
 
   // Verify that all pointer nodes are still mapped, otherwise the process of
-  // merging nodes may have lost information.
-  for (SILBasicBlock &BB : *F) {
-    for (auto &I : BB) {
-      if (isNonWritableMemoryAddress(&I))
+  // merging nodes may have lost information. Only visit reachable blocks,
+  // because the graph builder only mapped values from reachable blocks.
+  ReachableBlocks reachable;
+  reachable.visit(F, [this](SILBasicBlock *bb) {
+    for (auto &i : *bb) {
+      if (isNonWritableMemoryAddress(&i))
         continue;
 
-      if (auto ai = dyn_cast<ApplyInst>(&I)) {
+      if (auto ai = dyn_cast<ApplyInst>(&i)) {
         if (EA->canOptimizeArrayUninitializedCall(ai, this).isValid())
           continue;
       }
-      for (auto result : I.getResults()) {
+      for (auto result : i.getResults()) {
         if (EA->getPointerBase(result))
           continue;
 
@@ -1611,7 +1614,8 @@ void EscapeAnalysis::ConnectionGraph::verify() const {
         }
       }
     }
-  }
+    return true;
+  });
 #endif
 }
 
@@ -1705,27 +1709,21 @@ void EscapeAnalysis::buildConnectionGraph(FunctionInfo *FInfo,
   ConnectionGraph *ConGraph = &FInfo->Graph;
   assert(ConGraph->isEmpty());
 
-  // We use a worklist for iteration to visit the blocks in dominance order.
-  llvm::SmallPtrSet<SILBasicBlock*, 32> VisitedBlocks;
-  llvm::SmallVector<SILBasicBlock *, 16> WorkList;
-  VisitedBlocks.insert(&*ConGraph->F->begin());
-  WorkList.push_back(&*ConGraph->F->begin());
-
-  while (!WorkList.empty()) {
-    SILBasicBlock *BB = WorkList.pop_back_val();
-
+  // Visit the blocks in dominance order.
+  ReachableBlocks reachable;
+  reachable.visit(ConGraph->F, [&](SILBasicBlock *bb) {
     // Create edges for the instructions.
-    for (auto &I : *BB) {
-      analyzeInstruction(&I, FInfo, BottomUpOrder, RecursionDepth);
+    for (auto &i : *bb) {
+      analyzeInstruction(&i, FInfo, BottomUpOrder, RecursionDepth);
     }
-    for (auto &Succ : BB->getSuccessors()) {
-      if (VisitedBlocks.insert(Succ.getBB()).second)
-        WorkList.push_back(Succ.getBB());
-    }
-  }
+    return true;
+  });
 
   // Second step: create defer-edges for block arguments.
   for (SILBasicBlock &BB : *ConGraph->F) {
+    if (!reachable.isVisited(&BB))
+      continue;
+
     if (!linkBBArgs(&BB))
       continue;
 

lib/SILOptimizer/Utils/BasicBlockOptUtils.cpp

@@ -17,6 +17,30 @@
 
 using namespace swift;
 
+/// Invoke \p visitor for each reachable block in \p f in worklist order (at
+/// least one predecessor has been visited).
+bool ReachableBlocks::visit(SILFunction *f,
+                            function_ref<bool(SILBasicBlock *)> visitor) {
+  assert(visited.empty() && "blocks already visited");
+
+  // Walk over the CFG, starting at the entry block, until all reachable blocks
+  // are visited.
+  SILBasicBlock *entryBB = f->getEntryBlock();
+  SmallVector<SILBasicBlock *, 8> worklist = {entryBB};
+  visited.insert(entryBB);
+  while (!worklist.empty()) {
+    SILBasicBlock *bb = worklist.pop_back_val();
+    if (!visitor(bb))
+      return false;
+
+    for (auto &succ : bb->getSuccessors()) {
+      if (visited.insert(succ).second)
+        worklist.push_back(succ);
+    }
+  }
+  return true;
+}
+
 /// Remove all instructions in the body of \p bb in safe manner by using
 /// undef.
 void swift::clearBlockBody(SILBasicBlock *bb) {
@@ -42,25 +66,16 @@ void swift::removeDeadBlock(SILBasicBlock *bb) {
 }
 
 bool swift::removeUnreachableBlocks(SILFunction &f) {
-  // All reachable blocks, but does not include the entry block.
-  llvm::SmallPtrSet<SILBasicBlock *, 8> visited;
-
-  // Walk over the CFG, starting at the entry block, until all reachable blocks are visited.
-  llvm::SmallVector<SILBasicBlock *, 8> worklist(1, f.getEntryBlock());
-  while (!worklist.empty()) {
-    SILBasicBlock *bb = worklist.pop_back_val();
-    for (auto &Succ : bb->getSuccessors()) {
-      if (visited.insert(Succ).second)
-        worklist.push_back(Succ);
-    }
-  }
+  ReachableBlocks reachable;
+  // Visit all the blocks without doing any extra work.
+  reachable.visit(&f, [](SILBasicBlock *) { return true; });
 
-  // Remove the blocks we never reached. Exclude the entry block from the iteration because it's
-  // not included in the Visited set.
+  // Remove the blocks we never reached. Assume the entry block is visited.
+  // Reachable's visited set contains dangling pointers during this loop.
   bool changed = false;
   for (auto ii = std::next(f.begin()), end = f.end(); ii != end;) {
     auto *bb = &*ii++;
-    if (!visited.count(bb)) {
+    if (!reachable.isVisited(bb)) {
       removeDeadBlock(bb);
       changed = true;
     }

test/SILOptimizer/escape_analysis.sil

@@ -1909,3 +1909,32 @@ bb0(%0 : $*SomeData):
   %5 = tuple ()
   return %5 : $()
 }
+
+// -----------------------------------------------------------------------------
+// Unreachable blocks are not mapped to the connection graph. Test
+// that verification does not assert with "Missing escape connection
+// graph mapping". <rdar://problem/60373501> EscapeAnalysis crashes
+// with CFG with unreachable
+sil_global hidden @globalInt: $Int
+
+// CHECK-LABEL: CG of testUnreachable
+// CHECK-LABEL: End
+sil hidden [noinline] @testUnreachable : $@convention(thin) () -> () {
+bb0:
+  %g1 = global_addr @globalInt : $*Int
+  %a1 = begin_access [read] [dynamic] [no_nested_conflict] %g1 : $*Int // users: %18, %17
+  %v1 = load %a1 : $*Int
+  end_access %a1 : $*Int
+  br bb4
+
+bb2:
+  %g2 = global_addr @globalInt : $*Int
+  %a2 = begin_access [read] [dynamic] [no_nested_conflict] %g2 : $*Int // users: %18, %17
+  %v2 = load %a2 : $*Int
+  end_access %a2 : $*Int
+  br bb4
+
+bb4:
+  %z = tuple ()
+  return %z : $()
+}