[AddressLowering] Place projections below types.

Projections may involve opened archetypes.  They must be dominated by
the instructions which open those archetypes.

When determining the earliest storage point for a value and the point at
which to insert its projections, look for such obstructions in the
projection chain.  The first one found is the earliest storage point.

Author: nate-chandler

lib/SILOptimizer/Mandatory/AddressLowering.cpp

@@ -571,6 +571,13 @@ struct AddressLoweringState {
   void getDominandsForUseProjection(SILValue userValue,
                                     SmallVectorImpl<SILValue> &dominands) const;
 
+  /// Finds and caches the latest opening instruction of the type of the value
+  /// in \p pair.
+  ///
+  /// @returns nullable instruction
+  SILInstruction *
+  getLatestOpeningInst(const ValueStorageMap::ValueStoragePair *) const;
+
   /// The latest instruction which opens an archetype involved in the indicated
   /// type.
   ///
@@ -1256,6 +1263,14 @@ void OpaqueStorageAllocation::allocateValue(SILValue value) {
   // this value's storage with a branch use.
   createStackAllocationStorage(value);
 }
+SILInstruction *AddressLoweringState::getLatestOpeningInst(
+    const ValueStorageMap::ValueStoragePair *pair) const {
+  if (!pair->storage.latestOpeningInst.has_value()) {
+    auto *loi = getLatestOpeningInst(pair->value->getType());
+    pair->storage.latestOpeningInst = {loi};
+  }
+  return pair->storage.latestOpeningInst.value();
+}
 
 void AddressLoweringState::getDominandsForUseProjection(
     SILValue userValue, SmallVectorImpl<SILValue> &dominands) const {
@@ -1283,6 +1298,12 @@ void AddressLoweringState::getDominandsForUseProjection(
     assert(storage.isUseProjection || !storage.isProjection());
     assert(!(storage.isProjection() && storage.storageAddress) &&
            "projections have not yet been materialized!?");
+    if (auto *loi = getLatestOpeningInst(pair)) {
+      // In order for an opaque value to reuse the storage of some recursive
+      // aggregate, it must dominate the instructions that open archetypes that
+      // occur at every layer of the aggregation.
+      dominands.push_back(cast<SingleValueInstruction>(loi));
+    }
     if (!storage.isProjection()) {
       // Reached the bottom of the projection tower.  There must be storage.
       assert(storage.storageAddress);
@@ -1348,13 +1369,15 @@ bool OpaqueStorageAllocation::findProjectionIntoUseImpl(
       continue;
     }
 
-    // Recurse through all storage projections to find the point where the
-    // storage has been allocated.
+    // Recurse through all storage projections to find (1) the point where the
+    // storage has been allocated and (2) any opening instructions involved in
+    // any of those projections' types.
     //
-    // The base storage address must dominate `incomingValues` because the
-    // address projection for each `incomingValue` must be materialized no
-    // later than at `incomingValue->getDefiningInsertionPoint()` (but perhaps
-    // earlier, see getProjectionInsertionPoint).
+    // The base storage address and all of the opened types used by the
+    // projections must dominate `incomingValues` because the address
+    // projections for each `incomingValue` must be materialized no later than
+    // at `incomingValue->getDefiningInsertionPoint()` (but perhaps earlier,
+    // see getProjectionInsertionPoint).
     SmallVector<SILValue, 4> dominands;
     pass.getDominandsForUseProjection(userValue, dominands);
 
@@ -1830,6 +1853,7 @@ AddressMaterialization::materializeProjectionIntoUse(Operand *operand,
 
 SILInstruction *AddressMaterialization::getProjectionInsertionPoint(
     SILValue userValue, AddressLoweringState &pass) {
+  SILInstruction *latestOpeningInst = nullptr;
   SILInstruction *retval = userValue->getDefiningInsertionPoint();
   for (auto *pair : pass.valueStorageMap.getProjections(userValue)) {
     auto const &storage = pair->storage;
@@ -1838,8 +1862,33 @@ SILInstruction *AddressMaterialization::getProjectionInsertionPoint(
       retval = storage.storageAddress->getNextInstruction();
       break;
     }
+    // It's necessary to consider obstructions at every level of aggregation*
+    // because there is no ordering among the opening instructions which
+    // define the types used in the aggregate.
+    //
+    // * Levels above the first projection for which storage has already been
+    //   allocated, however, do not need to be considered _here_ because they
+    //   were already considered when determining where to create that
+    //   instruction, either in getProjectionInsertionPoint or in
+    //   OpaqueStorageAllocation::createStackAllocation.
+    if (auto *loi = pass.getLatestOpeningInst(pair)) {
+      if (latestOpeningInst) {
+        if (pass.domInfo->dominates(loi, latestOpeningInst)) {
+          continue;
+
+          assert(pass.domInfo->dominates(latestOpeningInst, loi));
+        }
+      }
+      latestOpeningInst = loi->getNextInstruction();
+    }
   }
   assert(retval);
+  if (latestOpeningInst) {
+    if (pass.domInfo->dominates(retval, latestOpeningInst))
+      retval = latestOpeningInst;
+    else
+      assert(pass.domInfo->dominates(latestOpeningInst, retval));
+  }
   return retval;
 }
 

lib/SILOptimizer/Mandatory/AddressLowering.h

@@ -128,6 +128,10 @@ struct ValueStorage {
   /// after materialization (during instruction rewriting).
   SILValue storageAddress;
 
+  /// The latest instruction which opens an archetype involved in the value's
+  /// type.  Just a cache of getLatestOpeningInst(value).
+  mutable llvm::Optional<SILInstruction *> latestOpeningInst = llvm::None;
+
   /// When either isDefProjection or isUseProjection is set, this refers to the
   /// storage whose "def" this value projects out of or whose operand this
   /// storage projects into via its "use.
@@ -205,6 +209,7 @@ struct ValueStorage {
 /// Mapped values are expected to be created in a single RPO pass. "erase" is
 /// unsupported. Values must be replaced using 'replaceValue()'.
 class ValueStorageMap {
+public:
   struct ValueStoragePair {
     SILValue value;
     ValueStorage storage;
@@ -213,6 +218,8 @@ class ValueStorageMap {
     void dump() const;
 #endif
   };
+
+private:
   typedef std::vector<ValueStoragePair> ValueVector;
   // Hash of values to ValueVector indices.
   typedef llvm::DenseMap<SILValue, unsigned> ValueHashMap;

test/SILOptimizer/address_lowering.sil

@@ -56,6 +56,11 @@ struct Pair<T> {
   var y : T
 }
 
+struct Box2<T, U> {
+    var v1: T
+    var v2: U
+}
+
 enum Mixed<T> {
   case i(Int)
   case t(T)
@@ -81,9 +86,11 @@ public protocol Comparable {
   static func < (lhs: Self, rhs: Self) -> Bool
 }
 
+
 sil [ossa] @unknown : $@convention(thin) () -> ()
 sil [ossa] @getT : $@convention(thin) <T> () -> @out T
 sil [ossa] @getPair : $@convention(thin) <T> () -> @out Pair<T>
+sil [ossa] @getOwned : $@convention(thin) <T : AnyObject> () -> (@owned T)
 sil [ossa] @takeGuaranteedObject : $@convention(thin) (@guaranteed AnyObject) -> ()
 sil [ossa] @takeIndirectClass : $@convention(thin) (@in_guaranteed C) -> ()
 sil [ossa] @takeTuple : $@convention(thin) <τ_0_0> (@in_guaranteed (τ_0_0, C)) -> ()
@@ -1090,6 +1097,48 @@ bb0(%error_exi : @guaranteed $any Error):
   return %tuple : $()
 }
 
+// Test a chain of projections multiple nodes of which feature an opened
+// archetype AND the archetype-definining instruction for the first projection
+// whose type has an opened archetype dominates all blocks but the
+// archetype-defining instruction for the SECOND projection does not.
+//
+// This test case demonstrates why getDominandsForUseProjection returns
+// multiple values all of which must dominate each incoming value.
+//
+// CHECK-LABEL: sil [ossa] @f165_testOpenedArchetypsDominance : {{.*}} {
+// CHECK:         alloc_stack $(T, T)
+// CHECK-LABEL: } // end sil function 'f165_testOpenedArchetypsDominance'
+sil [ossa] @f165_testOpenedArchetypsDominance : $@convention(thin) <T> () -> () {
+  %borrow = function_ref @takeInGuaranteed : $@convention(thin) <T> (@in_guaranteed T) -> ()
+  %take = function_ref @takeIn : $@convention(thin) <T> (@in T) -> ()
+  %get = function_ref @getT : $@convention(thin) <T> () -> (@out T)
+  %getOwned = function_ref @getOwned : $@convention(thin) <T : AnyObject> () -> (@owned T)
+
+
+  %exi = apply %getOwned<any P & C>() : $@convention(thin) <T : AnyObject> () -> (@owned T)
+  %exi2 = apply %getOwned<any P & C>() : $@convention(thin) <T : AnyObject> () -> (@owned T)
+  %O_1 = apply %get<T>() : $@convention(thin) <T> () -> (@out T)
+  %O_2 = apply %get<T>() : $@convention(thin) <T> () -> (@out T)
+  %O = tuple (%O_1 : $T, %O_2 : $T)
+  %ope = open_existential_ref %exi : $P & C to $@opened("00000000-0000-0000-0000-000000000000", P & C) Self
+  cond_br undef, left, right
+
+left:
+  %ope2 = open_existential_ref %exi2 : $P & C to $@opened("00000000-0000-0000-0000-000000000001", P & C) Self
+  %U = struct $Box2<(T, T), @opened("00000000-0000-0000-0000-000000000000", P & C) Self> (%O : $(T, T), %ope : $@opened("00000000-0000-0000-0000-000000000000", P & C) Self)
+  %U2 = struct $Box2<Box2<(T, T), @opened("00000000-0000-0000-0000-000000000000", P & C) Self>, @opened("00000000-0000-0000-0000-000000000001", P & C) Self> (%U : $Box2<(T, T), @opened("00000000-0000-0000-0000-000000000000", P & C) Self> , %ope2 : $@opened("00000000-0000-0000-0000-000000000001", P & C) Self)
+  destroy_value %U2 : $Box2<Box2<(T, T), @opened("00000000-0000-0000-0000-000000000000", P & C) Self>, @opened("00000000-0000-0000-0000-000000000001", P & C) Self>
+  br exit
+right:
+  destroy_value %exi2 : $any P & C
+  destroy_value %ope : $@opened("00000000-0000-0000-0000-000000000000", P & C) Self
+  destroy_value %O : $(T, T)
+  br exit
+exit:
+  %retval = tuple ()
+  return %retval : $()
+}
+
 // CHECK-LABEL: sil [ossa] @f170_compare : $@convention(thin) <T where T : Comparable> (@in_guaranteed T, @in_guaranteed T) -> @out T {
 // CHECK: bb0(%0 : $*T, %1 : $*T, %2 : $*T):
 // CHECK:   [[WT:%.*]] = witness_method $T, #Comparable."<" : <Self where Self : Comparable> (Self.Type) -> (Self, Self) -> Builtin.Int1 : $@convention(witness_method: Comparable) <τ_0_0 where τ_0_0 : Comparable> (@in_guaranteed τ_0_0, @in_guaranteed τ_0_0, @thick τ_0_0.Type) -> Builtin.Int1