Skip to content

Commit aa9d147

Browse files
authored
Merge pull request #41472 from nate-chandler/lexical_lifetimes/lexical_destroy_addr_hoisting/fixes/20220218/1
[SSADestroyHoisting] Aliasable addr args respect deinit barriers.
2 parents a96e495 + 39078cf commit aa9d147

File tree

2 files changed

+59
-6
lines changed

2 files changed

+59
-6
lines changed

lib/SILOptimizer/Transforms/SSADestroyHoisting.cpp

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -634,9 +634,13 @@ bool HoistDestroys::foldBarrier(SILInstruction *barrier,
634634
const KnownStorageUses &knownUses,
635635
const DeinitBarriers &deinitBarriers) {
636636
if (auto *eai = dyn_cast<EndAccessInst>(barrier)) {
637+
auto *bai = eai->getBeginAccess();
638+
// Don't hoist a destroy into an unrelated access scope.
639+
if (stripAccessMarkers(bai) != stripAccessMarkers(storageRoot))
640+
return false;
637641
SILInstruction *instruction = eai;
638642
while ((instruction = instruction->getPreviousInstruction())) {
639-
if (instruction == eai->getBeginAccess())
643+
if (instruction == bai)
640644
return false;
641645
if (foldBarrier(instruction, storageRoot))
642646
return true;
@@ -817,11 +821,17 @@ void SSADestroyHoisting::run() {
817821
// Arguments enclose everything.
818822
for (auto *arg : getFunction()->getArguments()) {
819823
if (arg->getType().isAddress()) {
820-
bool isInout = cast<SILFunctionArgument>(arg)
821-
->getArgumentConvention()
822-
.isInoutConvention();
823-
changed |= hoistDestroys(arg, /*ignoreDeinitBarriers=*/
824-
isInout, remainingDestroyAddrs, deleter);
824+
auto convention = cast<SILFunctionArgument>(arg)->getArgumentConvention();
825+
// This is equivalent to writing
826+
//
827+
// convention == SILArgumentConvention::Indirect_Inout
828+
//
829+
// but communicates the rationale: in order to ignore deinit barriers, the
830+
// address must be exclusively accessed and be a modification.
831+
bool ignoreDeinitBarriers = convention.isInoutConvention() &&
832+
convention.isExclusiveIndirectParameter();
833+
changed |= hoistDestroys(arg, ignoreDeinitBarriers, remainingDestroyAddrs,
834+
deleter);
825835
}
826836
}
827837

test/SILOptimizer/hoist_destroy_addr.sil

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,21 @@ entry(%instance : @none $TrivialStruct):
255255
return %retval : $()
256256
}
257257

258+
// CHECK-LABEL: sil [ossa] @nohoist_inout_aliasable : {{.*}} {
259+
// CHECK: apply
260+
// CHECK: destroy_addr
261+
// CHECK-LABEL: } // end sil function 'nohoist_inout_aliasable'
262+
sil [ossa] @nohoist_inout_aliasable : $@convention(thin) (@inout_aliasable X) -> () {
263+
entry(%addr : $*X):
264+
%value = load [copy] %addr : $*X
265+
%unknown = function_ref @unknown : $@convention(thin) () -> ()
266+
apply %unknown() : $@convention(thin) () -> ()
267+
destroy_addr %addr : $*X
268+
store %value to [init] %addr : $*X
269+
%tuple = tuple ()
270+
return %tuple : $()
271+
}
272+
258273
// Fold destroy_addr and a load [copy] into a load [take] even when that
259274
// load [take] is guarded by an access scope.
260275
//
@@ -398,3 +413,31 @@ entry(%instance : @owned $X):
398413

399414
return %loaded : $X
400415
}
416+
417+
// Don't fold a destroy_addr with a load [copy] that occurs within the scope of
418+
// an access to unrelated storage.
419+
//
420+
// CHECK-LABEL: sil [ossa] @nofold_into_unrelated_barrier_scope : {{.*}} {
421+
// CHECK: load [copy]
422+
// CHECK-LABEL: } // end sil function 'nofold_into_unrelated_barrier_scope'
423+
sil [ossa] @nofold_into_unrelated_barrier_scope : $@convention(thin) (@owned X) -> (@owned X) {
424+
entry(%instance : @owned $X):
425+
%copy = copy_value %instance : $X
426+
%addr_outer = alloc_stack $X
427+
%addr_inner = alloc_stack $X
428+
store %copy to [init] %addr_outer : $*X
429+
store %instance to [init] %addr_inner : $*X
430+
431+
%access = begin_access [modify] [static] %addr_outer : $*X
432+
apply undef(%access) : $@convention(thin) (@inout X) -> ()
433+
%unknown = function_ref @unknown : $@convention(thin) () -> ()
434+
destroy_addr %access : $*X
435+
apply %unknown() : $@convention(thin) () -> ()
436+
%value = load [copy] %addr_inner : $*X
437+
end_access %access : $*X
438+
destroy_addr %addr_inner : $*X
439+
440+
dealloc_stack %addr_inner : $*X
441+
dealloc_stack %addr_outer : $*X
442+
return %value : $X
443+
}

0 commit comments

Comments
 (0)