Skip to content

Commit 87d0c9f

Browse files
committed
[SSADestroyHoisting] Bitcasts obstruct folding.
Make loads and copy_addrs of casts of the underlying storage barriers to folding. Destroying the target address may not be equivalent to destroying the source address: for example, if the target address is a generic and the source address is AnyObject, specialization may turn the generic into a trivial type; the destruction of that trivial type fails to destroy the original stored AnyObject, resulting in a leak.
1 parent 266668d commit 87d0c9f

File tree

2 files changed

+59
-5
lines changed

2 files changed

+59
-5
lines changed

lib/SILOptimizer/Transforms/SSADestroyHoisting.cpp

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -786,8 +786,15 @@ bool HoistDestroys::checkFoldingBarrier(
786786
SILValue address;
787787
if (auto *load = dyn_cast<LoadInst>(instruction)) {
788788
auto loadee = load->getOperand();
789-
auto accessPath = AccessPath::compute(loadee);
790-
if (accessPath.getStorage().hasIdenticalStorage(storage)) {
789+
auto relativeAccessStorage = RelativeAccessStorageWithBase::compute(loadee);
790+
if (relativeAccessStorage.getStorage().hasIdenticalStorage(storage)) {
791+
// If the access path from the loaded address to its root storage involves
792+
// a (layout non-equivalent) typecast--a load [take] of the casted address
793+
// would not be equivalent to a load [copy] followed by a destroy_addr of
794+
// the corresponding uncast projection--the truncated portion might have
795+
// refcounted components.
796+
if (relativeAccessStorage.cast == AccessStorageCast::Type)
797+
return true;
791798
if (load->getOwnershipQualifier() == LoadOwnershipQualifier::Copy) {
792799
address = loadee;
793800
loads.push_back(load);
@@ -798,8 +805,15 @@ bool HoistDestroys::checkFoldingBarrier(
798805
}
799806
} else if (auto *copy = dyn_cast<CopyAddrInst>(instruction)) {
800807
auto source = copy->getSrc();
801-
auto accessPath = AccessPath::compute(source);
802-
if (accessPath.getStorage().hasIdenticalStorage(storage)) {
808+
auto relativeAccessStorage = RelativeAccessStorageWithBase::compute(source);
809+
if (relativeAccessStorage.getStorage().hasIdenticalStorage(storage)) {
810+
// If the access path from the copy_addr'd address to its root storage
811+
// involves a (layout non-equivalent) typecast--a copy_addr [take] of the
812+
// casted address would not be equivalent to a copy_addr followed by a
813+
// destroy_addr of the corresponding uncast projection--the truncated
814+
// portion might have refcounted components.
815+
if (relativeAccessStorage.cast == AccessStorageCast::Type)
816+
return true;
803817
address = source;
804818
copies.push_back(copy);
805819
}

test/SILOptimizer/hoist_destroy_addr.sil

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ sil_stage canonical
77

88
import Builtin
99

10+
typealias AnyObject = Builtin.AnyObject
11+
1012
class X {
1113
}
1214

@@ -390,7 +392,8 @@ entry(%addr : $*X, %instance : @owned $X):
390392
// CHECK-LABEL: sil [ossa] @hoist_upto_address_to_pointer_use : {{.*}} {
391393
// CHECK: address_to_pointer
392394
// CHECK: pointer_to_address
393-
// CHECK: load [take]
395+
// CHECK: load [copy]
396+
// CHECK: destroy_addr
394397
// CHECK: tuple
395398
// CHECK-LABEL: } // end sil function 'hoist_upto_address_to_pointer_use'
396399
sil [ossa] @hoist_upto_address_to_pointer_use : $@convention(thin) (@in X) -> (@owned X) {
@@ -795,3 +798,40 @@ entry(%instance : @owned $(X, I)):
795798
%retval = tuple (%copy : $(X, I), %i_2 : $I)
796799
return %retval : $((X, I), I)
797800
}
801+
802+
// Don't fold a destroy_addr of underlying storage into the copy_addr of a
803+
// bitcast of that address.
804+
//
805+
// CHECK-LABEL: sil [ossa] @nofold_destroy_addr_original_into_copy_addr_cast : {{.*}} {
806+
// CHECK: copy_addr {{%[^,]+}}
807+
// CHECK: destroy_addr
808+
// CHECK-LABEL: } // end sil function 'nofold_destroy_addr_original_into_copy_addr_cast'
809+
sil [ossa] @nofold_destroy_addr_original_into_copy_addr_cast : $@convention(thin) <T> (@owned AnyObject) -> @out T {
810+
entry(%out : $*T, %instance : @owned $AnyObject):
811+
%addr = alloc_stack $AnyObject
812+
store %instance to [init] %addr : $*AnyObject
813+
%cast_addr = unchecked_addr_cast %addr : $*AnyObject to $*T
814+
copy_addr %cast_addr to [initialization] %out : $*T
815+
destroy_addr %addr : $*AnyObject
816+
dealloc_stack %addr : $*AnyObject
817+
%retval = tuple ()
818+
return %retval : $()
819+
}
820+
821+
// Don't fold a destroy_addr of underlying storage into the load [copy] of a
822+
// bitcast of that address.
823+
//
824+
// CHECK-LABEL: sil [ossa] @nofold_destroy_addr_original_into_load_cast : {{.*}} {
825+
// CHECK: load [copy]
826+
// CHECK: destroy_addr
827+
// CHECK-LABEL: } // end sil function 'nofold_destroy_addr_original_into_load_cast'
828+
sil [ossa] @nofold_destroy_addr_original_into_load_cast : $@convention(thin) (@owned AnyObject) -> @owned X {
829+
entry(%instance : @owned $AnyObject):
830+
%addr = alloc_stack $AnyObject
831+
store %instance to [init] %addr : $*AnyObject
832+
%cast_addr = unchecked_addr_cast %addr : $*AnyObject to $*X
833+
%cast = load [copy] %cast_addr : $*X
834+
destroy_addr %addr : $*AnyObject
835+
dealloc_stack %addr : $*AnyObject
836+
return %cast : $X
837+
}

0 commit comments

Comments
 (0)