[SIL-opaque] add a test case for phi coalescing

Avoid attempting to coalesce enum payloads.

Author: atrick

lib/SILOptimizer/Mandatory/PhiStorageOptimizer.cpp

@@ -116,7 +116,6 @@ void PhiStorageOptimizer::optimize() {
     coalescedPhi.coalescedOperands.push_back(phi.getOperand(predecessor));
     return;
   }
-  occupiedBlocks.insert(phi.phiBlock);
   for (auto *incomingPred : phi.phiBlock->getPredecessorBlocks()) {
     tryCoalesceOperand(incomingPred);
   }
@@ -145,14 +144,15 @@ bool PhiStorageOptimizer::canCoalesceValue(SILValue incomingVal) {
 
   auto &incomingStorage = valueStorageMap.getStorage(incomingVal);
 
-  // If the incoming use is pre-allocated it can't be coalesced.
-  // This also handles incoming values that are already coalesced with
-  // another use.
+  // If the incoming use directly reuses its def storage, projects out of its
+  // def storage, or is pre-allocated, then it can't be coalesced. When incoming
+  // storage is directly reused, isAllocated() is false. isProjection() covers
+  // the other cases.
   //
   // Coalescing use projections from incomingVal into its other non-phi uses
-  // would require by recursively following uses across projections when
-  // computing liveness.
-  if (incomingStorage.isProjection())
+  // could be handled, but would require by recursively following uses across
+  // projections when computing liveness.
+  if (!incomingStorage.isAllocated() || incomingStorage.isProjection())
     return false;
 
   auto *defInst = incomingVal->getDefiningInstruction();
@@ -163,7 +163,6 @@ bool PhiStorageOptimizer::canCoalesceValue(SILValue incomingVal) {
     // analysis of the whole phi web before coalescing phi operands.
     return false;
   }
-  assert(incomingStorage.isAllocated() && "nonphi must be allocated");
 
   // Don't coalesce an incoming value unless it's storage is from a stack
   // allocation, which can be replaced with another alloc_stack.
@@ -213,7 +212,11 @@ bool PhiStorageOptimizer::recordUseLiveness(SILValue incomingVal,
   for (auto *use : incomingVal->getUses()) {
     StackList<SILBasicBlock *> liveBBWorklist(getFunction());
 
+    // If \p liveBB is already occupied by another value, return
+    // false. Otherwise, mark \p liveBB live and push it onto liveBBWorklist.
     auto visitLiveBlock = [&](SILBasicBlock *liveBB) {
+      assert(liveBB != phi.phiBlock && "phi operands are consumed");
+
       if (occupiedBlocks.contains(liveBB))
         return false;
 

test/SILOptimizer/address_lowering_phi.sil

@@ -10,6 +10,11 @@ typealias AnyObject = Builtin.AnyObject
 typealias Int = Builtin.Int64
 typealias Bool = Builtin.Int1
 
+enum Optional<T> {
+  case none
+  case some(T)
+}
+
 struct SRef<T> {
   @_hasStorage var object: AnyObject { get set }
   @_hasStorage var element: T { get set }
@@ -438,3 +443,30 @@ bb6(%phi6 : @owned $InnerStruct<T>):
   %outer = struct $OuterStruct<T> (%phi6 : $InnerStruct<T>, %3 : $AnyObject)
   return %outer : $OuterStruct<T>
 }
+
+// CHECK-LABEL: sil [ossa] @f090_payloadPhiOperand : $@convention(thin) <T> (@in Optional<T>, @in T) -> @out T {
+// CHECK: bb0(%0 : $*T, %1 : $*Optional<T>, %2 : $*T):
+// CHECK:   cond_br undef, bb2, bb1
+// CHECK: bb1:
+// CHECK:   destroy_addr %2 : $*T
+// CHECK:   [[P:%.*]] = unchecked_take_enum_data_addr %1 : $*Optional<T>, #Optional.some!enumelt
+// CHECK:   copy_addr [take] [[P]] to [initialization] %0 : $*T
+// CHECK:   br bb3
+// CHECK: bb2:
+// CHECK:   destroy_addr %1 : $*Optional<T>
+// CHECK:   copy_addr [take] %2 to [initialization] %0 : $*T
+// CHECK:   br bb3
+// CHECK-LABEL: } // end sil function 'f090_payloadPhiOperand'
+sil [ossa] @f090_payloadPhiOperand : $@convention(thin) <T> (@in Optional<T>, @in T) -> @out T {
+bb0(%0 : @owned $Optional<T>, %1 : @owned $T):
+  cond_br undef, bb2, bb1
+bb1:
+  destroy_value %1 : $T
+  %payload = unchecked_enum_data %0 : $Optional<T>, #Optional.some!enumelt
+  br bb3(%payload : $T)
+bb2:
+  destroy_value %0 : $Optional<T>
+  br bb3(%1 : $T)
+bb3(%phi : @owned $T):
+  return %phi : $T
+}