Merge pull request #28556 from atrick/escape-valueescapes

Author: swift-ci

include/swift/SILOptimizer/Analysis/EscapeAnalysis.h

@@ -243,7 +243,19 @@ class EscapeAnalysis : public BottomUpIPAnalysis {
   struct CGNodeWorklist;
 
   /// The int-part is an EdgeType and specifies which kind of predecessor it is.
-  typedef llvm::PointerIntPair<CGNode *, 1> Predecessor;
+  struct Predecessor {
+    llvm::PointerIntPair<CGNode *, 1> predAndEdgeType;
+
+    Predecessor(CGNode *predNode, EdgeType ty)
+        : predAndEdgeType(predNode, ty) {}
+    bool operator==(const Predecessor &pred) {
+      return predAndEdgeType == pred.predAndEdgeType;
+    }
+
+    bool is(EdgeType ty) const { return predAndEdgeType.getInt() == ty; }
+
+    CGNode *getPredNode() const { return predAndEdgeType.getPointer(); }
+  };
 
 public:
 
@@ -473,23 +485,36 @@ class EscapeAnalysis : public BottomUpIPAnalysis {
     /// Returns true if the node's value escapes within the function. This
     /// means that any unidentified pointer in the function may alias to
     /// the node's value.
+    ///
     /// Note that in the false-case the node's value can still escape via
     /// the return instruction.
     ///
     /// \p nodeValue is often the same as 'this->getRepValue().getValue()', but
     /// is sometimes a more refined value specific to a content nodes.
-    bool escapesInsideFunction(SILValue nodeValue) const {
+    bool valueEscapesInsideFunction(SILValue nodeValue) const {
       switch (getEscapeState()) {
         case EscapeState::None:
         case EscapeState::Return:
           return false;
         case EscapeState::Arguments:
-          return !isExclusiveArgument(nodeValue);
+          return !nodeValue || !isExclusiveArgument(nodeValue);
         case EscapeState::Global:
           return true;
       }
       llvm_unreachable("Unhandled EscapeState in switch.");
     }
+    /// Returns true if the node's value escapes within the function.
+    ///
+    /// This is a more conservative variant of valueEscapesInsideFunction, for
+    /// when the value being accessed is unknown. For content nodes, the node's
+    /// value cannot be used because it arbitrarily takes on one of the values
+    /// associated with the content. e.g. if that value is an exclusive
+    /// argument, it would be incorrect to assume all values for that content
+    /// are exclusive.
+    bool escapesInsideFunction() const {
+      SILValue nodeValue = isContent() ? SILValue() : SILValue(mappedValue);
+      return valueEscapesInsideFunction(nodeValue);
+    }
 
     /// Returns the content node of this node if it exists in the graph.
     CGNode *getContentNodeOrNull() const {
@@ -598,12 +623,6 @@ class EscapeAnalysis : public BottomUpIPAnalysis {
       return Node;
     }
 
-    /// Adds a defer-edge and updates pointsTo of all defer-reachable nodes.
-    /// The addition of a defer-edge may invalidate the graph invariance 4).
-    /// If this is the case, all "mismatching" Content nodes are merged until
-    /// invariance 4) is reached again.
-    bool addDeferEdge(CGNode *From, CGNode *To);
-
     /// Adds the node \p From (to be merged with \p To) to the ToMerge list.
     /// The actual merging is done in mergeAllScheduledNodes().
     void scheduleToMerge(CGNode *From, CGNode *To) {
@@ -837,6 +856,7 @@ class EscapeAnalysis : public BottomUpIPAnalysis {
     friend class EscapeAnalysis;
     friend struct CGNodeWorklist;
   };
+  using Traversal = ConnectionGraph::Traversal;
 
 private:
 
@@ -1053,9 +1073,9 @@ class EscapeAnalysis : public BottomUpIPAnalysis {
 
   /// Returns true if the pointers \p V1 and \p V2 can possibly point to the
   /// same memory.
+  ///
   /// If at least one of the pointers refers to a local object and the
   /// connection-graph-nodes of both pointers do not point to the same content
-  /// node, the pointers do not alias.
   bool canPointToSameMemory(SILValue V1, SILValue V2);
 
   /// Invalidate all information in this analysis.

lib/SILOptimizer/Analysis/EscapeAnalysis.cpp

@@ -336,11 +336,11 @@ std::pair<const CGNode *, unsigned> EscapeAnalysis::CGNode::getRepNode(
     return {this, 0};
 
   for (Predecessor pred : Preds) {
-    if (pred.getInt() != EdgeType::PointsTo)
+    if (!pred.is(EdgeType::PointsTo))
       continue;
-    if (!visited.insert(pred.getPointer()).second)
+    if (!visited.insert(pred.getPredNode()).second)
       continue;
-    auto repNodeAndDepth = pred.getPointer()->getRepNode(visited);
+    auto repNodeAndDepth = pred.getPredNode()->getRepNode(visited);
     if (repNodeAndDepth.first)
       return {repNodeAndDepth.first, repNodeAndDepth.second + 1};
     // If a representative node was not found on this pointsTo node, recursion
@@ -390,9 +390,9 @@ bool EscapeAnalysis::CGNode::visitSuccessors(Visitor &&visitor) const {
 template <typename Visitor>
 bool EscapeAnalysis::CGNode::visitDefers(Visitor &&visitor) const {
   for (Predecessor pred : Preds) {
-    if (pred.getInt() != EdgeType::Defer)
+    if (!pred.is(EdgeType::Defer))
       continue;
-    if (!visitor(pred.getPointer(), false))
+    if (!visitor(pred.getPredNode(), false))
       return false;
   }
   for (auto *deferred : defersTo) {
@@ -549,10 +549,10 @@ void EscapeAnalysis::ConnectionGraph::initializePointsTo(CGNode *initialNode,
       // distinguish these cases, always retry backward traversal--it just won't
       // revisit any edges in the later case.
       backwardTraverse(edgeNode, [&](Predecessor pred) {
-        if (pred.getInt() != EdgeType::Defer)
+        if (!pred.is(EdgeType::Defer))
           return Traversal::Backtrack;
 
-        CGNode *predNode = pred.getPointer();
+        CGNode *predNode = pred.getPredNode();
         if (predNode->pointsTo) {
           assert(predNode->pointsTo->getMergeTarget() == newPointsTo);
           return Traversal::Backtrack;
@@ -608,8 +608,8 @@ void EscapeAnalysis::ConnectionGraph::mergeAllScheduledNodes() {
     // initializePointsTo(). By ensuring that 'From' is unreachable first, the
     // graph appears consistent during those operations.
     for (Predecessor Pred : From->Preds) {
-      CGNode *PredNode = Pred.getPointer();
-      if (Pred.getInt() == EdgeType::PointsTo) {
+      CGNode *PredNode = Pred.getPredNode();
+      if (Pred.is(EdgeType::PointsTo)) {
         assert(PredNode->getPointsToEdge() == From
                && "Incoming pointsTo edge not set in predecessor");
         if (PredNode != From)
@@ -715,13 +715,13 @@ void EscapeAnalysis::ConnectionGraph::mergeAllScheduledNodes() {
     // pointsTo predecessors (which are now attached to 'To').
     for (unsigned PredIdx = 0; PredIdx < To->Preds.size(); ++PredIdx) {
       auto predEdge = To->Preds[PredIdx];
-      if (predEdge.getInt() != EdgeType::PointsTo)
+      if (!predEdge.is(EdgeType::PointsTo))
         continue;
-      predEdge.getPointer()->visitDefers(
-        [&](CGNode *deferred, bool /*isSucc*/) {
-          mergePointsTo(deferred, To);
-          return true;
-        });
+      predEdge.getPredNode()->visitDefers(
+          [&](CGNode *deferred, bool /*isSucc*/) {
+            mergePointsTo(deferred, To);
+            return true;
+          });
     }
     To->mergeEscapeState(From->State);
 
@@ -1024,7 +1024,7 @@ bool EscapeAnalysis::ConnectionGraph::backwardTraverse(
     for (Predecessor pred : reachingNode->Preds) {
       switch (visitor(pred)) {
       case Traversal::Follow: {
-        CGNode *predNode = pred.getPointer();
+        CGNode *predNode = pred.getPredNode();
         worklist.tryPush(predNode);
         break;
       }
@@ -1073,7 +1073,7 @@ bool EscapeAnalysis::ConnectionGraph::mayReach(CGNode *pointer,
 
   // This query is successful when the traversal halts and returns false.
   return !backwardTraverse(pointee, [pointer](Predecessor pred) {
-    if (pred.getPointer() == pointer)
+    if (pred.getPredNode() == pointer)
       return Traversal::Halt;
     return Traversal::Follow;
   });
@@ -1177,8 +1177,6 @@ std::string CGForDotView::getNodeLabel(const Node *Node) const {
   std::string Label;
   llvm::raw_string_ostream O(Label);
   Node->OrigNode->getRepValue().print(O, InstToIDMap);
-  if (Node->OrigNode->Type == EscapeAnalysis::NodeType::Return)
-    O << "return";
   O << '\n';
   if (Node->OrigNode->mappedValue) {
     std::string Inst;
@@ -1503,11 +1501,11 @@ void EscapeAnalysis::ConnectionGraph::verifyStructure(bool allowMerge) const {
     }
     // Check if predecessor and successor edges are linked correctly.
     for (Predecessor Pred : Nd->Preds) {
-      CGNode *PredNode = Pred.getPointer();
-      if (Pred.getInt() == EdgeType::Defer) {
+      CGNode *PredNode = Pred.getPredNode();
+      if (Pred.is(EdgeType::Defer)) {
         assert(PredNode->findDeferred(Nd) != PredNode->defersTo.end());
       } else {
-        assert(Pred.getInt() == EdgeType::PointsTo);
+        assert(Pred.is(EdgeType::PointsTo));
         assert(PredNode->getPointsToEdge() == Nd);
       }
     }
@@ -2349,7 +2347,7 @@ bool EscapeAnalysis::canEscapeToUsePoint(SILValue V, SILNode *UsePoint,
 
   // First check if there are escape paths which we don't explicitly see
   // in the graph.
-  if (Node->escapesInsideFunction(V))
+  if (Node->valueEscapesInsideFunction(V))
     return true;
 
   // No hidden escapes: check if the Node is reachable from the UsePoint.
@@ -2369,7 +2367,7 @@ bool EscapeAnalysis::canEscapeToUsePoint(SILValue V, SILNode *UsePoint,
   // As V1's content node is the same as V's content node, we also make the
   // check for the content node.
   CGNode *ContentNode = getValueContent(ConGraph, V);
-  if (ContentNode->escapesInsideFunction(V))
+  if (ContentNode->valueEscapesInsideFunction(V))
     return true;
 
   if (ConGraph->isUsePoint(UsePoint, ContentNode))
@@ -2450,10 +2448,10 @@ bool EscapeAnalysis::canPointToSameMemory(SILValue V1, SILValue V2) {
     return true;
 
   // Finish the check for one value being a non-escaping local object.
-  if (isUniq1 && Node1->escapesInsideFunction(V1))
+  if (isUniq1 && Node1->valueEscapesInsideFunction(V1))
     isUniq1 = false;
 
-  if (isUniq2 && Node2->escapesInsideFunction(V2))
+  if (isUniq2 && Node2->valueEscapesInsideFunction(V2))
     isUniq2 = false;
 
   if (!isUniq1 && !isUniq2)

test/SILOptimizer/basic-aa.sil

@@ -110,12 +110,12 @@ bb1(%1 : $*Builtin.NativeObject, %2 : $*Builtin.NativeObject):
 
 // Assume that inout arguments alias to preserve memory safety.
 //
-// CHECK-LABEL: @inout_args_may_alias
+// CHECK-LABEL: @inout_args_may_not_alias
 // CHECK: PAIR #1.
 // CHECK-NEXT: %0 = argument of bb0
 // CHECK-NEXT: %1 = argument of bb0
 // CHECK-NEXT: NoAlias
-sil @inout_args_may_alias: $@convention(thin) (@inout Builtin.NativeObject, @inout Builtin.NativeObject) -> () {
+sil @inout_args_may_not_alias: $@convention(thin) (@inout Builtin.NativeObject, @inout Builtin.NativeObject) -> () {
 bb0(%0 : $*Builtin.NativeObject, %1 : $*Builtin.NativeObject):
   %2 = tuple()
   return %2 : $()