Skip to content

[DestroyAddrHoisting] Don't fold into read access. #82557

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.

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 1 commit into from
Jun 27, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 10 additions & 0 deletions lib/SILOptimizer/Transforms/DestroyAddrHoisting.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -741,6 +741,11 @@ bool HoistDestroys::checkFoldingBarrier(
auto loadee = load->getOperand();
auto relativeAccessStorage = RelativeAccessStorageWithBase::compute(loadee);
if (relativeAccessStorage.getStorage().hasIdenticalStorage(storage)) {
// Only fold into access scopes which aren't [read].
auto scoped = AccessStorageWithBase::computeInScope(loadee);
auto *bai = dyn_cast_or_null<BeginAccessInst>(scoped.base);
if (bai && bai->getAccessKind() == SILAccessKind::Read)
return true;
// If the access path from the loaded address to its root storage involves
// a (layout non-equivalent) typecast--a load [take] of the casted address
// would not be equivalent to a load [copy] followed by a destroy_addr of
Expand All @@ -760,6 +765,11 @@ bool HoistDestroys::checkFoldingBarrier(
auto source = copy->getSrc();
auto relativeAccessStorage = RelativeAccessStorageWithBase::compute(source);
if (relativeAccessStorage.getStorage().hasIdenticalStorage(storage)) {
// Only fold into access scopes which aren't [read].
auto scoped = AccessStorageWithBase::computeInScope(source);
auto *bai = dyn_cast_or_null<BeginAccessInst>(scoped.base);
if (bai && bai->getAccessKind() == SILAccessKind::Read)
return true;
// If the access path from the copy_addr'd address to its root storage
// involves a (layout non-equivalent) typecast--a copy_addr [take] of the
// casted address would not be equivalent to a copy_addr followed by a
Expand Down
46 changes: 38 additions & 8 deletions test/SILOptimizer/hoist_destroy_addr.sil
Original file line number Diff line number Diff line change
Expand Up @@ -684,7 +684,7 @@ entry(%instance : @owned $S):
%store_scope = begin_access [modify] [static] %addr : $*S
store %instance to [init] %store_scope : $*S
end_access %store_scope : $*S
%load_scope = begin_access [read] [static] %addr : $*S
%load_scope = begin_access [modify] [static] %addr : $*S
%value = load [copy] %load_scope : $*S
end_access %load_scope : $*S
destroy_addr %addr : $*S
Expand All @@ -703,7 +703,7 @@ entry(%instance : @owned $S):
%store_scope = begin_access [modify] [static] %addr : $*S
store %instance to [init] %store_scope : $*S
end_access %store_scope : $*S
%load_scope = begin_access [read] [static] %addr : $*S
%load_scope = begin_access [modify] [static] %addr : $*S
%value = load [copy] %load_scope : $*S
%unknown = function_ref @unknown : $@convention(thin) () -> ()
apply %unknown() : $@convention(thin) () -> ()
Expand All @@ -725,7 +725,7 @@ entry(%instance : @owned $S):
%store_scope = begin_access [modify] [static] %addr : $*S
store %instance to [init] %store_scope : $*S
end_access %store_scope : $*S
%load_scope = begin_access [read] [static] %addr : $*S
%load_scope = begin_access [modify] [static] %addr : $*S
%value = load [copy] %load_scope : $*S
%unknown = function_ref @unknown : $@convention(thin) () -> ()
apply %unknown() : $@convention(thin) () -> ()
Expand All @@ -746,7 +746,7 @@ entry(%instance : @owned $S):
%store_scope = begin_access [modify] [static] %addr : $*S
store %instance to [init] %store_scope : $*S
end_access %store_scope : $*S
%load_scope = begin_access [read] [static] %addr : $*S
%load_scope = begin_access [modify] [static] %addr : $*S
%field_addr = struct_element_addr %load_scope : $*S, #S.x
%field = load [copy] %field_addr : $*X
end_access %load_scope : $*S
Expand All @@ -768,9 +768,9 @@ entry(%instance : @owned $S):
%store_scope = begin_access [modify] [static] %addr : $*S
store %instance to [init] %store_scope : $*S
end_access %store_scope : $*S
%outer = begin_access [read] [static] %addr : $*S
%outer = begin_access [modify] [static] %addr : $*S
apply undef(%outer) : $@convention(thin) (@inout S) -> ()
%inner = begin_access [read] [static] %outer : $*S
%inner = begin_access [modify] [static] %outer : $*S
%field_addr = struct_element_addr %inner : $*S, #S.x
%field_addr_2 = struct_element_addr %addr_2 : $*S, #S.x
copy_addr %field_addr to [init] %field_addr_2 : $*X
Expand All @@ -795,9 +795,9 @@ entry(%instance : @owned $S):
%store_scope = begin_access [modify] [static] %addr : $*S
store %instance to [init] %store_scope : $*S
end_access %store_scope : $*S
%outer = begin_access [read] [static] %addr : $*S
%outer = begin_access [modify] [static] %addr : $*S
apply undef(%outer) : $@convention(thin) (@inout S) -> ()
%inner = begin_access [read] [static] %outer : $*S
%inner = begin_access [modify] [static] %outer : $*S
copy_addr %inner to [init] %addr_2 : $*S
end_access %inner : $*S
destroy_addr %outer : $*S
Expand Down Expand Up @@ -1316,3 +1316,33 @@ bb0(%out : $*TwoCases):
inject_enum_addr %out, #TwoCases.A!enumelt
return %retval
}

// CHECK-LABEL: sil [ossa] @nofold_read_access_load : {{.*}} {
// CHECK: load [copy]
// CHECK-LABEL: } // end sil function 'nofold_read_access_load'
sil [ossa] @nofold_read_access_load : $@convention(thin) (@in X) -> () {
entry(%c_addr : $*X):
%c_access = begin_access [dynamic] [read] %c_addr
%c = load [copy] %c_access
end_access %c_access
destroy_addr %c_addr
apply undef(%c) : $@convention(thin) (@owned X) -> ()
%retval = tuple ()
return %retval
}

// CHECK-LABEL: sil [ossa] @nofold_read_access_copy_addr : {{.*}} {
// CHECK: copy_addr {{%[^,]+}} to [init]
// CHECK-LABEL: } // end sil function 'nofold_read_access_copy_addr'
sil [ossa] @nofold_read_access_copy_addr : $@convention(thin) (@in X) -> () {
entry(%c_addr : $*X):
%stack = alloc_stack $X
%c_access = begin_access [dynamic] [read] %c_addr
copy_addr %c_access to [init] %stack
end_access %c_access
destroy_addr %c_addr
apply undef(%stack) : $@convention(thin) (@in X) -> ()
dealloc_stack %stack
%retval = tuple ()
return %retval
}