Cleanup and document SIL memory behavior APIs.

This is code that I am fairly familiar with but it still took a day of
investigation to figure out how it is supposed to be used now in the
presence of bridging.

This primarily involved ruling out the possibity that the mid-level
Swift APIs could at some point call into the lower-level C++ APIs.

The biggest problem was that AliasAnalysis::getMemoryBehaviorOfInst()
was declared as a public interface, and it's name indicates that it
computes the memory behavior. But it is just a wrapper around a Swift
API and never actually calls into any of the C++ logic that is
responsible for computing memory behavior!

include/swift/SIL/SILNodes.def

@@ -137,8 +137,13 @@
 ///   NAME is the name of the instruction in SIL assembly.
 ///   The argument will be a bare identifier, not a string literal.
 ///
-///   MEMBEHAVIOR is an enum value that reflects the memory behavior of
-///   the instruction.
+///   MEMBEHAVIOR is an enum value that reflects the memory behavior of the
+///   instruction. It is only used in the implementation of
+///   SILInstruction::getMemoryBehavior(). Memory behavior is relative, so
+///   whenever possible, clients should instead use
+///   AliasAnalysis::computeMemoryBehavior(SILInstruction, SILValue) which only
+///   defaults to SILInstruction::getMemoryBehavior() when AliasAnalysis cannot
+///   disambiguate the instruction's effects from the value of interest.
 ///
 ///   MAYRELEASE indicates whether the execution of the
 ///   instruction may result in memory being released.

include/swift/SILOptimizer/Analysis/AliasAnalysis.h

@@ -22,6 +22,10 @@ namespace swift {
 
 /// This class is a simple wrapper around an alias analysis cache. This is
 /// needed since we do not have an "analysis" infrastructure.
+///
+/// This wrapper sits above the SwiftCompilerSource implementation of
+/// AliasAnalysis. The implementation calls into AliasAnalysis.swift via
+/// BridgedAliasAnalysis whenever the result may depend on escape analysis.
 class AliasAnalysis {
 public:
 
@@ -159,14 +163,24 @@ public:
     return alias(V1, V2, TBAAType1, TBAAType2) == AliasResult::MayAlias;
   }
 
-  /// Use the alias analysis to determine the memory behavior of Inst with
-  /// respect to V.
+  /// Compute the effects of Inst's memory behavior on the memory pointed to by
+  /// the value V.
+  ///
+  /// This is the top-level API for memory behavior.
+  ///
+  /// 1. MemoryBehaviorVisitor overrides select instruction types. Types that
+  /// have no override default to SILInstruction::getMemoryBehavior(), which is
+  /// not specific to the memory pointed to by V.
+  ///
+  /// 2. For instruction types overridden by MemoryBehaviorVisitor, this uses
+  /// alias analysis to disambiguate the Inst's memory effects from the memory
+  /// pointed to by value V. 'mayAlias' is used for memory operations and
+  /// 'getMemoryEffectOnEscapedAddress' is used for calls and releases.
+  ///
+  /// 3. For calls, alias analysis uses callee analysis to retrieve function
+  /// side effects which provides the memory behavior of each argument.
   MemoryBehavior computeMemoryBehavior(SILInstruction *Inst, SILValue V);
 
-  /// Use the alias analysis to determine the memory behavior of Inst with
-  /// respect to V.
-  MemoryBehavior computeMemoryBehaviorInner(SILInstruction *Inst, SILValue V);
-
   /// Returns true if \p Inst may read from memory at address \p V.
   ///
   /// For details see MemoryBehavior::MayRead.
@@ -203,20 +217,37 @@ public:
   /// Returns true if \p Ptr may be released by the builtin \p BI.
   bool canBuiltinDecrementRefCount(BuiltinInst *BI, SILValue Ptr);
 
-  /// Returns true if the address(es of) `addr` can escape to `toInst`.
-  MemoryBehavior getMemoryBehaviorOfInst(SILValue addr, SILInstruction *toInst);
+  int getEstimatedFunctionSize(SILValue valueInFunction);
 
   /// Returns true if the object(s of) `obj` can escape to `toInst`.
+  ///
+  /// Special entry point into BridgedAliasAnalysis (escape analysis) for use in
+  /// ARC analysis.
   bool isObjectReleasedByInst(SILValue obj, SILInstruction *toInst);
 
   /// Is the `addr` within all reachable objects/addresses, when start walking
   /// from `obj`?
+  ///
+  /// Special entry point into BridgedAliasAnalysis (escape analysis) for use in
+  /// ARC analysis.
   bool isAddrVisibleFromObject(SILValue addr, SILValue obj);
 
+  /// MARK: implementation helpers for MemBehaviorVisitor.
+
+  /// If the address(es of) `addr` can escape to `toInst` (based on escape
+  /// analysis), return the memory effect of `toInst` on the escaped memory.
+  ///
+  /// This should not be called directly; it is an implementation helper for
+  /// querying escape analysis.
+  MemoryBehavior getMemoryEffectOnEscapedAddress(SILValue addr, SILInstruction *toInst);
+
+protected:
+  /// Use the alias analysis to determine the memory behavior of Inst with
+  /// respect to V.
+  MemoryBehavior computeMemoryBehaviorInner(SILInstruction *Inst, SILValue V);
+
   /// Returns true if `lhs` can reference the same field as `rhs`.
   bool canReferenceSameField(SILValue lhs, SILValue rhs);
-
-  int getEstimatedFunctionSize(SILValue valueInFunction);
 };
 
 

lib/SILOptimizer/Analysis/AliasAnalysis.cpp

@@ -711,7 +711,7 @@ void BridgedAliasAnalysis::registerAnalysis(GetMemEffectFn getMemEffectsFn,
   canReferenceSameFieldFunction = canReferenceSameFieldFn;
 }
 
-MemoryBehavior AliasAnalysis::getMemoryBehaviorOfInst(
+MemoryBehavior AliasAnalysis::getMemoryEffectOnEscapedAddress(
             SILValue addr, SILInstruction *toInst) {
   if (getMemEffectsFunction) {
     return (MemoryBehavior)getMemEffectsFunction({PM->getSwiftPassInvocation()}, {addr},

lib/SILOptimizer/Analysis/MemoryBehavior.cpp

@@ -317,50 +317,50 @@ MemBehavior MemoryBehaviorVisitor::visitMarkUnresolvedMoveAddrInst(
 MemBehavior MemoryBehaviorVisitor::visitBuiltinInst(BuiltinInst *BI) {
   MemBehavior mb = BI->getMemoryBehavior();
   if (mb != MemBehavior::None) {
-    return AA->getMemoryBehaviorOfInst(V, BI);
+    return AA->getMemoryEffectOnEscapedAddress(V, BI);
   }
   return MemBehavior::None;
 }
 
 MemBehavior MemoryBehaviorVisitor::visitTryApplyInst(TryApplyInst *AI) {
-  return AA->getMemoryBehaviorOfInst(V, AI);
+  return AA->getMemoryEffectOnEscapedAddress(V, AI);
 }
 
 MemBehavior MemoryBehaviorVisitor::visitApplyInst(ApplyInst *AI) {
-  return AA->getMemoryBehaviorOfInst(V, AI);
+  return AA->getMemoryEffectOnEscapedAddress(V, AI);
 }
 
 MemBehavior MemoryBehaviorVisitor::visitBeginApplyInst(BeginApplyInst *AI) {
-  return AA->getMemoryBehaviorOfInst(V, AI);
+  return AA->getMemoryEffectOnEscapedAddress(V, AI);
 }
 
 MemBehavior MemoryBehaviorVisitor::visitEndApplyInst(EndApplyInst *EAI) {
-  return AA->getMemoryBehaviorOfInst(V, EAI->getBeginApply());
+  return AA->getMemoryEffectOnEscapedAddress(V, EAI->getBeginApply());
 }
 
 MemBehavior MemoryBehaviorVisitor::visitAbortApplyInst(AbortApplyInst *AAI) {
-  return AA->getMemoryBehaviorOfInst(V, AAI->getBeginApply());
+  return AA->getMemoryEffectOnEscapedAddress(V, AAI->getBeginApply());
 }
 
 MemBehavior
 MemoryBehaviorVisitor::visitStrongReleaseInst(StrongReleaseInst *SI) {
-  return AA->getMemoryBehaviorOfInst(V, SI);
+  return AA->getMemoryEffectOnEscapedAddress(V, SI);
 }
 
 #define ALWAYS_OR_SOMETIMES_LOADABLE_CHECKED_REF_STORAGE(Name, ...) \
 MemBehavior \
 MemoryBehaviorVisitor::visit##Name##ReleaseInst(Name##ReleaseInst *SI) { \
-  return AA->getMemoryBehaviorOfInst(V, SI); \
+  return AA->getMemoryEffectOnEscapedAddress(V, SI); \
 }
 #include "swift/AST/ReferenceStorage.def"
 
 MemBehavior MemoryBehaviorVisitor::visitReleaseValueInst(ReleaseValueInst *SI) {
-  return AA->getMemoryBehaviorOfInst(V, SI);
+  return AA->getMemoryEffectOnEscapedAddress(V, SI);
 }
 
 MemBehavior
 MemoryBehaviorVisitor::visitDestroyValueInst(DestroyValueInst *DVI) {
-  return AA->getMemoryBehaviorOfInst(V, DVI);
+  return AA->getMemoryEffectOnEscapedAddress(V, DVI);
 }
 
 MemBehavior MemoryBehaviorVisitor::visitSetDeallocatingInst(SetDeallocatingInst *SDI) {