Merge pull request #72267 from meg-gupta/enablesilcombineempty

meg-gupta · web-flow · commit 2d27ec581cd2 · 2024-03-12T20:34:09.000-07:00
Enable SILCombine of inject_enum_addr for empty types
diff --git a/include/swift/SILOptimizer/Utils/InstOptUtils.h b/include/swift/SILOptimizer/Utils/InstOptUtils.h
@@ -598,6 +598,12 @@ bool specializeClassMethodInst(ClassMethodInst *cm);
 bool specializeAppliesInFunction(SILFunction &F,
                                  SILTransform *transform,
                                  bool isMandatory);
+
+/// Instantiate the specified type by recursively tupling and structing the
+/// unique instances of the empty types and undef "instances" of the non-empty
+/// types aggregated together at each level.
+SILValue createEmptyAndUndefValue(SILType ty, SILInstruction *insertionPoint,
+                                  SILBuilderContext &ctx, bool noUndef = false);
 } // end namespace swift
 
 #endif // SWIFT_SILOPTIMIZER_UTILS_INSTOPTUTILS_H
diff --git a/lib/SILOptimizer/SILCombiner/SILCombinerMiscVisitors.cpp b/lib/SILOptimizer/SILCombiner/SILCombinerMiscVisitors.cpp
@@ -953,16 +953,6 @@ SILCombiner::visitInjectEnumAddrInst(InjectEnumAddrInst *IEAI) {
   // can't handle the payload case here due to the flow problems caused by the
   // dependency in between the enum and its data.
 
-  // Disable this for empty typle type because empty tuple stack locations maybe
-  // uninitialized. And converting to value form loses tag information.
-  if (IEAI->getElement()->hasAssociatedValues()) {
-    SILType elemType = IEAI->getOperand()->getType().getEnumElementType(
-        IEAI->getElement(), IEAI->getFunction());
-    if (elemType.isEmpty(*IEAI->getFunction())) {
-      return nullptr;
-    }
-  }
-
   assert(IEAI->getOperand()->getType().isAddress() && "Must be an address");
   Builder.setCurrentDebugScope(IEAI->getDebugScope());
 
@@ -1231,6 +1221,7 @@ SILCombiner::visitInjectEnumAddrInst(InjectEnumAddrInst *IEAI) {
   auto *AI = dyn_cast_or_null<ApplyInst>(getSingleNonDebugUser(DataAddrInst));
   if (!AI)
     return nullptr;
+
   unsigned ArgIdx = 0;
   Operand *EnumInitOperand = nullptr;
   for (auto &Opd : AI->getArgumentOperands()) {
@@ -1255,10 +1246,22 @@ SILCombiner::visitInjectEnumAddrInst(InjectEnumAddrInst *IEAI) {
                                               EnumInitOperand->get()->getType());
   EnumInitOperand->set(AllocStack);
   Builder.setInsertionPoint(std::next(SILBasicBlock::iterator(AI)));
-  SILValue Load(Builder.createLoad(DataAddrInst->getLoc(), AllocStack,
-                                   LoadOwnershipQualifier::Unqualified));
+  SILValue enumValue;
+
+  // If it is an empty type, apply may not initialize it.
+  // Create an empty value of the empty type and store it to a new local.
+  SILType elemType = IEAI->getOperand()->getType().getEnumElementType(
+      IEAI->getElement(), IEAI->getFunction());
+  if (elemType.isEmpty(*IEAI->getFunction())) {
+    enumValue = createEmptyAndUndefValue(
+        elemType.getObjectType(), &*Builder.getInsertionPoint(),
+        Builder.getBuilderContext(), /*noUndef*/ true);
+  } else {
+    enumValue = Builder.createLoad(DataAddrInst->getLoc(), AllocStack,
+                                   LoadOwnershipQualifier::Unqualified);
+  }
   EnumInst *E = Builder.createEnum(
-      DataAddrInst->getLoc(), Load, DataAddrInst->getElement(),
+      DataAddrInst->getLoc(), enumValue, DataAddrInst->getElement(),
       DataAddrInst->getOperand()->getType().getObjectType());
   Builder.createStore(DataAddrInst->getLoc(), E, DataAddrInst->getOperand(),
                       StoreOwnershipQualifier::Unqualified);
diff --git a/lib/SILOptimizer/Transforms/SILMem2Reg.cpp b/lib/SILOptimizer/Transforms/SILMem2Reg.cpp
@@ -679,43 +679,6 @@ replaceLoad(SILInstruction *inst, SILValue newValue, AllocStackInst *asi,
   }
 }
 
-/// Instantiate the specified type by recursively tupling and structing the
-/// unique instances of the empty types and undef "instances" of the non-empty
-/// types aggregated together at each level.
-static SILValue createEmptyAndUndefValue(SILType ty,
-                                         SILInstruction *insertionPoint,
-                                         SILBuilderContext &ctx) {
-  auto *function = insertionPoint->getFunction();
-  if (auto tupleTy = ty.getAs<TupleType>()) {
-    SmallVector<SILValue, 4> elements;
-    for (unsigned idx : range(tupleTy->getNumElements())) {
-      SILType elementTy = ty.getTupleElementType(idx);
-      auto element = createEmptyAndUndefValue(elementTy, insertionPoint, ctx);
-      elements.push_back(element);
-    }
-    SILBuilderWithScope builder(insertionPoint, ctx);
-    return builder.createTuple(insertionPoint->getLoc(), ty, elements);
-  } else if (auto *decl = ty.getStructOrBoundGenericStruct()) {
-    TypeExpansionContext tec = *function;
-    auto &module = function->getModule();
-    if (decl->isResilient(tec.getContext()->getParentModule(),
-                          tec.getResilienceExpansion())) {
-      llvm::errs() << "Attempting to create value for illegal empty type:\n";
-      ty.print(llvm::errs());
-      llvm::report_fatal_error("illegal empty type: resilient struct");
-    }
-    SmallVector<SILValue, 4> elements;
-    for (auto *field : decl->getStoredProperties()) {
-      auto elementTy = ty.getFieldType(field, module, tec);
-      auto element = createEmptyAndUndefValue(elementTy, insertionPoint, ctx);
-      elements.push_back(element);
-    }
-    SILBuilderWithScope builder(insertionPoint, ctx);
-    return builder.createStruct(insertionPoint->getLoc(), ty, elements);
-  } else {
-    return SILUndef::get(insertionPoint->getFunction(), ty);
-  }
-}
 
 /// Whether lexical lifetimes should be added for the values stored into the
 /// alloc_stack.
diff --git a/lib/SILOptimizer/Utils/InstOptUtils.cpp b/lib/SILOptimizer/Utils/InstOptUtils.cpp
@@ -1972,3 +1972,40 @@ IntegerLiteralInst *swift::optimizeBuiltinCanBeObjCClass(BuiltinInst *bi,
   }
   llvm_unreachable("Unhandled TypeTraitResult in switch.");
 }
+
+SILValue swift::createEmptyAndUndefValue(SILType ty,
+                                         SILInstruction *insertionPoint,
+                                         SILBuilderContext &ctx,
+                                         bool noUndef) {
+  auto *function = insertionPoint->getFunction();
+  if (auto tupleTy = ty.getAs<TupleType>()) {
+    SmallVector<SILValue, 4> elements;
+    for (unsigned idx : range(tupleTy->getNumElements())) {
+      SILType elementTy = ty.getTupleElementType(idx);
+      auto element = createEmptyAndUndefValue(elementTy, insertionPoint, ctx);
+      elements.push_back(element);
+    }
+    SILBuilderWithScope builder(insertionPoint, ctx);
+    return builder.createTuple(insertionPoint->getLoc(), ty, elements);
+  }
+  if (auto *decl = ty.getStructOrBoundGenericStruct()) {
+    TypeExpansionContext tec = *function;
+    auto &module = function->getModule();
+    if (decl->isResilient(tec.getContext()->getParentModule(),
+                          tec.getResilienceExpansion())) {
+      llvm::errs() << "Attempting to create value for illegal empty type:\n";
+      ty.print(llvm::errs());
+      llvm::report_fatal_error("illegal empty type: resilient struct");
+    }
+    SmallVector<SILValue, 4> elements;
+    for (auto *field : decl->getStoredProperties()) {
+      auto elementTy = ty.getFieldType(field, module, tec);
+      auto element = createEmptyAndUndefValue(elementTy, insertionPoint, ctx);
+      elements.push_back(element);
+    }
+    SILBuilderWithScope builder(insertionPoint, ctx);
+    return builder.createStruct(insertionPoint->getLoc(), ty, elements);
+  }
+  assert(!noUndef);
+  return SILUndef::get(insertionPoint->getFunction(), ty);
+}
diff --git a/test/Inputs/resilient_struct.swift b/test/Inputs/resilient_struct.swift
@@ -119,3 +119,5 @@ public struct UnavailableResilientInt {
     self.i = i
   }
 }
+
+public struct ResilientEmptyStruct {}
diff --git a/test/SILOptimizer/sil_combine_enum_addr.sil b/test/SILOptimizer/sil_combine_enum_addr.sil
@@ -1,10 +1,17 @@
-// RUN: %target-sil-opt -enable-sil-verify-all %s -sil-combine -jumpthread-simplify-cfg | %FileCheck %s
+// RUN: %empty-directory(%t)
+// RUN: %target-swift-frontend -emit-module -enable-library-evolution \
+// RUN:   -emit-module-path=%t/resilient_struct.swiftmodule \
+// RUN:   -module-name=resilient_struct %S/../Inputs/resilient_struct.swift
+
+// RUN: %target-sil-opt -enable-sil-verify-all %s -sil-combine -jumpthread-simplify-cfg -I %t | %FileCheck %s
 
 sil_stage canonical
 
 import Builtin
 import Swift
 
+import resilient_struct
+
 // CHECK-LABEL: sil  @convert_inject_enum_addr_select_enum_addr_into_cond_br : $@convention(thin) (@in Int, @inout _Stdout) -> ()
 // CHECK-NOT: init_existential_addr
 // CHECK-NOT: inject_enum_addr
@@ -254,8 +261,7 @@ bb22:
 sil @no_init : $@convention(thin) () -> (@out ())
 
 // CHECK-LABEL: sil @test_empty_tuple_uninintialized : $@convention(thin) () -> () {
-// CHECK: init_enum_data_addr
-// CHECK: inject_enum_addr
+// CHECK-NOT: inject_enum_addr
 // CHECK-LABEL: } // end sil function 'test_empty_tuple_uninintialized'
 sil @test_empty_tuple_uninintialized : $@convention(thin) () -> () {
 bb0:
@@ -279,3 +285,55 @@ bb3:
   return %8 : $()
 }
 
+sil @no_init_nested_tuple : $@convention(thin) () -> (@out ((), ()))
+
+// CHECK-LABEL: sil @test_empty_nested_tuple_uninintialized : $@convention(thin) () -> () {
+// CHECK-NOT: inject_enum_addr
+// CHECK-LABEL: } // end sil function 'test_empty_nested_tuple_uninintialized'
+sil @test_empty_nested_tuple_uninintialized : $@convention(thin) () -> () {
+bb0:
+  %0 = alloc_stack $Optional<((), ())>
+  %1 = init_enum_data_addr %0 : $*Optional<((), ())>, #Optional.some!enumelt
+  %f = function_ref @no_init_nested_tuple : $@convention(thin) () -> (@out ((), ()))
+  apply %f(%1) : $@convention(thin) () -> (@out ((), ()))
+  inject_enum_addr %0 : $*Optional<((), ())>, #Optional.some!enumelt
+  %2 = load %0 : $*Optional<((), ())>
+  switch_enum %2 : $Optional<((), ())>, case #Optional.some!enumelt: bb1, case #Optional.none!enumelt: bb2
+
+bb1(%4 : $((), ())):
+  br bb3
+
+bb2:
+  br bb3
+
+bb3:
+  dealloc_stack %0 : $*Optional<((), ())>
+  %8 = tuple ()
+  return %8 : $()
+}
+
+sil @no_init_struct : $@convention(thin) () -> (@out ResilientEmptyStruct)
+
+// CHECK-LABEL: sil @test_empty_struct_uninintialized : $@convention(thin) () -> () {
+// CHECK-NOT: switch_enum_addr
+// CHECK-LABEL: } // end sil function 'test_empty_struct_uninintialized'
+sil @test_empty_struct_uninintialized : $@convention(thin) () -> () {
+bb0:
+  %0 = alloc_stack $Optional<ResilientEmptyStruct>
+  %1 = init_enum_data_addr %0 : $*Optional<ResilientEmptyStruct>, #Optional.some!enumelt
+  %f = function_ref @no_init_struct : $@convention(thin) () -> (@out ResilientEmptyStruct)
+  apply %f(%1) : $@convention(thin) () -> (@out ResilientEmptyStruct)
+  inject_enum_addr %0 : $*Optional<ResilientEmptyStruct>, #Optional.some!enumelt
+  switch_enum_addr %0 : $*Optional<ResilientEmptyStruct>, case #Optional.some!enumelt: bb1, case #Optional.none!enumelt: bb2
+
+bb1:
+  br bb3
+
+bb2:
+  br bb3
+
+bb3:
+  dealloc_stack %0 : $*Optional<ResilientEmptyStruct>
+  %8 = tuple ()
+  return %8 : $()
+}

Original file line number	Diff line number	Diff line change
`@@ -119,3 +119,5 @@ public struct UnavailableResilientInt {`
`119`	`119`	`self.i = i`
`120`	`120`	`}`
`121`	`121`	`}`
	`122`	`+`
	`123`	`+public struct ResilientEmptyStruct {}`