Merge pull request swiftlang#21172 from eeckstein/silcombine-enums

SILCombine: peephole to propagate resilient enum cases

Author: eeckstein

lib/SILOptimizer/SILCombiner/SILCombinerMiscVisitors.cpp

@@ -106,23 +106,91 @@ SILCombiner::visitAllocExistentialBoxInst(AllocExistentialBoxInst *AEBI) {
   return nullptr;
 }
 
+/// Return the enum case injected by an inject_enum_addr if it is the only
+/// instruction which writes to \p Addr.
+static EnumElementDecl *getInjectEnumCaseTo(SILValue Addr) {
+  while (true) {
+    // For everything else than an alloc_stack we cannot easily prove that we
+    // see all writes.
+    if (!isa<AllocStackInst>(Addr))
+      return nullptr;
+
+    SILInstruction *WritingInst = nullptr;
+    int NumWrites = 0;
+    for (auto *Use : getNonDebugUses(Addr)) {
+      SILInstruction *User = Use->getUser();
+      switch (User->getKind()) {
+        // Handle a very narrow set of known not harmful instructions.
+        case swift::SILInstructionKind::DestroyAddrInst:
+        case swift::SILInstructionKind::DeallocStackInst:
+        case swift::SILInstructionKind::SwitchEnumAddrInst:
+          break;
+        case swift::SILInstructionKind::ApplyInst:
+        case swift::SILInstructionKind::TryApplyInst: {
+          // Check if the addr is only passed to in_guaranteed arguments.
+          FullApplySite AI(User);
+          for (Operand &Op : AI.getArgumentOperands()) {
+            if (Op.get() == Addr &&
+                AI.getArgumentConvention(Op) !=
+                  SILArgumentConvention::Indirect_In_Guaranteed)
+              return nullptr;
+          }
+          break;
+        }
+        case swift::SILInstructionKind::InjectEnumAddrInst:
+          WritingInst = User;
+          ++NumWrites;
+          break;
+        case swift::SILInstructionKind::CopyAddrInst:
+          if (Addr == cast<CopyAddrInst>(User)->getDest()) {
+            WritingInst = User;
+            ++NumWrites;
+          }
+          break;
+        default:
+          return nullptr;
+      }
+    }
+    if (NumWrites != 1)
+      return nullptr;
+    if (auto *IEA = dyn_cast<InjectEnumAddrInst>(WritingInst))
+      return IEA->getElement();
+
+    // In case of a copy_addr continue with the source of the copy.
+    Addr = dyn_cast<CopyAddrInst>(WritingInst)->getSrc();
+  }
+}
+
 SILInstruction *SILCombiner::visitSwitchEnumAddrInst(SwitchEnumAddrInst *SEAI) {
+  // Convert switch_enum_addr -> br
+  // if the only thing which writes to the address is an inject_enum_addr.
+  SILValue Addr = SEAI->getOperand();
+  if (EnumElementDecl *EnumCase = getInjectEnumCaseTo(Addr)) {
+    SILBasicBlock *Dest = SEAI->getCaseDestination(EnumCase);
+    // If the only instruction which writes to Addr is an inject_enum_addr we
+    // know that there cannot be an enum payload.
+    assert(Dest->getNumArguments() == 0 &&
+           "didn't expect a payload argument");
+    Builder.createBranch(SEAI->getLoc(), Dest);
+    return eraseInstFromFunction(*SEAI);
+  }
+
+  SILType Ty = Addr->getType();
+  if (!Ty.isLoadable(SEAI->getModule()))
+    return nullptr;
+
   // Promote switch_enum_addr to switch_enum if the enum is loadable.
   //   switch_enum_addr %ptr : $*Optional<SomeClass>, case ...
   //     ->
   //   %value = load %ptr
   //   switch_enum %value
-  SILType Ty = SEAI->getOperand()->getType();
-  if (!Ty.isLoadable(SEAI->getModule()))
-    return nullptr;
-
   SmallVector<std::pair<EnumElementDecl*, SILBasicBlock*>, 8> Cases;
   for (int i = 0, e = SEAI->getNumCases(); i < e; ++i)
     Cases.push_back(SEAI->getCase(i));
 
   Builder.setCurrentDebugScope(SEAI->getDebugScope());
   SILBasicBlock *Default = SEAI->hasDefault() ? SEAI->getDefaultBB() : nullptr;
-  LoadInst *EnumVal = Builder.createLoad(SEAI->getLoc(), SEAI->getOperand(),
+  LoadInst *EnumVal = Builder.createLoad(SEAI->getLoc(), Addr,
                                          LoadOwnershipQualifier::Unqualified);
   Builder.createSwitchEnum(SEAI->getLoc(), EnumVal, Default, Cases);
   return eraseInstFromFunction(*SEAI);

lib/SILOptimizer/Transforms/DeadObjectElimination.cpp

@@ -193,7 +193,8 @@ removeInstructions(ArrayRef<SILInstruction*> UsersToRemove) {
 
 /// Returns false if Inst is an instruction that would require us to keep the
 /// alloc_ref alive.
-static bool canZapInstruction(SILInstruction *Inst, bool acceptRefCountInsts) {
+static bool canZapInstruction(SILInstruction *Inst, bool acceptRefCountInsts,
+                              bool onlyAcceptTrivialStores) {
   if (isa<SetDeallocatingInst>(Inst) || isa<FixLifetimeInst>(Inst))
     return true;
 
@@ -204,6 +205,24 @@ static bool canZapInstruction(SILInstruction *Inst, bool acceptRefCountInsts) {
       isa<DeallocPartialRefInst>(Inst))
     return acceptRefCountInsts;
 
+  if (isa<InjectEnumAddrInst>(Inst))
+    return true;
+
+  // We know that the destructor has no side effects so we can remove the
+  // deallocation instruction too.
+  if (isa<DeallocationInst>(Inst) || isa<AllocationInst>(Inst))
+    return true;
+
+  // Much like deallocation, destroy addr is safe.
+  if (isa<DestroyAddrInst>(Inst))
+    return true;
+
+  // The only store instructions which is guaranteed to store a trivial value
+  // is an inject_enum_addr witout a payload (i.e. without init_enum_data_addr).
+  // There can also be a 'store [trivial]', but we don't handle that yet.
+  if (onlyAcceptTrivialStores)
+    return false;
+
   // If we see a store here, we have already checked that we are storing into
   // the pointer before we added it to the worklist, so we can skip it.
   if (isa<StoreInst>(Inst))
@@ -215,15 +234,6 @@ static bool canZapInstruction(SILInstruction *Inst, bool acceptRefCountInsts) {
       !isa<TermInst>(Inst))
     return true;
 
-  // We know that the destructor has no side effects so we can remove the
-  // deallocation instruction too.
-  if (isa<DeallocationInst>(Inst))
-    return true;
-
-  // Much like deallocation, destroy addr is safe.
-  if (isa<DestroyAddrInst>(Inst))
-    return true;
-
   // Otherwise we do not know how to handle this instruction. Be conservative
   // and don't zap it.
   return false;
@@ -233,7 +243,7 @@ static bool canZapInstruction(SILInstruction *Inst, bool acceptRefCountInsts) {
 /// zapping it completely.
 static bool
 hasUnremovableUsers(SILInstruction *AllocRef, UserList &Users,
-                    bool acceptRefCountInsts) {
+                    bool acceptRefCountInsts, bool onlyAcceptTrivialStores) {
   SmallVector<SILInstruction *, 16> Worklist;
   Worklist.push_back(AllocRef);
 
@@ -252,7 +262,7 @@ hasUnremovableUsers(SILInstruction *AllocRef, UserList &Users,
     }
 
     // If we can't zap this instruction... bail...
-    if (!canZapInstruction(I, acceptRefCountInsts)) {
+    if (!canZapInstruction(I, acceptRefCountInsts, onlyAcceptTrivialStores)) {
       LLVM_DEBUG(llvm::dbgs() << "        Found instruction we can't zap...\n");
       return true;
     }
@@ -701,7 +711,8 @@ bool DeadObjectElimination::processAllocRef(AllocRefInst *ARI) {
   // escape, then we can completely remove the use graph of this alloc_ref.
   UserList UsersToRemove;
   if (hasUnremovableUsers(ARI, UsersToRemove,
-                          /*acceptRefCountInsts=*/ !HasSideEffects)) {
+                          /*acceptRefCountInsts=*/ !HasSideEffects,
+                          /*onlyAcceptTrivialStores*/false)) {
     LLVM_DEBUG(llvm::dbgs() << "    Found a use that cannot be zapped...\n");
     return false;
   }
@@ -716,12 +727,11 @@ bool DeadObjectElimination::processAllocRef(AllocRefInst *ARI) {
 }
 
 bool DeadObjectElimination::processAllocStack(AllocStackInst *ASI) {
-  // Trivial types don't have destructors. Let's try to zap this AllocStackInst.
-  if (!ASI->getElementType().isTrivial(ASI->getModule()))
-    return false;
-
+  // Trivial types don't have destructors.
+  bool isTrivialType = ASI->getElementType().isTrivial(ASI->getModule());
   UserList UsersToRemove;
-  if (hasUnremovableUsers(ASI, UsersToRemove, /*acceptRefCountInsts=*/ true)) {
+  if (hasUnremovableUsers(ASI, UsersToRemove, /*acceptRefCountInsts=*/ true,
+      /*onlyAcceptTrivialStores*/!isTrivialType)) {
     LLVM_DEBUG(llvm::dbgs() << "    Found a use that cannot be zapped...\n");
     return false;
   }

test/SILOptimizer/dead_alloc_elim.sil

@@ -325,8 +325,9 @@ sil @non_trivial_destructor_objc_destructor : $@convention(thin) () -> () {
 }
 
 // CHECK-LABEL: sil @non_trivial_destructor_on_stack : $@convention(thin) () -> () {
-// CHECK: alloc_stack
-// CHECK: return
+// CHECK:      bb0:
+// CHECK-NEXT:   %0 = tuple ()
+// CHECK-NEXT:   return %0
 sil @non_trivial_destructor_on_stack : $@convention(thin) () -> () {
   %0 = alloc_stack $NonTrivialDestructor
   dealloc_stack %0 : $*NonTrivialDestructor
@@ -358,3 +359,18 @@ sil @trivial_destructor_may_trap : $@convention(thin) () -> () {
   %4 = tuple()
   return %4 : $()
 }
+
+// CHECK-LABEL: sil @remove_dead_enum_stackloc
+// CHECK:      bb0:
+// CHECK-NEXT:   %0 = tuple ()
+// CHECK-NEXT:   return %0
+sil @remove_dead_enum_stackloc : $@convention(thin) () -> () {
+bb0:
+  %0 = alloc_stack $FloatingPointRoundingRule
+  inject_enum_addr %0 : $*FloatingPointRoundingRule, #FloatingPointRoundingRule.toNearestOrEven!enumelt
+  destroy_addr %0 : $*FloatingPointRoundingRule
+  dealloc_stack %0 : $*FloatingPointRoundingRule
+  %1 = tuple ()
+  return %1 : $()
+}
+

test/SILOptimizer/fp_rounding.swift

@@ -0,0 +1,30 @@
+// RUN: %target-swift-frontend -parse-as-library -O -emit-sil  %s | %FileCheck %s
+// REQUIRES: swift_stdlib_no_asserts,optimized_stdlib,CPU=x86_64
+
+// This is an end-to-end test to ensure that the optimizer can propagate
+// resilient enum cases (FloatingPointRoundingRule) and produces optimal
+// code for Float.rounded().
+
+// CHECK-LABEL: sil @{{.*}}propagate_roundingmode
+// CHECK:      bb0:
+// CHECK-NEXT:   %0 = integer_literal {{.*}}, 0
+// CHECK-NEXT:   %1 = struct $Int (%0 {{.*}})
+// CHECK-NEXT:   return %1
+public func propagate_roundingmode() -> Int {
+  let rm = FloatingPointRoundingRule.toNearestOrEven
+  switch rm {
+    case .toNearestOrAwayFromZero:
+      return 1
+    default:
+      return 0
+  }
+}
+
+// CHECK-LABEL: sil @{{.*}}round_floating_point
+// CHECK: bb0({{.*}}):
+// CHECK:   [[R:%[0-9]+]] = builtin "int_round{{.*}}"
+// CHECK:   [[F:%[0-9]+]] = struct $Float ([[R]]
+// CHECK:   return [[F]]
+public func round_floating_point(_ x: Float) -> Float {
+  return x.rounded()
+}

test/SILOptimizer/sil_combine.sil

@@ -2106,6 +2106,36 @@ bb3:
   return %2 : $()
 }
 
+// CHECK-LABEL: sil @resilient_enum_case_propagation
+// CHECK: bb0:
+// CHECK:   br bb2
+// CHECK: bb1:
+// CHECK:   return
+sil @resilient_enum_case_propagation : $@convention(thin) () -> Builtin.Int64 {
+bb0:
+  %0 = alloc_stack $FloatingPointRoundingRule
+  inject_enum_addr %0 : $*FloatingPointRoundingRule, #FloatingPointRoundingRule.toNearestOrEven!enumelt
+  %2 = alloc_stack $FloatingPointRoundingRule
+  copy_addr %0 to [initialization] %2 : $*FloatingPointRoundingRule
+  destroy_addr %0 : $*FloatingPointRoundingRule
+  switch_enum_addr %2 : $*FloatingPointRoundingRule, case #FloatingPointRoundingRule.toNearestOrAwayFromZero!enumelt: bb1, default bb2
+
+bb1:
+  %6 = integer_literal $Builtin.Int64, 1
+  br bb3(%6 : $Builtin.Int64)
+
+bb2:
+  destroy_addr %2 : $*FloatingPointRoundingRule
+  %9 = integer_literal $Builtin.Int64, 0
+  br bb3(%9 : $Builtin.Int64)
+
+bb3(%11 : $Builtin.Int64):
+  dealloc_stack %2 : $*FloatingPointRoundingRule
+  dealloc_stack %0 : $*FloatingPointRoundingRule
+  return %11 : $Builtin.Int64
+}
+
+
 @objc(XX) protocol XX {
 }
 

validation-test/Evolution/test_backward_deploy_enum.swift

@@ -5,7 +5,7 @@ import StdlibUnittest
 import backward_deploy_enum
 
 // <rdar://problem/46438568>
-// XFAIL: *
+// REQUIRES: rdar46438568
 
 var BackwardDeployEnumTest = TestSuite("BackwardDeployEnum")