Fix -escapes-internal-verify for summary graph merging.

Don't merge node properties until after node merging begins, so
internal verification can run right before each merge.

Rework ConnectionGraph::mergeFrom. Remove an extra loop. Defer
mergeAllScheduledNodes until all the source graph's mapped nodes are
added so that the graph is always structurally valid before a
merge. This is also necessary to avoid EscapeAnalysis
assert: (!To->mergeTo), in setPointsToEdge.

Enable -escapes-internal-verify to all tests in escape_analysis.sil.

Add hand-reduced unit tests in escape_analysis_reduced.sil.

Author: atrick

include/swift/SILOptimizer/Analysis/EscapeAnalysis.h

@@ -688,7 +688,6 @@ class EscapeAnalysis : public BottomUpIPAnalysis {
       CGNode *FromMergeTarget = From->getMergeTarget();
       CGNode *ToMergeTarget = To->getMergeTarget();
       if (FromMergeTarget != ToMergeTarget) {
-        ToMergeTarget->mergeProperties(FromMergeTarget);
         FromMergeTarget->mergeTo = ToMergeTarget;
         ToMerge.push_back(FromMergeTarget);
       }

lib/SILOptimizer/Analysis/EscapeAnalysis.cpp

@@ -558,7 +558,8 @@ void EscapeAnalysis::ConnectionGraph::initializePointsTo(CGNode *initialNode,
 
         CGNode *predNode = pred.getPredNode();
         if (predNode->pointsTo) {
-          assert(predNode->pointsTo->getMergeTarget() == newPointsTo);
+          assert(predNode->pointsTo->getMergeTarget()
+                 == newPointsTo->getMergeTarget());
           return Traversal::Backtrack;
         }
         predNode->pointsTo = newPointsTo;
@@ -761,6 +762,11 @@ void EscapeAnalysis::ConnectionGraph::mergePointsTo(CGNode *initialNode,
                                                     CGNode *newPointsTo) {
   CGNode *oldPointsTo = initialNode->pointsTo;
   assert(oldPointsTo && "merging content should not initialize any pointsTo");
+
+  // newPointsTo may already be scheduled for a merge. Only create new edges to
+  // unmerged nodes. This may create a temporary pointsTo mismatch in the defer
+  // web, but Graph verification takes merged nodes into consideration.
+  newPointsTo = newPointsTo->getMergeTarget();
   if (oldPointsTo == newPointsTo)
     return;
 
@@ -870,8 +876,6 @@ CGNode *EscapeAnalysis::ConnectionGraph::createContentNode(
   CGNode *newContent =
       allocNode(nullptr, NodeType::Content, isInterior, hasReferenceOnly);
   initializePointsToEdge(addrNode, newContent);
-  assert(ToMerge.empty()
-         && "Initially setting pointsTo should not require any node merges");
   return newContent;
 }
 
@@ -1002,57 +1006,58 @@ bool EscapeAnalysis::ConnectionGraph::mergeFrom(ConnectionGraph *SourceGraph,
                                                 CGNodeMap &Mapping) {
   // The main point of the merging algorithm is to map each content node in the
   // source graph to a content node in this (destination) graph. This may
-  // require to create new nodes or to merge existing nodes in this graph.
+  // require creating new nodes or merging existing nodes in this graph.
 
   // First step: replicate the points-to edges and the content nodes of the
   // source graph in this graph.
   bool Changed = false;
-  bool NodesMerged;
-  do {
-    NodesMerged = false;
-    for (unsigned Idx = 0; Idx < Mapping.getMappedNodes().size(); ++Idx) {
-      CGNode *SourceNd = Mapping.getMappedNodes()[Idx];
-      CGNode *DestNd = Mapping.get(SourceNd);
-      assert(DestNd);
-      
-      if (SourceNd->getEscapeState() >= EscapeState::Global) {
-        // We don't need to merge the source subgraph of nodes which have the
-        // global escaping state set.
-        // Just set global escaping in the caller node and that's it.
-        Changed |= DestNd->mergeEscapeState(EscapeState::Global);
-        // If DestNd is an interior node, its content still needs to be created.
-        if (!DestNd->isInterior())
-          continue;
-      }
-
-      CGNode *SourcePT = SourceNd->pointsTo;
-      if (!SourcePT)
+  for (unsigned Idx = 0; Idx < Mapping.getMappedNodes().size(); ++Idx) {
+    CGNode *SourceNd = Mapping.getMappedNodes()[Idx];
+    CGNode *DestNd = Mapping.get(SourceNd);
+    assert(DestNd);
+
+    if (SourceNd->getEscapeState() >= EscapeState::Global) {
+      // We don't need to merge the source subgraph of nodes which have the
+      // global escaping state set.
+      // Just set global escaping in the caller node and that's it.
+      Changed |= DestNd->mergeEscapeState(EscapeState::Global);
+      // If DestNd is an interior node, its content still needs to be created.
+      if (!DestNd->isInterior())
         continue;
+    }
+
+    CGNode *SourcePT = SourceNd->pointsTo;
+    if (!SourcePT)
+      continue;
 
-      CGNode *DestPT = DestNd->pointsTo;
+    CGNode *MappedDestPT = Mapping.get(SourcePT);
+    CGNode *DestPT = DestNd->pointsTo;
+    if (!MappedDestPT) {
       if (!DestPT) {
         DestPT = createMergedContent(DestNd, SourcePT);
         Changed = true;
       }
-      CGNode *MappedDestPT = Mapping.get(SourcePT);
-      if (!MappedDestPT) {
-        // This is the first time the dest node is seen; just add the mapping.
-        Mapping.add(SourcePT, DestPT);
-        continue;
-      }
-      // We already found the destination node through another path.
-      assert(Mapping.getMappedNodes().contains(SourcePT));
-      if (DestPT == MappedDestPT)
-        continue;
+      // This is the first time the dest node is seen; just add the mapping.
+      Mapping.add(SourcePT, DestPT);
+      continue;
+    }
+    if (DestPT == MappedDestPT)
+      continue;
 
-      // There are two content nodes in this graph which map to the same
-      // content node in the source graph -> we have to merge them.
-      scheduleToMerge(DestPT, MappedDestPT);
-      mergeAllScheduledNodes();
-      Changed = true;
-      NodesMerged = true;
+    // We already found the destination node through another path.
+    assert(Mapping.getMappedNodes().contains(SourcePT));
+    Changed = true;
+    if (!DestPT) {
+      initializePointsToEdge(DestNd, MappedDestPT);
+      continue;
     }
-  } while (NodesMerged);
+    // There are two content nodes in this graph which map to the same
+    // content node in the source graph -> we have to merge them.
+    // Defer merging the nodes until all mapped nodes are created so that the
+    // graph is structurally valid before merging.
+    scheduleToMerge(DestPT, MappedDestPT);
+  }
+  mergeAllScheduledNodes();
   Mapping.getMappedNodes().reset(); // Make way for a different worklist.
 
   // Second step: add the source graph's defer edges to this graph.

test/SILOptimizer/escape_analysis.sil

@@ -1,4 +1,4 @@
-// RUN: %target-sil-opt %s -escapes-dump -o /dev/null | %FileCheck %s
+// RUN: %target-sil-opt %s -escapes-dump -escapes-internal-verify -o /dev/null | %FileCheck %s
 
 // REQUIRES: asserts
 
@@ -594,7 +594,7 @@ sil_global @global_ln : $LinkedNode
 // CHECK-NEXT:    Arg [ref] %0 Esc: A, Succ: (%1)
 // CHECK-NEXT:    Con [int] %1 Esc: A, Succ: (%2)
 // CHECK-NEXT:    Con [ref] %2 Esc: G, Succ:
-// CHECK-NEXT:    Val [ref] %4 Esc: , Succ: (%4.1), %2
+// CHECK-NEXT:    Val [ref] %4 Esc: G, Succ: (%4.1), %2
 // CHECK-NEXT:    Con [int] %4.1 Esc: G, Succ: (%4.2)
 // CHECK-NEXT:    Con [ref] %4.2 Esc: G, Succ: 
 // CHECK-NEXT:    Ret [ref] return Esc: , Succ: %4
@@ -614,7 +614,7 @@ bb0(%0 : $LinkedNode):
 // CHECK-NEXT:    Con [ref] %0.2 Esc: G, Succ:
 // CHECK-NEXT:    Val %1 Esc: , Succ: (%1.1)
 // CHECK-NEXT:    Con [ref] %1.1 Esc: G, Succ: %0
-// CHECK-NEXT:    Val [ref] %4 Esc: , Succ: (%0.1), %0
+// CHECK-NEXT:    Val [ref] %4 Esc: G, Succ: (%0.1), %0
 // CHECK-NEXT:    Ret [ref] return Esc: , Succ: %4
 // CHECK-NEXT:  End
 sil @let_escape : $@convention(thin) (@owned LinkedNode) -> @owned LinkedNode {
@@ -628,7 +628,7 @@ bb0(%0 : $LinkedNode):
 
 // CHECK-LABEL: CG of return_same
 // CHECK-NEXT:    Arg [ref] %0 Esc: A, Succ: (%5.1)
-// CHECK-NEXT:    Val [ref] %3 Esc: , Succ: (%5.1), %5.2
+// CHECK-NEXT:    Val [ref] %3 Esc: G, Succ: (%5.1), %5.2
 // CHECK-NEXT:    Val [ref] %5 Esc: , Succ: (%5.1), %0, %3
 // CHECK-NEXT:    Con [int] %5.1 Esc: G, Succ: (%5.2)
 // CHECK-NEXT:    Con [ref] %5.2 Esc: G, Succ: (%5.1)

test/SILOptimizer/escape_analysis_dead_store.sil

@@ -1,4 +1,4 @@
-// RUN: %target-sil-opt %s -dead-store-elim -enable-sil-verify-all | %FileCheck %s
+// RUN: %target-sil-opt %s -dead-store-elim -enable-sil-verify-all -escapes-internal-verify | %FileCheck %s
 
 sil_stage canonical
 

test/SILOptimizer/escape_analysis_invalidate.sil

@@ -1,4 +1,4 @@
-// RUN: %target-sil-opt %s -temp-rvalue-opt -enable-sil-verify-all | %FileCheck %s
+// RUN: %target-sil-opt %s -temp-rvalue-opt -enable-sil-verify-all -escapes-internal-verify | %FileCheck %s
 //
 // TempRValue iteratively uses EscapeAnalysis and deletes
 // instructions. Make sure that the connection graph remains valid