Skip to content

Commit 2de7bb9

Browse files
swift-ciswift-ci
committed
Merge remote-tracking branch 'origin/master' into master-rebranch
2 parents c36e5f9 + ec3975a commit 2de7bb9

File tree

2 files changed

+119
-1
lines changed

2 files changed

+119
-1
lines changed

‎lib/SILOptimizer/Mandatory/SemanticARCOpts.cpp

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -340,6 +340,62 @@ bool SemanticARCOptVisitor::performGuaranteedCopyValueOptimization(CopyValueInst
340340
if (isConsumed(cvi, destroys, &guaranteedForwardingInsts))
341341
return false;
342342

343+
// Next check if we have any destroys at all of our copy_value and an operand
344+
// that is not a function argument. Otherwise, due to the way we ignore dead
345+
// end blocks, we may eliminate the copy_value, creating a use of the borrowed
346+
// value after the end_borrow.
347+
//
348+
// DISCUSSION: Consider the following SIL:
349+
//
350+
// ```
351+
// %1 = begin_borrow %0 : $KlassPair (1)
352+
// %2 = struct_extract %1 : $KlassPair, #KlassPair.firstKlass
353+
// %3 = copy_value %2 : $Klass
354+
// ...
355+
// end_borrow %1 : $LintCommand (2)
356+
// cond_br ..., bb1, bb2
357+
//
358+
// ...
359+
//
360+
// bbN:
361+
// // Never return type implies dead end block.
362+
// apply %f(%3) : $@convention(thin) (@guaranteed Klass) -> Never (3)
363+
// unreachable
364+
// ```
365+
//
366+
// For simplicity, note that if bbN post-dominates %3, given that when we
367+
// compute linear lifetime errors we ignore dead end blocks, we would not
368+
// register that the copy_values only use is outside of the begin_borrow
369+
// region defined by (1), (2) and thus would eliminate the copy. This would
370+
// result in %2 being used by %f, causing the linear lifetime checker to
371+
// error.
372+
//
373+
// Naively one may assume that the solution to this is to just check if %3 has
374+
// /any/ destroy_values at all and if it doesn't have any reachable
375+
// destroy_values, then we are in this case. But is this correct in
376+
// general. We prove this below:
377+
//
378+
// The only paths along which the copy_value can not be destroyed or consumed
379+
// is along paths to dead end blocks. Trivially, we know that such a dead end
380+
// block, can not be reachable from the end_borrow since by their nature dead
381+
// end blocks end in unreachables.
382+
//
383+
// So we know that we can only run into this bug if we have a dead end block
384+
// reachable from the end_borrow, meaning that the bug can not occur if we
385+
// branch before the end_borrow since in that case, the borrow scope would
386+
// last over the dead end block's no return meaning that we will not use the
387+
// borrowed value after its lifetime is ended by the end_borrow.
388+
//
389+
// With that in hand, we note again that if we have exactly one consumed,
390+
// destroy_value /after/ the end_borrow we will not optimize here. This means
391+
// that this bug can only occur if the copy_value is only post-dominated by
392+
// dead end blocks that use the value in a non-consuming way.
393+
if (destroys.empty() && llvm::any_of(borrowIntroducers, [](SILValue v) {
394+
return !isa<SILFunctionArgument>(v);
395+
})) {
396+
return false;
397+
}
398+
343399
// If we reached this point, then we know that all of our users can
344400
// accept a guaranteed value and our owned value is destroyed only
345401
// by destroy_value. Check if all of our destroys are joint

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

Lines changed: 63 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,12 @@ import Builtin
88
// Declarations //
99
//////////////////
1010

11+
enum MyNever {}
12+
1113
sil @guaranteed_user : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
1214
sil @owned_user : $@convention(thin) (@owned Builtin.NativeObject) -> ()
1315
sil @get_owned_obj : $@convention(thin) () -> @owned Builtin.NativeObject
16+
sil @unreachable_guaranteed_user : $@convention(thin) (@guaranteed Builtin.NativeObject) -> MyNever
1417

1518
struct NativeObjectPair {
1619
var obj1 : Builtin.NativeObject
@@ -775,4 +778,63 @@ bb0(%0 : $*NativeObjectPair):
775778
destroy_addr %0 : $*NativeObjectPair
776779
%9999 = tuple()
777780
return %9999 : $()
778-
}
781+
}
782+
783+
// Make sure we do not eliminate the copy_value below to ensure that all uses of
784+
// %2 are before %2's end_borrow.
785+
//
786+
// We used to eliminate the copy_value and change %func to use %2.
787+
//
788+
// CHECK-LABEL: sil [ossa] @begin_borrow_used_by_postdominating_no_return_function : $@convention(thin) () -> MyNever {
789+
// CHECK: copy_value
790+
// CHECK: } // end sil function 'begin_borrow_used_by_postdominating_no_return_function'
791+
sil [ossa] @begin_borrow_used_by_postdominating_no_return_function : $@convention(thin) () -> MyNever {
792+
bb0:
793+
%0 = function_ref @get_nativeobject_pair : $@convention(thin) () -> @owned NativeObjectPair
794+
%1 = apply %0() : $@convention(thin) () -> @owned NativeObjectPair
795+
%2 = begin_borrow %1 : $NativeObjectPair
796+
%3 = struct_extract %2 : $NativeObjectPair, #NativeObjectPair.obj1
797+
%4 = copy_value %3 : $Builtin.NativeObject
798+
end_borrow %2 : $NativeObjectPair
799+
%func = function_ref @unreachable_guaranteed_user : $@convention(thin) (@guaranteed Builtin.NativeObject) -> MyNever
800+
apply %func(%4) : $@convention(thin) (@guaranteed Builtin.NativeObject) -> MyNever
801+
unreachable
802+
}
803+
804+
// Make sure we do not eliminate the copy_value below to ensure that all uses of
805+
// %2 are before %2's end_borrow.
806+
//
807+
// We used to eliminate the copy_value and change %func to use %2.
808+
//
809+
// CHECK-LABEL: sil [ossa] @load_borrow_used_by_postdominating_no_return_function : $@convention(thin) () -> MyNever {
810+
// CHECK: copy_value
811+
// CHECK: } // end sil function 'load_borrow_used_by_postdominating_no_return_function'
812+
sil [ossa] @load_borrow_used_by_postdominating_no_return_function : $@convention(thin) () -> MyNever {
813+
bb0:
814+
%0 = function_ref @get_nativeobject_pair : $@convention(thin) () -> @owned NativeObjectPair
815+
%1 = apply %0() : $@convention(thin) () -> @owned NativeObjectPair
816+
%stackSlot = alloc_stack $NativeObjectPair
817+
store %1 to [init] %stackSlot : $*NativeObjectPair
818+
%2 = load_borrow %stackSlot : $*NativeObjectPair
819+
%3 = struct_extract %2 : $NativeObjectPair, #NativeObjectPair.obj1
820+
%4 = copy_value %3 : $Builtin.NativeObject
821+
end_borrow %2 : $NativeObjectPair
822+
%func = function_ref @unreachable_guaranteed_user : $@convention(thin) (@guaranteed Builtin.NativeObject) -> MyNever
823+
apply %func(%4) : $@convention(thin) (@guaranteed Builtin.NativeObject) -> MyNever
824+
unreachable
825+
}
826+
827+
// Make sure we do perform the optimization if our borrowed value is an
828+
// argument.
829+
//
830+
// CHECK-LABEL: sil [ossa] @guaranteed_arg_used_by_postdominating_no_return_function : $@convention(thin) (@guaranteed NativeObjectPair) -> MyNever {
831+
// CHECK-NOT: copy_value
832+
// CHECK: } // end sil function 'guaranteed_arg_used_by_postdominating_no_return_function'
833+
sil [ossa] @guaranteed_arg_used_by_postdominating_no_return_function : $@convention(thin) (@guaranteed NativeObjectPair) -> MyNever {
834+
bb0(%0 : @guaranteed $NativeObjectPair):
835+
%3 = struct_extract %0 : $NativeObjectPair, #NativeObjectPair.obj1
836+
%4 = copy_value %3 : $Builtin.NativeObject
837+
%func = function_ref @unreachable_guaranteed_user : $@convention(thin) (@guaranteed Builtin.NativeObject) -> MyNever
838+
apply %func(%4) : $@convention(thin) (@guaranteed Builtin.NativeObject) -> MyNever
839+
unreachable
840+
}

0 commit comments

Comments
 (0)