[sil-cse] Fix a bug in the CSE of open_existential_ref instructions

When performing a CSE of open_existential_ref instructions, we replace the new archetype by the old archetype by cloning the uses and re-mapping the archetypes. But we also need to consider that some of the uses of a open_existential_ref instruction (e.g. loads) may produce results depending on the opened archetype being replaced. Therefore, for every such use its own uses (and their uses) should be eventually recursively cloned and type-remapped as well if they depend on the opened archetype being replaced.

Fixes rdar://28136015 and https://bugs.swift.org/browse/SR-2545

Author: swiftix

lib/SILOptimizer/Transforms/CSE.cpp

@@ -681,13 +681,48 @@ bool CSE::processOpenExistentialRef(SILInstruction *Inst, ValueBase *V,
   auto &Builder = Cloner.getBuilder();
   Builder.setOpenedArchetypesTracker(&OpenedArchetypesTracker);
 
+  llvm::SmallPtrSet<SILInstruction *, 16> Processed;
   // Now clone each candidate and replace the opened archetype
   // by a dominating one.
-  for (auto Candidate : Candidates) {
+  while (!Candidates.empty()) {
+    auto Candidate = Candidates.pop_back_val();
+    if (Processed.count(Candidate))
+      continue;
+    // True if a candidate depends on the old opened archetype.
+    bool DependsOnOldOpenedArchetype = !Candidate->getTypeDependentOperands().empty();
+    if (!Candidate->use_empty() &&
+        Candidate->getType().getSwiftRValueType()->hasOpenedExistential()) {
+      // Check if the result type of the candidate depends on the opened
+      // existential in question.
+      auto ResultDependsOnOldOpenedArchetype =
+          Candidate->getType().getSwiftRValueType().findIf(
+              [&OldOpenedArchetype](Type t) -> bool {
+                if (t.getCanonicalTypeOrNull() == OldOpenedArchetype)
+                  return true;
+                return false;
+              });
+      if (ResultDependsOnOldOpenedArchetype) {
+        DependsOnOldOpenedArchetype |= ResultDependsOnOldOpenedArchetype;
+        // We need to update uses of this candidate, because their types
+        // may be affected.
+        for (auto Use : Candidate->getUses()) {
+          Candidates.insert(Use->getUser());
+        }
+      }
+    }
+    // Remember that this candidate was processed already.
+    Processed.insert(Candidate);
+
+    // No need to clone if there is no dependency on the old opened archetype.
+    if (!DependsOnOldOpenedArchetype)
+      continue;
+
     Builder.getOpenedArchetypes().addOpenedArchetypeOperands(
         Candidate->getTypeDependentOperands());
     Builder.setInsertionPoint(Candidate);
     auto NewI = Cloner.clone(Candidate);
+    // Result types of candidate's uses instructions may be using this archetype.
+    // Thus, we need to try to replace it there.
     Candidate->replaceAllUsesWith(NewI);
     if (I == Candidate->getIterator())
       I = NewI->getIterator();

test/SILOptimizer/cse.sil

@@ -1325,6 +1325,8 @@ protocol Proto : class {
 }
 
 // Check that all open_existential_ref instructions are CSEd 
+// Any reference to the $@opened("1B685052-4796-11E6-B7DF-B8E856428C60") Proto should be replaced by
+// references to $@opened("1B68354A-4796-11E6-B7DF-B8E856428C60") Proto.
 // CHECK-LABEL: sil @cse_open_existential : $@convention(thin) (@guaranteed Proto, Bool) -> ()
 // CHECK: bb0
 // CHECK: %[[OPENED_EXISTENTIAL:[0-9]+]] = open_existential_ref %{{[0-9]+}} : $Proto to $@opened("1B68354A-4796-11E6-B7DF-B8E856428C60") Proto
@@ -1334,10 +1336,12 @@ protocol Proto : class {
 // CHECK: bb1:
 // CHECK-NEXT: %[[WM2:[0-9]+]] = witness_method $@opened("1B68354A-4796-11E6-B7DF-B8E856428C60") Proto, #Proto.doThat!1
 // CHECK-NEXT: apply %[[WM2]]{{.*}}(%[[OPENED_EXISTENTIAL]])
-// CHECK-NEXT: br bb
+// CHECK-NOT: 1B6851A6-4796-11E6-B7DF-B8E856428C60
+// CHECK: br bb
 // CHECK: bb2:
 // CHECK-NEXT: apply %[[WM1]]{{.*}}(%[[OPENED_EXISTENTIAL]])
-// CHECK-NEXT: br bb
+// CHECK-NOT: 1B6851A6-4796-11E6-B7DF-B8E856428C60
+// CHECK: br bb
 // CHECK: bb3:
 // CHECK-NEXT: tuple
 // CHECK-NEXT: return
@@ -1359,10 +1363,16 @@ bb2:                                              // Preds: bb0
   %13 = open_existential_ref %0 : $Proto to $@opened("1B6851A6-4796-11E6-B7DF-B8E856428C60") Proto
   %14 = witness_method $@opened("1B6851A6-4796-11E6-B7DF-B8E856428C60") Proto, #Proto.doThis!1, %13 : $@opened("1B6851A6-4796-11E6-B7DF-B8E856428C60") Proto : $@convention(witness_method) <τ_0_0 where τ_0_0 : Proto> (@guaranteed τ_0_0) -> ()
   %15 = apply %14<@opened("1B6851A6-4796-11E6-B7DF-B8E856428C60") Proto>(%13) : $@convention(witness_method) <τ_0_0 where τ_0_0 : Proto> (@guaranteed τ_0_0) -> ()
+  %16 = alloc_stack $@opened("1B6851A6-4796-11E6-B7DF-B8E856428C60") Proto
+  store %13 to %16 : $*@opened("1B6851A6-4796-11E6-B7DF-B8E856428C60") Proto
+  // This is to check that result types of instructions are updated by CSE as well.
+  %17 = load %16 : $*@opened("1B6851A6-4796-11E6-B7DF-B8E856428C60") Proto
+  strong_release %17 : $@opened("1B6851A6-4796-11E6-B7DF-B8E856428C60") Proto
+  dealloc_stack %16 : $*@opened("1B6851A6-4796-11E6-B7DF-B8E856428C60") Proto
   br bb3
 
 bb3:
-  %17 = tuple ()
-  return %17 : $()
+  %20 = tuple ()
+  return %20 : $()
 }