Skip to content

5.9: [TempRValueOpt] Fold new destroy_addrs into loads. #65070

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Apr 11, 2023
Merged

5.9: [TempRValueOpt] Fold new destroy_addrs into loads. #65070

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
61335d7
[TempRValueOpt] NFC: Renamed variable.
nate-chandler Apr 10, 2023
e3163bb
[Test] Added tests showing extra copies.
nate-chandler Apr 10, 2023
a5f4a16
[TempRValueOpt] Fold new destroy_addrs into loads.
nate-chandler Apr 10, 2023
315c3c9
[TempRValueOpt] NFC: Assert no copy of uncopyable.
nate-chandler Apr 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 40 additions & 8 deletions lib/SILOptimizer/Transforms/TempRValueElimination.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -535,10 +535,11 @@ void TempRValueOptPass::tryOptimizeCopyIntoTemp(CopyAddrInst *copyInst) {
SILValue copySrc = copyInst->getSrc();
assert(tempObj != copySrc && "can't initialize temporary with itself");

// If the source of the copyInst is taken, we must insert a compensating
// destroy_addr. This must be done at the right spot: after the last use
// tempObj, but before any (potential) re-initialization of the source.
bool needToInsertDestroy = copyInst->isTakeOfSrc();
// If the source of the copyInst is taken, it must be deinitialized (via
// destroy_addr, load [take], copy_addr [take]). This must be done at the
// right spot: after the last use tempObj, but before any (potential)
// re-initialization of the source.
bool needFinalDeinit = copyInst->isTakeOfSrc();

// Scan all uses of the temporary storage (tempObj) to verify they all refer
// to the value initialized by this copy. It is sufficient to check that the
Expand All @@ -557,7 +558,7 @@ void TempRValueOptPass::tryOptimizeCopyIntoTemp(CopyAddrInst *copyInst) {

// Also, destroys are allowed to be in a different block.
if (isa<DestroyAddrInst>(user)) {
if (!isOSSA && needToInsertDestroy) {
if (!isOSSA && needFinalDeinit) {
// In non-OSSA mode, for the purpose of inserting the destroy of
// copySrc, we have to be conservative and assume that the lifetime of
// tempObj goes beyond it's last use - until the final destroy_addr.
Expand Down Expand Up @@ -588,7 +589,7 @@ void TempRValueOptPass::tryOptimizeCopyIntoTemp(CopyAddrInst *copyInst) {
// Example:
// copy_addr [take] %copySrc to [init] %tempObj // copyInst
// copy_addr [take] %tempObj to [init] %copySrc // lastLoadInst
if (needToInsertDestroy && lastLoadInst != copyInst &&
if (needFinalDeinit && lastLoadInst != copyInst &&
!isa<DestroyAddrInst>(lastLoadInst) &&
aa->mayWriteToMemory(lastLoadInst, copySrc))
return;
Expand All @@ -601,6 +602,34 @@ void TempRValueOptPass::tryOptimizeCopyIntoTemp(CopyAddrInst *copyInst) {

LLVM_DEBUG(llvm::dbgs() << " Success: replace temp" << *tempObj);

// If copyInst's source must be deinitialized, whether that must be done via
// a newly created destroy_addr.
//
// If lastLoadInst is a load or a copy_addr, then the deinitialization can be
// done in that instruction.
//
// This is necessary for correctness: otherwise, copies of move-only values
// would be introduced.
bool needToInsertDestroy = [&]() {
if (!needFinalDeinit)
return false;
if (lastLoadInst == copyInst)
return true;
if (auto *cai = dyn_cast<CopyAddrInst>(lastLoadInst)) {
auto retval = cai->getSrc() != tempObj || !cai->isTakeOfSrc();
assert(!tempObj->getType().isMoveOnly() ||
!retval && "introducing copy of move-only value!?");
return retval;
}
if (auto *li = dyn_cast<LoadInst>(lastLoadInst)) {
auto retval = li->getOperand() != tempObj ||
li->getOwnershipQualifier() != LoadOwnershipQualifier::Take;
assert(!tempObj->getType().isMoveOnly() ||
!retval && "introducing copy of move-only value!?");
return retval;
}
return true;
}();
if (needToInsertDestroy) {
// Compensate the [take] of the original copyInst.
SILBuilderWithScope::insertAfter(lastLoadInst, [&] (SILBuilder &builder) {
Expand Down Expand Up @@ -630,16 +659,19 @@ void TempRValueOptPass::tryOptimizeCopyIntoTemp(CopyAddrInst *copyInst) {
auto *cai = cast<CopyAddrInst>(user);
if (cai != copyInst) {
assert(cai->getSrc() == tempObj);
if (cai->isTakeOfSrc())
if (cai->isTakeOfSrc() && (!needFinalDeinit || lastLoadInst != cai)) {
cai->setIsTakeOfSrc(IsNotTake);
}
}
use->set(copySrc);
break;
}
case SILInstructionKind::LoadInst: {
auto *li = cast<LoadInst>(user);
if (li->getOwnershipQualifier() == LoadOwnershipQualifier::Take)
if (li->getOwnershipQualifier() == LoadOwnershipQualifier::Take &&
(!needFinalDeinit || li != lastLoadInst)) {
li->setOwnershipQualifier(LoadOwnershipQualifier::Copy);
}
use->set(copySrc);
break;
}
Expand Down
241 changes: 239 additions & 2 deletions test/SILOptimizer/temp_rvalue_opt_ossa.sil
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,18 @@ public enum FakeOptional<T> {
case some(T)
}

@_moveOnly struct MOS {}

sil [ossa] @getKlass : $@convention(thin) () -> @owned Klass
sil [ossa] @getNonTrivialStruct : $@convention(thin) () -> @owned NonTrivialStruct
sil [ossa] @getMOS : $@convention(thin) () -> @owned MOS

sil @unknown : $@convention(thin) () -> ()

sil [ossa] @guaranteed_user : $@convention(thin) (@guaranteed Klass) -> ()
sil [ossa] @guaranteed_user_with_result : $@convention(thin) (@guaranteed Klass) -> @out Klass
sil [ossa] @inguaranteed_user_without_result_NTS : $@convention(thin) (@in_guaranteed NonTrivialStruct) -> ()
sil [ossa] @inguaranteed_user_without_result_MOS : $@convention(thin) (@in_guaranteed MOS) -> ()

sil [ossa] @inguaranteed_user_without_result : $@convention(thin) (@in_guaranteed Klass) -> () {
bb0(%0 : $*Klass):
Expand Down Expand Up @@ -764,8 +772,7 @@ bb0(%0 : $*Builtin.NativeObject):
//
// CHECK-LABEL: sil [ossa] @takeWithLoadRelease : $@convention(thin) (@in Builtin.NativeObject) -> () {
// CHECK: bb0(%0 : $*Builtin.NativeObject):
// CHECK: [[V:%.*]] = load [copy] %0 : $*Builtin.NativeObject
// CHECK: destroy_addr %0 : $*Builtin.NativeObject
// CHECK: [[V:%.*]] = load [take] %0 : $*Builtin.NativeObject
// CHECK: apply %{{.*}}([[V]]) : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
// CHECK: destroy_value [[V]] : $Builtin.NativeObject
// CHECK-LABEL: } // end sil function 'takeWithLoadRelease'
Expand Down Expand Up @@ -1506,6 +1513,208 @@ bb2:
unwind
}

// CHECK-LABEL: sil [ossa] @take_from_original_copy_addr__final_use_load_take : {{.*}} {
// CHECK: [[GET:%[^,]+]] = function_ref @getKlass
// CHECK: [[USER:%[^,]+]] = function_ref @inguaranteed_user_without_result
// CHECK: [[INSTANCE_1:%[^,]+]] = apply [[GET]]()
// CHECK: [[ADDR:%[^,]+]] = alloc_stack $Klass
// CHECK: store [[INSTANCE_1]] to [init] [[ADDR]]
// CHECK: apply [[USER]]([[ADDR]])
// CHECK: [[INSTANCE_2:%[^,]+]] = load [take] [[ADDR]]
// CHECK: dealloc_stack [[ADDR]]
// CHECK: return [[INSTANCE_2]]
// CHECK-LABEL: } // end sil function 'take_from_original_copy_addr__final_use_load_take'
sil [ossa] @take_from_original_copy_addr__final_use_load_take : $() -> @owned Klass {
%getKlass = function_ref @getKlass : $@convention(thin) () -> @owned Klass
%user = function_ref @inguaranteed_user_without_result : $@convention(thin) (@in_guaranteed Klass) -> ()
%instance_1 = apply %getKlass() : $@convention(thin) () -> @owned Klass
%src = alloc_stack $Klass
store %instance_1 to [init] %src : $*Klass
apply %user(%src) : $@convention(thin) (@in_guaranteed Klass) -> ()
%tmp = alloc_stack $Klass
copy_addr [take] %src to [init] %tmp : $*Klass
%instance_2 = load [take] %tmp : $*Klass
dealloc_stack %tmp : $*Klass
dealloc_stack %src : $*Klass
return %instance_2 : $Klass
}

// CHECK-LABEL: sil [ossa] @take_from_original_copy_addr__final_use_copy_addr_take : {{.*}} {
// CHECK: {{bb[0-9]+}}([[OUT:%[^,]+]] :
// CHECK: [[GET:%[^,]+]] = function_ref @getKlass
// CHECK: [[USER:%[^,]+]] = function_ref @inguaranteed_user_without_result
// CHECK: [[INSTANCE_1:%[^,]+]] = apply [[GET]]()
// CHECK: [[SRC:%[^,]+]] = alloc_stack $Klass
// CHECK: store [[INSTANCE_1]] to [init] [[SRC]]
// CHECK: apply [[USER]]([[SRC]])
// CHECK: copy_addr [take] [[SRC]] to [init] [[OUT]]
// CHECK: dealloc_stack [[SRC]]
// CHECK-LABEL: } // end sil function 'take_from_original_copy_addr__final_use_copy_addr_take'
sil [ossa] @take_from_original_copy_addr__final_use_copy_addr_take : $() -> @out Klass {
entry(%out : $*Klass):
%getKlass = function_ref @getKlass : $@convention(thin) () -> @owned Klass
%user = function_ref @inguaranteed_user_without_result : $@convention(thin) (@in_guaranteed Klass) -> ()
%instance_1 = apply %getKlass() : $@convention(thin) () -> @owned Klass
%src = alloc_stack $Klass
store %instance_1 to [init] %src : $*Klass
apply %user(%src) : $@convention(thin) (@in_guaranteed Klass) -> ()
%tmp = alloc_stack $Klass
copy_addr [take] %src to [init] %tmp : $*Klass
copy_addr [take] %tmp to [init] %out : $*Klass
dealloc_stack %tmp : $*Klass
dealloc_stack %src : $*Klass
%retval = tuple ()
return %retval : $()
}

// CHECK-LABEL: sil [ossa] @take_from_original_copy_addr__final_use_field_load_copy : {{.*}} {
// CHECK: [[GET:%[^,]+]] = function_ref @getNonTrivialStruct
// CHECK: [[USER:%[^,]+]] = function_ref @inguaranteed_user_without_result_NTS
// CHECK: [[INSTANCE:%[^,]+]] = apply [[GET]]()
// CHECK: [[SRC:%[^,]+]] = alloc_stack $NonTrivialStruct
// CHECK: store [[INSTANCE]] to [init] [[SRC]]
// CHECK: apply [[USER]]([[SRC]])
// CHECK: [[FIELD_ADDR:%[^,]+]] = struct_element_addr [[SRC]]
// CHECK: [[FIELD:%[^,]+]] = load [copy] [[FIELD_ADDR]]
// CHECK: destroy_addr [[SRC]]
// CHECK: dealloc_stack [[SRC]]
// CHECK: return [[FIELD]]
// CHECK-LABEL: } // end sil function 'take_from_original_copy_addr__final_use_field_load_copy'
sil [ossa] @take_from_original_copy_addr__final_use_field_load_copy : $() -> @owned Klass {
%get = function_ref @getNonTrivialStruct : $@convention(thin) () -> @owned NonTrivialStruct
%user = function_ref @inguaranteed_user_without_result_NTS : $@convention(thin) (@in_guaranteed NonTrivialStruct) -> ()
%instance_1 = apply %get() : $@convention(thin) () -> @owned NonTrivialStruct
%src = alloc_stack $NonTrivialStruct
store %instance_1 to [init] %src : $*NonTrivialStruct
apply %user(%src) : $@convention(thin) (@in_guaranteed NonTrivialStruct) -> ()
%tmp = alloc_stack $NonTrivialStruct
copy_addr [take] %src to [init] %tmp : $*NonTrivialStruct
%field_addr = struct_element_addr %tmp : $*NonTrivialStruct, #NonTrivialStruct.val
%field = load [copy] %field_addr : $*Klass
destroy_addr %tmp : $*NonTrivialStruct
dealloc_stack %tmp : $*NonTrivialStruct
dealloc_stack %src : $*NonTrivialStruct
return %field : $Klass
}

// CHECK-LABEL: sil [ossa] @take_from_original_copy_addr__final_use_field_copy_addr_take : {{.*}} {
// CHECK: {{bb[0-9]+}}([[OUT:%[^,]+]] :
// CHECK: [[GET:%[^,]+]] = function_ref @getNonTrivialStruct
// CHECK: [[USER:%[^,]+]] = function_ref @inguaranteed_user_without_result_NTS
// CHECK: [[INSTANCE:%[^,]+]] = apply [[GET]]()
// CHECK: [[SRC:%[^,]+]] = alloc_stack $NonTrivialStruct
// CHECK: store [[INSTANCE]] to [init] [[SRC]]
// CHECK: apply [[USER]]([[SRC]])
// CHECK: [[FIELD_ADDR:%[^,]+]] = struct_element_addr [[SRC]]
// CHECK: copy_addr [[FIELD_ADDR]] to [init] [[OUT]]
// CHECK: destroy_addr [[SRC]]
// CHECK: dealloc_stack [[SRC]]
// CHECK-LABEL: } // end sil function 'take_from_original_copy_addr__final_use_field_copy_addr_take'
sil [ossa] @take_from_original_copy_addr__final_use_field_copy_addr_take : $() -> @out Klass {
entry(%out : $*Klass):
%getNonTrivialStruct = function_ref @getNonTrivialStruct : $@convention(thin) () -> @owned NonTrivialStruct
%user = function_ref @inguaranteed_user_without_result_NTS : $@convention(thin) (@in_guaranteed NonTrivialStruct) -> ()
%instance_1 = apply %getNonTrivialStruct() : $@convention(thin) () -> @owned NonTrivialStruct
%src = alloc_stack $NonTrivialStruct
store %instance_1 to [init] %src : $*NonTrivialStruct
apply %user(%src) : $@convention(thin) (@in_guaranteed NonTrivialStruct) -> ()
%tmp = alloc_stack $NonTrivialStruct
copy_addr [take] %src to [init] %tmp : $*NonTrivialStruct
%field_addr = struct_element_addr %tmp : $*NonTrivialStruct, #NonTrivialStruct.val
copy_addr %field_addr to [init] %out : $*Klass
destroy_addr %tmp : $*NonTrivialStruct
dealloc_stack %tmp : $*NonTrivialStruct
dealloc_stack %src : $*NonTrivialStruct
%retval = tuple ()
return %retval : $()
}

// CHECK-LABEL: sil [ossa] @take_from_original_copy_addr__final_use_load_copy : {{.*}} {
// CHECK: [[GET:%[^,]+]] = function_ref @getKlass
// CHECK: [[USER:%[^,]+]] = function_ref @inguaranteed_user_without_result
// CHECK: [[INSTANCE_1:%[^,]+]] = apply [[GET]]()
// CHECK: [[SRC:%[^,]+]] = alloc_stack $Klass
// CHECK: store [[INSTANCE_1]] to [init] [[SRC]]
// CHECK: apply [[USER]]([[SRC]])
// CHECK: [[INSTANCE_2:%[^,]+]] = load [copy] [[SRC]]
// CHECK: destroy_addr [[SRC]]
// CHECK: dealloc_stack [[SRC]]
// CHECK: return [[INSTANCE_2]]
// CHECK-LABEL: } // end sil function 'take_from_original_copy_addr__final_use_load_copy'
sil [ossa] @take_from_original_copy_addr__final_use_load_copy : $() -> @owned Klass {
%getKlass = function_ref @getKlass : $@convention(thin) () -> @owned Klass
%user = function_ref @inguaranteed_user_without_result : $@convention(thin) (@in_guaranteed Klass) -> ()
%instance_1 = apply %getKlass() : $@convention(thin) () -> @owned Klass
%src = alloc_stack $Klass
store %instance_1 to [init] %src : $*Klass
apply %user(%src) : $@convention(thin) (@in_guaranteed Klass) -> ()
%tmp = alloc_stack $Klass
copy_addr [take] %src to [init] %tmp : $*Klass
%instance_2 = load [copy] %tmp : $*Klass
destroy_addr %tmp : $*Klass
dealloc_stack %tmp : $*Klass
dealloc_stack %src : $*Klass
return %instance_2 : $Klass
}

// CHECK-LABEL: sil [ossa] @take_from_original_copy_addr__final_use_copy_addr_copy : {{.*}} {
// CHECK: {{bb[0-9]+}}([[OUT:%[^,]+]] : $*Klass):
// CHECK: [[GET:%[^,]+]] = function_ref @getKlass
// CHECK: [[USER:%[^,]+]] = function_ref @inguaranteed_user_without_result
// CHECK: [[INSTANCE:%[^,]+]] = apply [[GET]]()
// CHECK: [[SRC:%[^,]+]] = alloc_stack $Klass
// CHECK: store [[INSTANCE]] to [init] [[SRC]]
// CHECK: apply [[USER]]([[SRC]])
// CHECK: copy_addr [[SRC]] to [init] [[OUT]]
// CHECK: destroy_addr [[SRC]]
// CHECK: dealloc_stack [[SRC]]
// CHECK-LABEL: } // end sil function 'take_from_original_copy_addr__final_use_copy_addr_copy'
sil [ossa] @take_from_original_copy_addr__final_use_copy_addr_copy : $() -> @out Klass {
entry(%out : $*Klass):
%getKlass = function_ref @getKlass : $@convention(thin) () -> @owned Klass
%user = function_ref @inguaranteed_user_without_result : $@convention(thin) (@in_guaranteed Klass) -> ()
%instance_1 = apply %getKlass() : $@convention(thin) () -> @owned Klass
%src = alloc_stack $Klass
store %instance_1 to [init] %src : $*Klass
apply %user(%src) : $@convention(thin) (@in_guaranteed Klass) -> ()
%tmp = alloc_stack $Klass
copy_addr [take] %src to [init] %tmp : $*Klass
copy_addr %tmp to [init] %out : $*Klass
destroy_addr %tmp : $*Klass
dealloc_stack %tmp : $*Klass
dealloc_stack %src : $*Klass
%retval = tuple ()
return %retval : $()
}

// CHECK-LABEL: sil [ossa] @take_from_original_copy_addr__final_use_apply__move_only : {{.*}} {
// CHECK: [[GET:%[^,]+]] = function_ref @getMOS
// CHECK: [[USER:%[^,]+]] = function_ref @inguaranteed_user_without_result_MOS
// CHECK: [[INSTANCE:%[^,]+]] = apply [[GET]]()
// CHECK: [[SRC:%[^,]+]] = alloc_stack $MOS
// CHECK: store [[INSTANCE]] to [init] [[SRC]]
// CHECK: apply [[USER]]([[SRC]])
// CHECK: apply [[USER]]([[SRC]])
// CHECK: destroy_addr [[SRC]]
// CHECK: dealloc_stack [[SRC]]
// CHECK-LABEL: } // end sil function 'take_from_original_copy_addr__final_use_apply__move_only'
sil [ossa] @take_from_original_copy_addr__final_use_apply__move_only : $() -> () {
%getMOS = function_ref @getMOS : $@convention(thin) () -> @owned MOS
%user = function_ref @inguaranteed_user_without_result_MOS : $@convention(thin) (@in_guaranteed MOS) -> ()
%instance_1 = apply %getMOS() : $@convention(thin) () -> @owned MOS
%src = alloc_stack $MOS
store %instance_1 to [init] %src : $*MOS
apply %user(%src) : $@convention(thin) (@in_guaranteed MOS) -> ()
%tmp = alloc_stack $MOS
copy_addr [take] %src to [init] %tmp : $*MOS
apply %user(%tmp) : $@convention(thin) (@in_guaranteed MOS) -> ()
destroy_addr %tmp : $*MOS
dealloc_stack %tmp : $*MOS
dealloc_stack %src : $*MOS
%tuple = tuple ()
return %tuple : $()
}

// This does not get optimized correctly because of the conservative treatment of load_borrow/end_borrow in MemBehavior
// CHECK-LABEL: sil [ossa] @test_temprvoborrowboundary1 :
// CHECK: copy_addr
Expand Down Expand Up @@ -1584,3 +1793,31 @@ entry(%instance : $*NonTrivialStruct):
%retval = tuple ()
return %retval : $()
}

// Verify that no copy of an instance of the move-only type MOS is introduced.
// CHECK-LABEL: sil hidden [ossa] @dont_copy_move_only_struct : {{.*}} {
// CHECK: [[SRC:%[^,]+]] = alloc_stack $MOS
// CHECK: [[GET:%[^,]+]] = function_ref @getMOS
// CHECK: [[INSTANCE_1:%[^,]+]] = apply [[GET]]()
// CHECK: store [[INSTANCE_1]] to [init] [[SRC]]
// CHECK: [[INSTANCE_2:%[^,]+]] = load [take] [[SRC]]
// CHECK: store [[INSTANCE_2]] to [init] [[SRC]]
// CHECK: [[INSTANCE_3:%[^,]+]] = load [take] [[SRC]]
// CHECK: dealloc_stack [[SRC]]
// CHECK: return [[INSTANCE_3]]
// CHECK-LABEL: } // end sil function 'dont_copy_move_only_struct'
sil hidden [ossa] @dont_copy_move_only_struct : $@convention(thin) () -> @owned MOS {
bb0:
%src = alloc_stack $MOS
%getMOS = function_ref @getMOS : $@convention(thin) () -> @owned MOS
%instance_1 = apply %getMOS() : $@convention(thin) () -> @owned MOS
store %instance_1 to [init] %src : $*MOS
%tmp = alloc_stack $MOS
copy_addr [take] %src to [init] %tmp : $*MOS
%instance_2 = load [take] %tmp : $*MOS
store %instance_2 to [init] %src : $*MOS
%instance_3 = load [take] %src : $*MOS
dealloc_stack %tmp : $*MOS
dealloc_stack %src : $*MOS
return %instance_3 : $MOS
}