CSE: visit uses of block arguments when replacing an open_existental_ref

Fixes a verifier crash.

rdar://122190393

Author: eeckstein

lib/SILOptimizer/Transforms/CSE.cpp

@@ -832,7 +832,8 @@ bool CSE::processLazyPropertyGetters(SILFunction &F) {
 /// according to the provided type substitution map.
 static void updateBasicBlockArgTypes(SILBasicBlock *BB,
                                      ArchetypeType *OldOpenedArchetype,
-                                     ArchetypeType *NewOpenedArchetype) {
+                                     ArchetypeType *NewOpenedArchetype,
+                                     InstructionWorklist &usersToHandle) {
   // Check types of all BB arguments.
   for (auto *Arg : BB->getSILPhiArguments()) {
     if (!Arg->getType().hasOpenedExistential())
@@ -868,6 +869,7 @@ static void updateBasicBlockArgTypes(SILBasicBlock *BB,
     // Restore all uses to refer to the BB argument with updated type.
     for (auto ArgUse : OriginalArgUses) {
       ArgUse->set(NewArg);
+      usersToHandle.pushIfNotVisited(ArgUse->getUser());
     }
   }
 }
@@ -879,7 +881,7 @@ static void updateBasicBlockArgTypes(SILBasicBlock *BB,
 /// \V is the dominating open_existential_ref instruction
 bool CSE::processOpenExistentialRef(OpenExistentialRefInst *Inst,
                                     OpenExistentialRefInst *VI) {
-  llvm::SmallSetVector<SILInstruction *, 16> Candidates;
+  InstructionWorklist usersToHandle(Inst->getFunction());
   const auto OldOpenedArchetype = Inst->getDefinedOpenedArchetype();
   const auto NewOpenedArchetype = VI->getDefinedOpenedArchetype();
 
@@ -895,8 +897,7 @@ bool CSE::processOpenExistentialRef(OpenExistentialRefInst *Inst,
         }
       }
     }
-
-    Candidates.insert(User);
+    usersToHandle.pushIfNotVisited(User);
   }
 
   // Now process candidates.
@@ -907,43 +908,36 @@ bool CSE::processOpenExistentialRef(OpenExistentialRefInst *Inst,
       OldOpenedArchetype->castTo<ArchetypeType>(), NewOpenedArchetype);
   auto &Builder = Cloner.getBuilder();
 
-  InstructionSet Processed(Inst->getFunction());
   // Now clone each candidate and replace the opened archetype
   // by a dominating one.
-  while (!Candidates.empty()) {
-    auto Candidate = Candidates.pop_back_val();
-    if (Processed.contains(Candidate))
-      continue;
-
-    if (isa<TermInst>(Candidate)) {
+  while (SILInstruction *user = usersToHandle.pop()) {
+    if (isa<TermInst>(user)) {
       // The current use of the opened archetype is a terminator instruction.
       // Check if any of the successor BBs uses this opened archetype in the
       // types of its basic block arguments. If this is the case, replace
       // those uses by the new opened archetype.
-      // FIXME: What about uses of those arguments?
-      for (auto *Successor : Candidate->getParent()->getSuccessorBlocks()) {
+      for (auto *Successor : user->getParent()->getSuccessorBlocks()) {
         if (Successor->args_empty())
           continue;
         // If a BB has any arguments, update their types if necessary.
         updateBasicBlockArgTypes(Successor, OldOpenedArchetype,
-                                 NewOpenedArchetype);
+                                 NewOpenedArchetype, usersToHandle);
       }
     }
 
     // Compute if a candidate depends on the old opened archetype.
     // It always does if it has any type-dependent operands.
     bool DependsOnOldOpenedArchetype =
-      !Candidate->getTypeDependentOperands().empty();
+      !user->getTypeDependentOperands().empty();
 
     // Look for dependencies propagated via the candidate's results.
-    for (auto CandidateResult : Candidate->getResults()) {
-      if (CandidateResult->use_empty() ||
-          !CandidateResult->getType().hasOpenedExistential())
+    for (auto result : user->getResults()) {
+      if (result->use_empty() || !result->getType().hasOpenedExistential())
         continue;
 
       // Check if the result type depends on this specific opened existential.
       auto ResultDependsOnOldOpenedArchetype =
-          CandidateResult->getType().getASTType().findIf(
+          result->getType().getASTType().findIf(
               [&OldOpenedArchetype](Type t) -> bool {
                 return (CanType(t) == OldOpenedArchetype);
               });
@@ -953,24 +947,22 @@ bool CSE::processOpenExistentialRef(OpenExistentialRefInst *Inst,
         DependsOnOldOpenedArchetype = true;
 
         // The users of this candidate are new candidates.
-        for (auto Use : CandidateResult->getUses()) {
-          Candidates.insert(Use->getUser());
+        for (auto Use : result->getUses()) {
+          usersToHandle.pushIfNotVisited(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.setInsertionPoint(Candidate);
-    auto NewI = Cloner.clone(Candidate);
+    Builder.setInsertionPoint(user);
+    auto NewI = Cloner.clone(user);
     // Result types of candidate's uses instructions may be using this archetype.
     // Thus, we need to try to replace it there.
-    Candidate->replaceAllUsesPairwiseWith(NewI);
-    eraseFromParentWithDebugInsts(Candidate);
+    user->replaceAllUsesPairwiseWith(NewI);
+    eraseFromParentWithDebugInsts(user);
   }
   return true;
 }

test/SILOptimizer/cse_objc.sil

@@ -277,3 +277,30 @@ sil_vtable Bar {
   #Bar.walk: @_TFC4test3Bar4walkfS0_FT_T_	// test.Bar.walk (test.Bar)() -> ()
   #Bar.deinit!deallocator: @_TFC4test3BarD	// test.Bar.__deallocating_deinit
 }
+
+@objc public protocol P {
+  @objc optional func f() -> Bool
+}
+
+// CHECK-LABEL: sil @test_open_existential :
+// CHECK:         partial_apply [callee_guaranteed] {{%[0-9]+}}(%1) : $@convention(objc_method) (@opened("2FD52A00-DA20-11EE-8801-0EA13E3AABAF", any P) Self) -> ObjCBool // type-defs: %1
+// CHECK:       } // end sil function 'test_open_existential'
+sil @test_open_existential : $@convention(thin) (@guaranteed P) -> () {
+bb0(%0 : $any P):
+  %1 = open_existential_ref %0 : $any P to $@opened("2FD52A00-DA20-11EE-8801-0EA13E3AABAF", any P) Self
+  fix_lifetime %1 : $@opened("2FD52A00-DA20-11EE-8801-0EA13E3AABAF", any P) Self
+  %2 = open_existential_ref %0 : $any P to $@opened("2FD52A04-DA20-11EE-8801-0EA13E3AABAF", any P) Self
+  br bb2
+
+bb1(%4 : $@convention(objc_method) (@opened("2FD52A04-DA20-11EE-8801-0EA13E3AABAF", any P) Self) -> ObjCBool):
+  %5 = partial_apply [callee_guaranteed] %4(%2) : $@convention(objc_method) (@opened("2FD52A04-DA20-11EE-8801-0EA13E3AABAF", any P) Self) -> ObjCBool
+  %r = tuple ()
+  return %r : $()
+
+bb2:
+  dynamic_method_br %2 : $@opened("2FD52A04-DA20-11EE-8801-0EA13E3AABAF", any P) Self, #P.f!foreign, bb1, bb3
+
+bb3:
+  unreachable
+}
+