Skip to content

Commit 36e95ca

Browse files
gottesmmgottesmm
authored
Merge pull request #34280 from gottesmm/pr-94bbbe81fdf8a4d1212dd582f8fad1c619207575
When converting load [copy] -> load_borrow, do not insert end_borrow if the block insert point is a dead end block.
2 parents 5f5d9e4 + 0e05b51 commit 36e95ca

File tree

2 files changed

+136
-0
lines changed

2 files changed

+136
-0
lines changed

‎lib/SILOptimizer/SemanticARC/OwnershipLiveRange.cpp

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
#include "OwnershipLiveRange.h"
1414
#include "OwnershipPhiOperand.h"
1515

16+
#include "swift/SIL/BasicBlockUtils.h"
17+
1618
using namespace swift;
1719
using namespace swift::semanticarc;
1820

@@ -162,6 +164,61 @@ void OwnershipLiveRange::insertEndBorrowsAtDestroys(
162164
auto loc = RegularLocation::getAutoGeneratedLocation();
163165
while (!scratch.empty()) {
164166
auto *insertPoint = scratch.pop_back_val();
167+
168+
// Do not insert end_borrow if the block insert point is a dead end block.
169+
//
170+
// DISCUSSION: This is important to do since otherwise, we may be implicitly
171+
// reducing the lifetime of a value which we can not do yet since we do not
172+
// require all interior pointer instructions to be guarded by borrows
173+
// (yet). Once that constraint is in place, we will not have this problem.
174+
//
175+
// Consider a situation where one has a @owned switch_enum on an
176+
// indirect box case which is post-dominated by an unreachable that we want
177+
// to convert to @guaranteed:
178+
//
179+
// enum MyEnum {
180+
// indirect case FirstCase(Int)
181+
// ...
182+
// }
183+
//
184+
// bb0(%in_guaranteed_addr : $*MyEnum):
185+
// ...
186+
// %0 = load [copy] %in_guaranteed_addr : $*MyEnum
187+
// switch_enum %0 : $MyEnum, case #MyEnum.FirstCase: bb1, ...
188+
//
189+
// bb1(%1 : @owned ${ var Int }):
190+
// %2 = project_box %1 : ${ var Int }, 0
191+
// %3 = load [trivial] %2 : $*Int
192+
// apply %log(%3) : $@convention(thin) (Int) -> ()
193+
// unreachable
194+
//
195+
// In this case, we will not have a destroy_value on the box, but we may
196+
// have a project_box on the box. This is ok since we are going to leak the
197+
// value. But since we are using all consuming uses to determine the
198+
// lifetime, we will want to insert an end_borrow at the head of the
199+
// switch_enum dest block like follows:
200+
//
201+
// bb0(%in_guaranteed_addr : $*MyEnum):
202+
// ...
203+
// %0 = load_borrow %in_guaranteed_addr : $*MyEnum
204+
// switch_enum %0 : $MyEnum, case #MyEnum.FirstCase: bb1, ...
205+
//
206+
// bb1(%1 : @guaranteed ${ var Int }):
207+
// end_borrow %1 : ${ var Int }
208+
// %2 = project_box %1 : ${ var Int }, 0
209+
// %3 = load [trivial] %2 : $*Int
210+
// apply %log(%3) : $@convention(thin) (Int) -> ()
211+
// unreachable
212+
//
213+
// which would violate ownership invariants. Instead, we need to realize
214+
// that %1 is dominated by a dead end block so we may not have a
215+
// destroy_value upon it meaning we should just not insert the end_borrow
216+
// here. If we have a destroy_value upon it (since we did not get rid of a
217+
// destroy_value), then we will still get rid of the destroy_value if we are
218+
// going to optimize this, so we are still correct.
219+
if (deadEndBlocks.isDeadEnd(insertPoint->getParent()))
220+
continue;
221+
165222
SILBuilderWithScope builder(insertPoint);
166223
builder.createEndBorrow(loc, newGuaranteedValue);
167224
}

‎test/SILOptimizer/semantic-arc-opts.sil

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,16 @@ class SubclassLet: ClassLet {}
7979
sil_global [let] @a_let_global : $Klass
8080
sil_global @a_var_global : $Klass
8181

82+
enum EnumWithIndirectCase {
83+
case first
84+
indirect case second(Builtin.NativeObject)
85+
}
86+
87+
struct StructWithEnumWithIndirectCaseField {
88+
var i: Builtin.Int23
89+
var field : EnumWithIndirectCase
90+
}
91+
8292
///////////
8393
// Tests //
8494
///////////
@@ -2727,3 +2737,72 @@ bbEnd:
27272737
return %9999 : $()
27282738
}
27292739

2740+
// CHECK-LABEL: sil [ossa] @enum_with_indirect_case_projectbox_copyvalue_deadend : $@convention(thin) (@guaranteed StructWithEnumWithIndirectCaseField) -> () {
2741+
// CHECK-NOT: copy_value
2742+
// CHECK: } // end sil function 'enum_with_indirect_case_projectbox_copyvalue_deadend'
2743+
sil [ossa] @enum_with_indirect_case_projectbox_copyvalue_deadend : $@convention(thin) (@guaranteed StructWithEnumWithIndirectCaseField) -> () {
2744+
bb0(%0 : @guaranteed $StructWithEnumWithIndirectCaseField):
2745+
%1 = struct_extract %0 : $StructWithEnumWithIndirectCaseField, #StructWithEnumWithIndirectCaseField.field
2746+
%1a = copy_value %1 : $EnumWithIndirectCase
2747+
switch_enum %1a : $EnumWithIndirectCase, case #EnumWithIndirectCase.first!enumelt: bb1, case #EnumWithIndirectCase.second!enumelt: bb2
2748+
2749+
bb1:
2750+
%9999 = tuple()
2751+
return %9999 : $()
2752+
2753+
// NOTE: Eventually this will need to be changed when project_box has to be
2754+
// guarded by begin_borrow.
2755+
bb2(%2 : @owned ${ var Builtin.NativeObject }):
2756+
%3 = project_box %2 : ${ var Builtin.NativeObject }, 0
2757+
%4 = load [copy] %3 : $*Builtin.NativeObject
2758+
%user = function_ref @guaranteed_user : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
2759+
apply %user(%4) : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
2760+
unreachable
2761+
}
2762+
2763+
// CHECK-LABEL: sil [ossa] @enum_with_indirect_case_projectbox_loadcopy_to_loadborrow_deadend : $@convention(thin) (@in_guaranteed EnumWithIndirectCase) -> () {
2764+
// CHECK: bb0
2765+
// CHECK-NEXT: load_borrow
2766+
// CHECK: } // end sil function 'enum_with_indirect_case_projectbox_loadcopy_to_loadborrow_deadend'
2767+
sil [ossa] @enum_with_indirect_case_projectbox_loadcopy_to_loadborrow_deadend : $@convention(thin) (@in_guaranteed EnumWithIndirectCase) -> () {
2768+
bb0(%0 : $*EnumWithIndirectCase):
2769+
%1 = load [copy] %0 : $*EnumWithIndirectCase
2770+
switch_enum %1 : $EnumWithIndirectCase, case #EnumWithIndirectCase.first!enumelt: bb1, case #EnumWithIndirectCase.second!enumelt: bb2
2771+
2772+
bb1:
2773+
%9999 = tuple()
2774+
return %9999 : $()
2775+
2776+
// NOTE: Eventually this will need to be changed when project_box has to be
2777+
// guarded by begin_borrow.
2778+
bb2(%2 : @owned ${ var Builtin.NativeObject }):
2779+
%3 = project_box %2 : ${ var Builtin.NativeObject }, 0
2780+
%4 = load [copy] %3 : $*Builtin.NativeObject
2781+
%user = function_ref @guaranteed_user : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
2782+
apply %user(%4) : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
2783+
unreachable
2784+
}
2785+
2786+
// CHECK-LABEL: sil [ossa] @enum_with_indirect_case_projectbox_loadcopy_to_loadborrow_deadend_2 : $@convention(thin) (@in_guaranteed EnumWithIndirectCase) -> () {
2787+
// CHECK: bb0
2788+
// CHECK-NEXT: load_borrow
2789+
// CHECK: } // end sil function 'enum_with_indirect_case_projectbox_loadcopy_to_loadborrow_deadend_2'
2790+
sil [ossa] @enum_with_indirect_case_projectbox_loadcopy_to_loadborrow_deadend_2 : $@convention(thin) (@in_guaranteed EnumWithIndirectCase) -> () {
2791+
bb0(%0 : $*EnumWithIndirectCase):
2792+
%1 = load [copy] %0 : $*EnumWithIndirectCase
2793+
switch_enum %1 : $EnumWithIndirectCase, case #EnumWithIndirectCase.first!enumelt: bb1, case #EnumWithIndirectCase.second!enumelt: bb2
2794+
2795+
bb1:
2796+
%9999 = tuple()
2797+
return %9999 : $()
2798+
2799+
// NOTE: Eventually this will need to be changed when project_box has to be
2800+
// guarded by begin_borrow.
2801+
bb2(%2 : @owned ${ var Builtin.NativeObject }):
2802+
%3 = project_box %2 : ${ var Builtin.NativeObject }, 0
2803+
%4 = load [copy] %3 : $*Builtin.NativeObject
2804+
%user = function_ref @guaranteed_user : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
2805+
apply %user(%4) : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
2806+
destroy_value %2 : ${ var Builtin.NativeObject }
2807+
unreachable
2808+
}

0 commit comments

Comments
 (0)