Rewrite AliasAnalysis may-release/may-decrement queries.

Use the new EscapeAnalysis infrastructure to make ARC code motion and
ARC sequence opts much more powerful and fix a latent bug in
AliasAnalysis.

Adds a new API `EscapeAnalysis::mayReleaseContent()`. This replaces
all uses if `EscapeAnalysis::canEscapeValueTo()`, which affects
`AliasAnalysis::can[Apply|Builtin]DecrementRefCount()`.

Also rewrite `AliasAnalysis::mayValueReleaseInterferWithInstruction` to
directly use `EscapeAnalysis::mayReleaseContent`.

The new implementation in `EscapeAnalysis::mayReleaseContent()`
generalizes the logic to handle more cases while avoiding an incorrect
assumption in the prior code. In particular, it adds support for
disambiguating local references from accessed addresses. This helps
handle cases in which inlining was defeating ARC optimization. The
incorrect assumption was that a non-escaping address is never
reachable via a reference. However, if a reference does not escape,
then an address into its object also does not escape.

The bug in `AliasAnalysis::mayValueReleaseInterfereWithInstruction()`
appears not to have broken anything yet because it is always called by
`AliasAnalysis::mayHaveSymmetricInteference()`, which later checks
whether the accessed address may alias with the released reference
using a separate query, `EscapeAnalysis::canPointToSameMemory()`. This
happens to work because an address into memory that is directly
released when destroying a reference necesasarilly points to the same
memory object. For this reason, I couldn't figure out a simple way to
hand-code SIL tests to expose this bug.

The changes in diff order:

Replace EscapeAnalysis `canEscapeToValue` with `mayReleaseContent` to
make the semantics clear. It queries: "Can the given reference release
the content pointed to the given address".

Change `AliasAnalysis::canApplyDecrementRefCount` to use
`mayReleaseContent` instead if 'canEscapeToValue'.

Change `AliasAnalysis::mayValueReleaseInterferWithInstruction`: after
getting the memory address accessed by the instruction, simply call
`EscapeAnalysis::mayReleaseContent`, which now implements all the
logic. This avoids the bad assumption made by AliasAnalysis.

Handle two cases in mayReleaseContent: non-escaping instruction
addresses and non-escaping referenecs. Fix the non-escaping address
case by following all content nodes to determine whether the address
is reachable from the released reference. Introduce a new optimization
for the case in which the reference being released is allocated
locally.

The following test case is now optimized in arcsequenceopts.sil:
remove_as_local_object_indirectly_escapes_to_callee. It was trying to
test that ARC optimization was not too aggressive when it removed a
retain/release of a child object whose parent container is still in
use. But the retain/release should be removed. The original example
already over-releases the parent object.

Add new unit tests to late_release_hoisting.sil.

Author: atrick

include/swift/SILOptimizer/Analysis/AliasAnalysis.h

@@ -194,10 +194,10 @@ class AliasAnalysis : public SILAnalysis {
     return alias(V1, V2, TBAAType1, TBAAType2) == AliasResult::MayAlias;
   }
 
-  /// \returns True if the release of the \p Ptr can access memory accessed by
-  /// \p User.
+  /// \returns True if the release of the \p releasedReference can access or
+  /// free memory accessed by \p User.
   bool mayValueReleaseInterfereWithInstruction(SILInstruction *User,
-                                               SILValue Ptr);
+                                               SILValue releasedReference);
 
   /// Use the alias analysis to determine the memory behavior of Inst with
   /// respect to V.

include/swift/SILOptimizer/Analysis/EscapeAnalysis.h

@@ -672,10 +672,10 @@ class EscapeAnalysis : public BottomUpIPAnalysis {
     /// isInterior is always false for non-content nodes and is set for content
     /// nodes based on the type and origin of the pointer.
     CGNode *allocNode(ValueBase *V, NodeType Type, bool isInterior,
-                      bool isReference) {
+                      bool hasReferenceOnly) {
       assert((Type == NodeType::Content) || !isInterior);
       CGNode *Node = new (NodeAllocator.Allocate())
-          CGNode(V, Type, isInterior, isReference);
+          CGNode(V, Type, isInterior, hasReferenceOnly);
       Nodes.push_back(Node);
       return Node;
     }
@@ -871,10 +871,6 @@ class EscapeAnalysis : public BottomUpIPAnalysis {
     template <typename CGNodeVisitor>
     bool forwardTraverseDefer(CGNode *startNode, CGNodeVisitor &&visitor);
 
-    /// Return true if \p pointer may indirectly point to \pointee via pointers
-    /// and object references.
-    bool mayReach(CGNode *pointer, CGNode *pointee);
-
   public:
     /// Gets or creates a node for a value \p V.
     /// If V is a projection(-path) then the base of the projection(-path) is
@@ -1164,16 +1160,16 @@ class EscapeAnalysis : public BottomUpIPAnalysis {
   /// Note that if \p RI is a retain-instruction always false is returned.
   bool canEscapeTo(SILValue V, RefCountingInst *RI);
 
-  /// Returns true if the value \p V can escape to any other pointer \p To.
-  /// This means that either \p To is the same as \p V or contains a reference
-  /// to \p V.
-  bool canEscapeToValue(SILValue V, SILValue To);
+  /// Return true if \p releasedReference deinitialization may release memory
+  /// pointed to by \p accessedAddress.
+  bool mayReleaseContent(SILValue releasedReference, SILValue accessedAddress);
 
   /// 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
+  /// First checks that the pointers are known not to alias outside this
+  /// function, then checks the connection graph to determine that their content
+  /// is not in the same points-to chain based on access inside this function.
   bool canPointToSameMemory(SILValue V1, SILValue V2);
 
   /// Invalidate all information in this analysis.

lib/SILOptimizer/Analysis/AliasAnalysis.cpp

@@ -678,7 +678,7 @@ bool AliasAnalysis::canApplyDecrementRefCount(FullApplySite FAS, SILValue Ptr) {
     if (ArgEffect.mayRelease()) {
       // The function may release this argument, so check if the pointer can
       // escape to it.
-      if (EA->canEscapeToValue(Ptr, FAS.getArgument(Idx)))
+      if (EA->mayReleaseContent(FAS.getArgument(Idx), Ptr))
         return true;
     }
   }
@@ -696,54 +696,51 @@ bool AliasAnalysis::canBuiltinDecrementRefCount(BuiltinInst *BI, SILValue Ptr) {
 
     // A builtin can only release an object if it can escape to one of the
     // builtin's arguments.
-    if (EA->canEscapeToValue(Ptr, Arg))
+    if (EA->mayReleaseContent(Arg, Ptr))
       return true;
   }
   return false;
 }
 
+// If the deinit for releasedReference can release any values used by User, then
+// this is an interference. (The retains that originally forced liveness of
+// those values may have already been eliminated). Note that we only care about
+// avoiding a dangling pointer. The memory side affects of Release are
+// unordered.
+//
+// \p releasedReference must be a value that directly contains the references
+// being released. It cannot be an address or other kind of pointer that
+// indirectly releases a reference. Otherwise, the escape analysis query is
+// invalid.
+bool AliasAnalysis::mayValueReleaseInterfereWithInstruction(
+    SILInstruction *User, SILValue releasedReference) {
+  assert(!releasedReference->getType().isAddress()
+         && "an address is never a reference");
 
-bool AliasAnalysis::mayValueReleaseInterfereWithInstruction(SILInstruction *User,
-                                                            SILValue Ptr) {
-  // TODO: Its important to make this as precise as possible.
-  //
-  // TODO: Eventually we can plug in some analysis on the what the release of
-  // the Ptr can do, i.e. be more precise about Ptr's deinit.
-  //
-  // TODO: If we know the specific release instruction, we can potentially do
-  // more.
-  //
   // If this instruction can not read or write any memory. Its OK.
   if (!User->mayReadOrWriteMemory())
     return false;
 
-  // These instructions do read or write memory, get memory directly
-  // accessed. 'V' must be the only memory accessed by User, and it must be
-  // directly accessed. Any memory indirectly accessed via 'User' may have
-  // escaped.
-  SILValue V = getDirectlyAccessedMemory(User);
-  if (!V)
-    return true;
-
-  // If the 'User' instruction's memory is uniquely identified and does not
-  // escape in the local scope, then it can't be accessed by a deinit in the
-  // local scope. Note that an exclusive argument's content may have escaped in
-  // the caller, but the argument value itself can't be accessed via aliasing
-  // references and we know that User doesn't see through any indirection.
-  if (!isUniquelyIdentified(V))
+  // Get a pointer to the memory directly accessed by 'Users' (either via an
+  // address or heap reference operand). If additional memory may be indirectly
+  // accessed by 'User', such as via an inout argument, then stop here because
+  // mayReleaseContent can only reason about one level of memory access.
+  //
+  // TODO: Handle @inout arguments by iterating over the apply arguments. For
+  // each argument find out if any reachable content can be released. This is
+  // slightly more involved than mayReleaseContent because it needs to check all
+  // connection graph nodes reachable from accessedPointer that don't pass
+  // through another stored reference.
+  SILValue accessedPointer = getDirectlyAccessedMemory(User);
+  if (!accessedPointer)
     return true;
 
-  // This is a scoped allocation.
-  // The most important check: does the object escape the current function?
-  auto LO = getUnderlyingObject(V);
-  auto *ConGraph = EA->getConnectionGraph(User->getFunction());
-  auto *Content = ConGraph->getValueContent(LO);
-  if (Content && !Content->escapes())
-    return false;
-
-  // This is either a non-local allocation or a scoped allocation that escapes.
-  // We failed to prove anything, it could be read or written by the deinit.
-  return true;
+  // If releasedReference can reach the first refcounted object reachable from
+  // accessedPointer, then releasing it early may destroy the object accessed by
+  // accessedPointer. Access to any objects beyond the first released refcounted
+  // object are irrelevant--they must already have sufficient refcount that they
+  // won't be released when releasing Ptr.
+  return EA->mayReleaseContent(releasedReference, accessedPointer);
 }
 
 bool swift::isLetPointer(SILValue V) {

lib/SILOptimizer/Analysis/EscapeAnalysis.cpp

@@ -72,7 +72,8 @@ EscapeAnalysis::findRecursivePointerKind(SILType Ty,
   };
   if (auto *Str = Ty.getStructOrBoundGenericStruct()) {
     for (auto *Field : Str->getStoredProperties()) {
-      SILType fieldTy = Ty.getFieldType(Field, M, F.getTypeExpansionContext());
+      SILType fieldTy = Ty.getFieldType(Field, M, F.getTypeExpansionContext())
+                            .getObjectType();
       meetAggregateKind(findCachedPointerKind(fieldTy, F));
     }
     return aggregateKind;
@@ -925,8 +926,10 @@ EscapeAnalysis::ConnectionGraph::getOrCreateReferenceContent(SILValue refVal,
     if (auto *C = refType.getClassOrBoundGenericClass()) {
       PointerKind aggregateKind = NoPointer;
       for (auto *field : C->getStoredProperties()) {
-        SILType fieldType = refType.getFieldType(field, F->getModule(),
-                                                 F->getTypeExpansionContext());
+        SILType fieldType = refType
+                                .getFieldType(field, F->getModule(),
+                                              F->getTypeExpansionContext())
+                                .getObjectType();
         PointerKind fieldKind = EA->findCachedPointerKind(fieldType, *F);
         if (fieldKind > aggregateKind)
           aggregateKind = fieldKind;
@@ -1161,19 +1164,6 @@ bool EscapeAnalysis::ConnectionGraph::forwardTraverseDefer(
   return true;
 }
 
-bool EscapeAnalysis::ConnectionGraph::mayReach(CGNode *pointer,
-                                               CGNode *pointee) {
-  if (pointer == pointee)
-    return true;
-
-  // This query is successful when the traversal halts and returns false.
-  return !backwardTraverse(pointee, [pointer](Predecessor pred) {
-    if (pred.getPredNode() == pointer)
-      return Traversal::Halt;
-    return Traversal::Follow;
-  });
-}
-
 void EscapeAnalysis::ConnectionGraph::removeFromGraph(ValueBase *V) {
   CGNode *node = Values2Nodes.lookup(V);
   if (!node)
@@ -1193,10 +1183,7 @@ void EscapeAnalysis::ConnectionGraph::removeFromGraph(ValueBase *V) {
 /// This makes iterating over the edges easier.
 struct CGForDotView {
 
-  enum EdgeTypes {
-    PointsTo,
-    Deferred
-  };
+  enum EdgeTypes { PointsTo, Reference, Deferred };
 
   struct Node {
     EscapeAnalysis::CGNode *OrigNode;
@@ -1248,7 +1235,10 @@ CGForDotView::CGForDotView(const EscapeAnalysis::ConnectionGraph *CG) :
     Nd.OrigNode = OrigNode;
     if (auto *PT = OrigNode->getPointsToEdge()) {
       Nd.Children.push_back(Orig2Node[PT]);
-      Nd.ChildrenTypes.push_back(PointsTo);
+      if (OrigNode->hasReferenceOnly())
+        Nd.ChildrenTypes.push_back(Reference);
+      else
+        Nd.ChildrenTypes.push_back(PointsTo);
     }
     for (auto *Def : OrigNode->defersTo) {
       Nd.Children.push_back(Orig2Node[Def]);
@@ -1396,8 +1386,12 @@ namespace llvm {
                                          const CGForDotView *Graph) {
       unsigned ChildIdx = I - Node->Children.begin();
       switch (Node->ChildrenTypes[ChildIdx]) {
-        case CGForDotView::PointsTo: return "";
-        case CGForDotView::Deferred: return "color=\"gray\"";
+      case CGForDotView::PointsTo:
+        return "";
+      case CGForDotView::Reference:
+        return "color=\"green\"";
+      case CGForDotView::Deferred:
+        return "color=\"gray\"";
       }
 
       llvm_unreachable("Unhandled CGForDotView in switch.");
@@ -2570,24 +2564,6 @@ static SILFunction *getCommonFunction(SILValue V1, SILValue V2) {
   return F;
 }
 
-bool EscapeAnalysis::canEscapeToValue(SILValue V, SILValue To) {
-  if (!isUniquelyIdentified(V))
-    return true;
-
-  SILFunction *F = getCommonFunction(V, To);
-  if (!F)
-    return true;
-  auto *ConGraph = getConnectionGraph(F);
-
-  CGNode *valueContent = ConGraph->getValueContent(V);
-  if (!valueContent)
-    return true;
-  CGNode *userContent = ConGraph->getValueContent(To);
-  if (!userContent)
-    return true;
-  return ConGraph->mayReach(userContent, valueContent);
-}
-
 bool EscapeAnalysis::canPointToSameMemory(SILValue V1, SILValue V2) {
   // At least one of the values must be a non-escaping local object.
   bool isUniq1 = isUniquelyIdentified(V1);
@@ -2642,6 +2618,99 @@ bool EscapeAnalysis::canPointToSameMemory(SILValue V1, SILValue V2) {
   return true;
 }
 
+// Return true if deinitialization of \p releasedReference may release memory
+// directly pointed to by \p accessAddress.
+//
+// Note that \p accessedAddress could be a reference itself, an address of a
+// local/argument that contains a reference, or even a pointer to the middle of
+// an object (even if it is an exclusive argument).
+//
+// This is almost the same as asking "is the content node for accessedAddress
+// reachable via releasedReference", with three subtle differences:
+//
+// (1) A locally referenced object can only be freed when deinitializing
+// releasedReference if it is the same object. Indirect references will be kept
+// alive by their distinct local references--ARC can't remove those without
+// inserting a mark_dependence/end_dependence scope.
+//
+// (2) the content of exclusive arguments may be indirectly reachable via
+// releasedReference, but the exclusive argument must have it's own reference
+// count, so cannot be freed via the locally released reference.
+//
+// (3) Objects may contain raw pointers into themselves or into other
+// objects. Any access to the raw pointer is not considered a use of the object
+// because that access must be "guarded" by a fix_lifetime or
+// mark_dependence/end_dependence that acts as a placeholder.
+//
+// There are two interesting cases in which a connection graph query can
+// determine that the accessed memory cannot be released:
+//
+// Case #1: accessedAddress points to a uniquely identified object that does not
+// escape within this function.
+//
+// Note: A "uniquely identified object" is either a locally allocated object,
+// which is obviously not reachable outside this function, or an exclusive
+// address argument, which *is* reachable outside this function, but must
+// have its own reference count so cannot be released locally.
+//
+// Case #2: The released reference points to a local object and no connection
+// graph path exists from the referenced object to a global-escaping or
+// argument-escaping node without traversing a non-interior edge.
+//
+// In both cases, the connection graph is sufficient to determine if the
+// accessed content may be released. To prove that the accessed memory is
+// distinct from any released memory it is now sufficient to check that no
+// connection graph path exists from the released object's node to the accessed
+// content node without traversing a non-interior edge.
+bool EscapeAnalysis::mayReleaseContent(SILValue releasedReference,
+                                       SILValue accessedAddress) {
+  assert(!releasedReference->getType().isAddress()
+         && "an address is never a reference");
+
+  SILFunction *f = getCommonFunction(releasedReference, accessedAddress);
+  if (!f)
+    return true;
+
+  auto *conGraph = getConnectionGraph(f);
+
+  CGNode *addrContentNode = conGraph->getValueContent(accessedAddress);
+  if (!addrContentNode)
+    return true;
+
+  // Case #1: Unique accessedAddress whose content does not escape.
+  bool isAccessUniq =
+      isUniquelyIdentified(accessedAddress)
+      && !addrContentNode->valueEscapesInsideFunction(accessedAddress);
+
+  // Case #2: releasedReference points to a local object.
+  if (!isAccessUniq && !pointsToLocalObject(releasedReference))
+    return true;
+
+  CGNode *releasedObjNode = conGraph->getValueContent(releasedReference);
+  // Make sure we have at least one value CGNode for releasedReference.
+  if (!releasedObjNode)
+    return true;
+
+  // Check for reachability from releasedObjNode to addrContentNode.
+  // A pointsTo cycle is equivalent to a null pointsTo.
+  CGNodeWorklist worklist(conGraph);
+  for (CGNode *releasedNode = releasedObjNode;
+       releasedNode && worklist.tryPush(releasedNode);
+       releasedNode = releasedNode->getContentNodeOrNull()) {
+    // A path exists from released content to accessed content.
+    if (releasedNode == addrContentNode)
+      return true;
+
+    // A path exists to an escaping node.
+    if (!isAccessUniq && releasedNode->escapesInsideFunction())
+      return true;
+
+    if (!releasedNode->isInterior())
+      break;
+  }
+  return false; // no path to escaping memory that may be freed.
+}
+
 void EscapeAnalysis::invalidate() {
   Function2Info.clear();
   Allocator.DestroyAll();