Merge pull request #77560 from eeckstein/fix-silcombine

Optimizer: when propagating the concrete type of an existential, make sure to not violate dominance order
swiftlang · Nov 13, 2024 · fa819fe · fa819fe
2 parents af58e6e + 63141b6
commit fa819fe
Show file tree

Hide file tree

Showing 2 changed files with 59 additions and 15 deletions.
diff --git a/lib/SILOptimizer/Utils/InstOptUtils.cpp b/lib/SILOptimizer/Utils/InstOptUtils.cpp
@@ -904,30 +904,34 @@ swift::findInitAddressForTrivialEnum(UncheckedTakeEnumDataAddrInst *utedai) {
   if (!asi)
     return nullptr;
 
-  SILInstruction *singleUser = nullptr;
+  InjectEnumAddrInst *singleInject = nullptr;
+  InitEnumDataAddrInst *singleInit = nullptr;
   for (auto use : asi->getUses()) {
     auto *user = use->getUser();
     if (user == utedai)
       continue;
 
-    // As long as there's only one UncheckedTakeEnumDataAddrInst and one
-    // InitEnumDataAddrInst, we don't care how many InjectEnumAddr and
-    // DeallocStack users there are.
-    if (isa<InjectEnumAddrInst>(user) || isa<DeallocStackInst>(user))
+    // If there is a single init_enum_data_addr and a single inject_enum_addr,
+    // those instructions must dominate the unchecked_take_enum_data_addr.
+    // Otherwise the enum wouldn't be initialized on all control flow paths.
+    if (auto *inj = dyn_cast<InjectEnumAddrInst>(user)) {
+      if (singleInject)
+        return nullptr;
+      singleInject = inj;
       continue;
+    }
 
-    if (singleUser)
-      return nullptr;
+    if (auto *init = dyn_cast<InitEnumDataAddrInst>(user)) {
+      if (singleInit)
+        return nullptr;
+      singleInit = init;
+      continue;
+    }
 
-    singleUser = user;
+    if (isa<DeallocStackInst>(user) || isa<DebugValueInst>(user))
+      continue;
   }
-  if (!singleUser)
-    return nullptr;
-
-  // Assume, without checking, that the returned InitEnumDataAddr dominates the
-  // given UncheckedTakeEnumDataAddrInst, because that's how SIL is defined. I
-  // don't know where this is actually verified.
-  return dyn_cast<InitEnumDataAddrInst>(singleUser);
+  return singleInit;
 }
 
 //===----------------------------------------------------------------------===//

diff --git a/test/SILOptimizer/sil_combine_enums.sil b/test/SILOptimizer/sil_combine_enums.sil
@@ -16,6 +16,11 @@ enum Numerals {
   case Four
 }
 
+protocol P {
+  mutating func foo()
+}
+
+sil @createit : $@convention(thin) <τ_0_0 where τ_0_0 : P> (@thick τ_0_0.Type) -> @out τ_0_0
 sil @external_func: $@convention(thin) () -> ()
 
 //CHECK-LABEL: eliminate_sw_enum_addr
@@ -663,3 +668,38 @@ bb0(%0 : $S):
   return %11 : $()
 }
 
+// CHECK-LABEL: sil @dont_promote_existential_type_from_non_dominating_block :
+//   Just check that this doesn't crash
+// CHECK:       } // end sil function 'dont_promote_existential_type_from_non_dominating_block'
+sil @dont_promote_existential_type_from_non_dominating_block : $@convention(thin) (@thick any P.Type) -> () {
+bb0(%0 : $@thick any P.Type):
+  %1 = alloc_stack $Optional<any P>
+  cond_br undef, bb1, bb2
+
+bb1:
+  inject_enum_addr %1 : $*Optional<any P>, #Optional.none!enumelt
+  br bb3
+
+bb2:
+  %5 = init_enum_data_addr %1 : $*Optional<any P>, #Optional.some!enumelt
+  %6 = open_existential_metatype %0 : $@thick any P.Type to $@thick (@opened("B0100688-9C78-11EF-8A3C-4EA2A866E4C4", any P) Self).Type
+  %7 = init_existential_addr %5 : $*any P, $@opened("B0100688-9C78-11EF-8A3C-4EA2A866E4C4", any P) Self
+  %f = function_ref @createit : $@convention(thin) <τ_0_0 where τ_0_0 : P> (@thick τ_0_0.Type) -> @out τ_0_0
+  %a = apply %f<@opened("B0100688-9C78-11EF-8A3C-4EA2A866E4C4", any P) Self>(%7, %6) : $@convention(thin) <τ_0_0 where τ_0_0 : P> (@thick τ_0_0.Type) -> @out τ_0_0
+  inject_enum_addr %1 : $*Optional<any P>, #Optional.some!enumelt
+  br bb3
+
+bb3:
+  %9 = unchecked_take_enum_data_addr %1 : $*Optional<any P>, #Optional.some!enumelt
+  dealloc_stack %1 : $*Optional<any P>
+  %11 = alloc_stack [lexical] [var_decl] $any P, var, name "comp"
+  copy_addr [take] %9 to [init] %11 : $*any P
+  %13 = open_existential_addr mutable_access %11 : $*any P to $*@opened("36EE10D8-9C78-11EF-8A3C-4EA2A866E4C4", any P) Self
+  %14 = witness_method $@opened("36EE10D8-9C78-11EF-8A3C-4EA2A866E4C4", any P) Self, #P.foo : <Self where Self : P> (inout Self) -> () -> (), %13 : $*@opened("36EE10D8-9C78-11EF-8A3C-4EA2A866E4C4", any P) Self : $@convention(witness_method: P) <τ_0_0 where τ_0_0 : P> (@inout τ_0_0) -> ()
+  %15 = apply %14<@opened("36EE10D8-9C78-11EF-8A3C-4EA2A866E4C4", any P) Self>(%13) : $@convention(witness_method: P) <τ_0_0 where τ_0_0 : P> (@inout τ_0_0) -> ()
+  destroy_addr %11 : $*any P
+  dealloc_stack %11 : $*any P
+  %18 = tuple ()
+  return %18 : $()
+}
+