From ffc71e95e8c3e3651b025b716e66149f46831974 Mon Sep 17 00:00:00 2001 From: Erik Eckstein Date: Wed, 15 Oct 2025 11:24:12 +0200 Subject: [PATCH 1/2] SimplifyCFG: insert compensating destroy when replacing a `switch_enum` When replacing a `switch_enum` of an owned enum value with a branch to a non-payload case destination, we need to insert a destroy of the enum value. Fixes a SIL ownership verification failure. https://github.com/swiftlang/swift/issues/84552 rdar://161482601 --- lib/SILOptimizer/Transforms/SimplifyCFG.cpp | 6 +++++- test/SILOptimizer/simplify_cfg_ossa.sil | 18 ++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/lib/SILOptimizer/Transforms/SimplifyCFG.cpp b/lib/SILOptimizer/Transforms/SimplifyCFG.cpp index 95a835cb07a14..6e62610cfdd16 100644 --- a/lib/SILOptimizer/Transforms/SimplifyCFG.cpp +++ b/lib/SILOptimizer/Transforms/SimplifyCFG.cpp @@ -1791,7 +1791,11 @@ bool SimplifyCFG::simplifySwitchEnumUnreachableBlocks(SwitchEnumInst *SEI) { } if (Dest->args_empty()) { - SILBuilderWithScope(SEI).createBranch(SEI->getLoc(), Dest); + SILBuilderWithScope builder(SEI); + if (SEI->getOperand()->getOwnershipKind() == OwnershipKind::Owned) { + builder.createDestroyValue(SEI->getLoc(), SEI->getOperand()); + } + builder.createBranch(SEI->getLoc(), Dest); addToWorklist(SEI->getParent()); addToWorklist(Dest); diff --git a/test/SILOptimizer/simplify_cfg_ossa.sil b/test/SILOptimizer/simplify_cfg_ossa.sil index 60be68a61b632..917b5c9e0cce3 100644 --- a/test/SILOptimizer/simplify_cfg_ossa.sil +++ b/test/SILOptimizer/simplify_cfg_ossa.sil @@ -1944,6 +1944,24 @@ bb3: return %t : $() } +// CHECK-LABEL: sil [ossa] @insert_compensating_destroy_in_switch_enum_destination_block : +// CHECK: bb0(%0 : @owned $Optional): +// CHECK-NEXT: destroy_value %0 +// CHECK-NEXT: tuple +// CHECK: } // end sil function 'insert_compensating_destroy_in_switch_enum_destination_block' +sil [ossa] @insert_compensating_destroy_in_switch_enum_destination_block : $@convention(thin) (@owned Optional) -> () { +bb0(%0 : @owned $Optional): + switch_enum %0, case #Optional.none!enumelt: bb1, case #Optional.some!enumelt: bb2 + +bb1: + %15 = tuple () + return %15 + +bb2(%4 : @owned $AnyObject): + destroy_value %4 + unreachable +} + // CHECK-LABEL: sil [ossa] @replace_phi_arg_with_borrowed_from_use : // CHECK: bb3([[R:%.*]] : @reborrow $B): // CHECK: bb6([[G:%.*]] : @guaranteed $E): From c4c1c50e735d4b3f3d9d4f3e829ae82bdb2c324c Mon Sep 17 00:00:00 2001 From: Erik Eckstein Date: Mon, 20 Oct 2025 15:56:20 +0200 Subject: [PATCH 2/2] SimplifyCFG: insert compensating `end_lifetime` when replacing a `switch_enum` This is a follow-up of https://github.com/swiftlang/swift/pull/84905, which handles non-copyable enums with a deinit correctly. Also, for copyable enums it's more efficient to use `end_lifetime` than `destroy_value`, because we already know that the enum contains a trivial case. Therefore no destroy operation is needed. --- lib/SILOptimizer/Transforms/SimplifyCFG.cpp | 3 +- test/SILOptimizer/simplify_cfg_ossa.sil | 38 ++++++++++++++++++--- 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/lib/SILOptimizer/Transforms/SimplifyCFG.cpp b/lib/SILOptimizer/Transforms/SimplifyCFG.cpp index 6e62610cfdd16..f4781b4ca5c0c 100644 --- a/lib/SILOptimizer/Transforms/SimplifyCFG.cpp +++ b/lib/SILOptimizer/Transforms/SimplifyCFG.cpp @@ -1793,7 +1793,8 @@ bool SimplifyCFG::simplifySwitchEnumUnreachableBlocks(SwitchEnumInst *SEI) { if (Dest->args_empty()) { SILBuilderWithScope builder(SEI); if (SEI->getOperand()->getOwnershipKind() == OwnershipKind::Owned) { - builder.createDestroyValue(SEI->getLoc(), SEI->getOperand()); + // Note that a `destroy_value` would be wrong for non-copyable enums with deinits. + builder.createEndLifetime(SEI->getLoc(), SEI->getOperand()); } builder.createBranch(SEI->getLoc(), Dest); diff --git a/test/SILOptimizer/simplify_cfg_ossa.sil b/test/SILOptimizer/simplify_cfg_ossa.sil index 917b5c9e0cce3..9bb6480af1d83 100644 --- a/test/SILOptimizer/simplify_cfg_ossa.sil +++ b/test/SILOptimizer/simplify_cfg_ossa.sil @@ -1,4 +1,6 @@ -// RUN: %target-sil-opt -sil-print-types -enable-objc-interop -enable-sil-verify-all %s -simplify-cfg | %FileCheck %s +// RUN: %target-sil-opt -sil-print-types -enable-objc-interop -enable-experimental-feature MoveOnlyEnumDeinits -enable-sil-verify-all %s -simplify-cfg | %FileCheck %s + +// REQUIRES: swift_feature_MoveOnlyEnumDeinits // OSSA form of tests from simplify_cfg.sil and simplify_cfg_simple.sil. // @@ -64,6 +66,12 @@ struct S { var b: NonTrivial } +enum NCE: ~Copyable { + case a + case b(AnyObject) + deinit +} + /////////// // Tests // /////////// @@ -1944,12 +1952,12 @@ bb3: return %t : $() } -// CHECK-LABEL: sil [ossa] @insert_compensating_destroy_in_switch_enum_destination_block : +// CHECK-LABEL: sil [ossa] @insert_compensating_endlifetime_in_switch_enum_destination_block : // CHECK: bb0(%0 : @owned $Optional): -// CHECK-NEXT: destroy_value %0 +// CHECK-NEXT: end_lifetime %0 // CHECK-NEXT: tuple -// CHECK: } // end sil function 'insert_compensating_destroy_in_switch_enum_destination_block' -sil [ossa] @insert_compensating_destroy_in_switch_enum_destination_block : $@convention(thin) (@owned Optional) -> () { +// CHECK: } // end sil function 'insert_compensating_endlifetime_in_switch_enum_destination_block' +sil [ossa] @insert_compensating_endlifetime_in_switch_enum_destination_block : $@convention(thin) (@owned Optional) -> () { bb0(%0 : @owned $Optional): switch_enum %0, case #Optional.none!enumelt: bb1, case #Optional.some!enumelt: bb2 @@ -1962,6 +1970,26 @@ bb2(%4 : @owned $AnyObject): unreachable } +// CHECK-LABEL: sil [ossa] @insert_compensating_endlifetime_in_switch_enum_destination_block2 : +// CHECK: bb0(%0 : @owned $NCE): +// CHECK-NEXT: %1 = drop_deinit %0 +// CHECK-NEXT: end_lifetime %1 +// CHECK-NEXT: tuple +// CHECK: } // end sil function 'insert_compensating_endlifetime_in_switch_enum_destination_block2' +sil [ossa] @insert_compensating_endlifetime_in_switch_enum_destination_block2 : $@convention(thin) (@owned NCE) -> () { +bb0(%0 : @owned $NCE): + %1 = drop_deinit %0 + switch_enum %1, case #NCE.a!enumelt: bb1, case #NCE.b!enumelt: bb2 + +bb1: + %15 = tuple () + return %15 + +bb2(%4 : @owned $AnyObject): + destroy_value %4 + unreachable +} + // CHECK-LABEL: sil [ossa] @replace_phi_arg_with_borrowed_from_use : // CHECK: bb3([[R:%.*]] : @reborrow $B): // CHECK: bb6([[G:%.*]] : @guaranteed $E):