Skip to content

Commit 028afc0

Browse files
eecksteineeckstein
committed
DeadObjectElimination: delete dead arrays for which the "destroyArray" builtin is inlined.
This is needed for non-OSSA SIL: in case we see a destroyArray builtin, we can safely remove the otherwise dead array. This optimization kicks in later in the pipeline, after array semantics are already inlined (for early SIL, DeadObjectElimination can remove arrays based on semantics). rdar://73569282 https://bugs.swift.org/browse/SR-14100
1 parent 34ae1d9 commit 028afc0

File tree

3 files changed

+338
-8
lines changed

3 files changed

+338
-8
lines changed

‎lib/SILOptimizer/Transforms/DeadObjectElimination.cpp

Lines changed: 116 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
#include "swift/SIL/SILInstruction.h"
3737
#include "swift/SIL/SILModule.h"
3838
#include "swift/SIL/SILUndef.h"
39+
#include "swift/SIL/MemAccessUtils.h"
3940
#include "swift/SILOptimizer/Analysis/ArraySemantic.h"
4041
#include "swift/SILOptimizer/PassManager/Passes.h"
4142
#include "swift/SILOptimizer/PassManager/Transforms.h"
@@ -248,18 +249,112 @@ static bool canZapInstruction(SILInstruction *Inst, bool acceptRefCountInsts,
248249
return false;
249250
}
250251

252+
/// Returns true if all stores in \p users store to the tail elements of
253+
/// \p allocRef, which are destroyed by the \p destroyArray builtin.
254+
static bool onlyStoresToTailObjects(BuiltinInst *destroyArray,
255+
const UserList &users,
256+
AllocRefInst *allocRef) {
257+
// Get the number of destroyed elements.
258+
auto *literal = dyn_cast<IntegerLiteralInst>(destroyArray->getArguments()[2]);
259+
if (!literal || literal->getValue().getMinSignedBits() > 32)
260+
return false;
261+
int numDestroyed = literal->getValue().getSExtValue();
262+
263+
SILFunction *func = destroyArray->getFunction();
264+
SILBasicBlock *storesBlock = nullptr;
265+
266+
// Check if the destroyArray destroys the tail elements of allocRef.
267+
auto destroyPath = AccessPath::compute(destroyArray->getArguments()[1]);
268+
if (destroyPath != AccessPath::forTailStorage(allocRef))
269+
return false;
270+
271+
SmallVector<AccessPath, 32> pathsToCheck;
272+
273+
// Check all stores to the tail elements.
274+
for (SILInstruction *user : users) {
275+
auto *store = dyn_cast<StoreInst>(user);
276+
if (!store)
277+
continue;
278+
279+
assert(users.count(store->getSrc()->getDefiningInstruction()) == 0 &&
280+
"Storing a use of an array (that would mean the array escapes)?");
281+
282+
// All stores must be in the same block. This ensure that the stores
283+
// dominate the destroyArray (which may be in a different block).
284+
if (storesBlock && store->getParent() != storesBlock)
285+
return false;
286+
storesBlock = store->getParent();
287+
288+
AccessPath storePath = AccessPath::compute(store->getDest());
289+
if (!storePath.isValid())
290+
return false;
291+
292+
// We don't care about trivial stores.
293+
if (store->getSrc()->getType().isTrivial(*func))
294+
continue;
295+
296+
// Check if it's a store to the tail elements.
297+
if (!destroyPath.contains(storePath.withOffset(0)))
298+
return false;
299+
300+
// Check if the store is within the range of the destroyed array. In OSSA
301+
// we would not need this check. Otherwise it would be a memory lifetime
302+
// failure.
303+
if (storePath.getOffset() < 0 || storePath.getOffset() >= numDestroyed)
304+
return false;
305+
306+
pathsToCheck.push_back(storePath);
307+
}
308+
309+
// In non-OSSA we have to check if two paths overlap, because we could end up
310+
// over-releasing the stored objects.
311+
// Group the paths by tail-element index, so that we only have to check within
312+
// a tail-element group.
313+
std::sort(pathsToCheck.begin(), pathsToCheck.end(), [](AccessPath p1, AccessPath p2) {
314+
return p1.getOffset() < p2.getOffset();
315+
});
316+
for (unsigned i = 0, n = pathsToCheck.size(); i < n; ++i) {
317+
for (unsigned j = i + 1;
318+
j < n && pathsToCheck[i].getOffset() == pathsToCheck[j].getOffset(); ++j) {
319+
if (pathsToCheck[i].mayOverlap(pathsToCheck[j]))
320+
return false;
321+
// Limit the number of checks to avoid quadratic complexity.
322+
if (j > i + 8)
323+
return false;
324+
}
325+
}
326+
return true;
327+
}
328+
329+
static bool isDestroyArray(SILInstruction *inst) {
330+
BuiltinInst *bi = dyn_cast<BuiltinInst>(inst);
331+
return bi && bi->getBuiltinInfo().ID == BuiltinValueKind::DestroyArray;
332+
}
333+
334+
/// Inserts releases of all stores in \p users.
335+
static void insertCompensatingReleases(SILInstruction *before,
336+
const UserList &users) {
337+
for (SILInstruction *user : users) {
338+
if (auto *store = dyn_cast<StoreInst>(user)) {
339+
createDecrementBefore(store->getSrc(), before);
340+
}
341+
}
342+
}
343+
251344
/// Analyze the use graph of AllocRef for any uses that would prevent us from
252345
/// zapping it completely.
253346
static bool
254-
hasUnremovableUsers(SILInstruction *AllocRef, UserList *Users,
347+
hasUnremovableUsers(SILInstruction *allocation, UserList *Users,
255348
bool acceptRefCountInsts, bool onlyAcceptTrivialStores) {
256349
SmallVector<SILInstruction *, 16> Worklist;
257-
Worklist.push_back(AllocRef);
350+
Worklist.push_back(allocation);
258351

259352
LLVM_DEBUG(llvm::dbgs() << " Analyzing Use Graph.");
260353

261354
SmallVector<RefElementAddrInst *, 8> refElementAddrs;
262355
bool deallocationMaybeInlined = false;
356+
BuiltinInst *destroyArray = nullptr;
357+
AllocRefInst *allocRef = dyn_cast<AllocRefInst>(allocation);
263358

264359
while (!Worklist.empty()) {
265360
SILInstruction *I = Worklist.pop_back_val();
@@ -273,17 +368,19 @@ hasUnremovableUsers(SILInstruction *AllocRef, UserList *Users,
273368
continue;
274369
}
275370

276-
// If we can't zap this instruction... bail...
277-
if (!canZapInstruction(I, acceptRefCountInsts, onlyAcceptTrivialStores)) {
278-
LLVM_DEBUG(llvm::dbgs() << " Found instruction we can't zap...\n");
279-
return true;
280-
}
281-
282371
if (auto *rea = dyn_cast<RefElementAddrInst>(I)) {
283372
if (!rea->getType().isTrivial(*rea->getFunction()))
284373
refElementAddrs.push_back(rea);
285374
} else if (isa<SetDeallocatingInst>(I)) {
286375
deallocationMaybeInlined = true;
376+
} else if (allocRef && Users && isDestroyArray(I)) {
377+
if (destroyArray)
378+
return true;
379+
destroyArray = cast<BuiltinInst>(I);
380+
} else if (!canZapInstruction(I, acceptRefCountInsts,
381+
onlyAcceptTrivialStores)) {
382+
LLVM_DEBUG(llvm::dbgs() << " Found instruction we can't zap...\n");
383+
return true;
287384
}
288385

289386
// At this point, we can remove the instruction as long as all of its users
@@ -309,6 +406,12 @@ hasUnremovableUsers(SILInstruction *AllocRef, UserList *Users,
309406
}
310407
}
311408

409+
// In OSSA, we don't have to do this check. We can always accept a
410+
// destroyArray and insert the compensating destroys right at the store
411+
// instructions.
412+
if (destroyArray)
413+
return !onlyStoresToTailObjects(destroyArray, *Users, allocRef);
414+
312415
if (deallocationMaybeInlined) {
313416
// The alloc_ref is not destructed by a strong_release which is calling the
314417
// deallocator (destroying all stored properties).
@@ -767,6 +870,11 @@ bool DeadObjectElimination::processAllocRef(AllocRefInst *ARI) {
767870
return false;
768871
}
769872

873+
auto iter = std::find_if(UsersToRemove.begin(), UsersToRemove.end(),
874+
isDestroyArray);
875+
if (iter != UsersToRemove.end())
876+
insertCompensatingReleases(*iter, UsersToRemove);
877+
770878
// Remove the AllocRef and all of its users.
771879
removeInstructions(
772880
ArrayRef<SILInstruction*>(UsersToRemove.begin(), UsersToRemove.end()));

‎test/SILOptimizer/dead_alloc_elim.sil

Lines changed: 199 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import Swift
44
import Builtin
5+
import SwiftShims
56

67
//////////
78
// Data //
@@ -22,6 +23,11 @@ class NontrivialDestructor {
2223
init()
2324
}
2425

26+
class ArrayStorage {
27+
@_hasStorage var bodyField : Kl
28+
init()
29+
}
30+
2531
sil @$s4main17TrivialDestructorCfD : $@convention(method) (@owned TrivialDestructor) -> () {
2632
bb0(%0 : $TrivialDestructor):
2733
// Alloc/Dealloc stack should not disrupt elimination of the
@@ -445,3 +451,196 @@ bb0(%0 : $X):
445451
return %10 : $()
446452
}
447453

454+
// CHECK-LABEL: sil @remove_dead_array_with_destroy_simple
455+
// CHECK-NOT: alloc_ref
456+
// CHECK-NOT: dealloc_ref
457+
// CHECK: strong_release %0 : $Kl
458+
// CHECK-NEXT: strong_release %0 : $Kl
459+
// CHECK-NEXT: tuple
460+
// CHECK-NEXT: return
461+
// CHECK: } // end sil function 'remove_dead_array_with_destroy_simple'
462+
sil @remove_dead_array_with_destroy_simple : $@convention(thin) (@owned Kl) -> () {
463+
bb0(%0 : $Kl):
464+
%3 = integer_literal $Builtin.Word, 2
465+
%4 = alloc_ref [tail_elems $Kl * %3 : $Builtin.Word] $ArrayStorage
466+
%11 = ref_tail_addr %4 : $ArrayStorage, $Kl
467+
store %0 to %11 : $*Kl
468+
%27 = integer_literal $Builtin.Word, 1
469+
%28 = index_addr %11 : $*Kl, %27 : $Builtin.Word
470+
retain_value %0 : $Kl
471+
store %0 to %28 : $*Kl
472+
set_deallocating %4 : $ArrayStorage
473+
%65 = address_to_pointer %11 : $*Kl to $Builtin.RawPointer
474+
%66 = metatype $@thick Kl.Type
475+
%67 = builtin "destroyArray"<Kl>(%66 : $@thick Kl.Type, %65 : $Builtin.RawPointer, %3 : $Builtin.Word) : $()
476+
dealloc_ref %4 : $ArrayStorage
477+
%10 = tuple ()
478+
return %10 : $()
479+
}
480+
481+
// CHECK-LABEL: sil @remove_dead_array_with_destroy_complex
482+
// CHECK-NOT: alloc_ref
483+
// CHECK-NOT: dealloc_ref
484+
// CHECK: release_value %0 : $String
485+
// CHECK-NEXT: release_value %0 : $String
486+
// CHECK-NEXT: tuple
487+
// CHECK-NEXT: return
488+
// CHECK: } // end sil function 'remove_dead_array_with_destroy_complex'
489+
sil @remove_dead_array_with_destroy_complex : $@convention(thin) (@guaranteed String, Int, UInt) -> () {
490+
bb0(%0 : $String, %1 : $Int, %2 : $UInt):
491+
%3 = integer_literal $Builtin.Word, 2
492+
%4 = alloc_ref [stack] [tail_elems $(Int, String) * %3 : $Builtin.Word] $_ContiguousArrayStorage<(Int, String)>
493+
%5 = upcast %4 : $_ContiguousArrayStorage<(Int, String)> to $__ContiguousArrayStorageBase
494+
%6 = struct $_SwiftArrayBodyStorage (%1 : $Int, %2 : $UInt)
495+
%7 = struct $_ArrayBody (%6 : $_SwiftArrayBodyStorage)
496+
%9 = ref_element_addr %5 : $__ContiguousArrayStorageBase, #__ContiguousArrayStorageBase.countAndCapacity
497+
store %7 to %9 : $*_ArrayBody
498+
%11 = ref_tail_addr %5 : $__ContiguousArrayStorageBase, $(Int, String)
499+
%12 = tuple_element_addr %11 : $*(Int, String), 0
500+
%13 = tuple_element_addr %11 : $*(Int, String), 1
501+
retain_value %0 : $String
502+
store %1 to %12 : $*Int
503+
store %0 to %13 : $*String
504+
%27 = integer_literal $Builtin.Word, 1
505+
%28 = index_addr %11 : $*(Int, String), %27 : $Builtin.Word
506+
%29 = tuple_element_addr %28 : $*(Int, String), 0
507+
%30 = tuple_element_addr %28 : $*(Int, String), 1
508+
retain_value %0 : $String
509+
store %1 to %29 : $*Int
510+
store %0 to %30 : $*String
511+
set_deallocating %4 : $_ContiguousArrayStorage<(Int, String)>
512+
%64 = ref_tail_addr %4 : $_ContiguousArrayStorage<(Int, String)>, $(Int, String)
513+
%65 = address_to_pointer %64 : $*(Int, String) to $Builtin.RawPointer
514+
%66 = metatype $@thick (Int, String).Type
515+
%67 = builtin "destroyArray"<(Int, String)>(%66 : $@thick (Int, String).Type, %65 : $Builtin.RawPointer, %3 : $Builtin.Word) : $()
516+
fix_lifetime %4 : $_ContiguousArrayStorage<(Int, String)>
517+
dealloc_ref %4 : $_ContiguousArrayStorage<(Int, String)>
518+
dealloc_ref [stack] %4 : $_ContiguousArrayStorage<(Int, String)>
519+
%10 = tuple ()
520+
return %10 : $()
521+
}
522+
523+
// CHECK-LABEL: sil @dont_remove_dead_array_overlapping_stores
524+
// CHECK: alloc_ref
525+
// CHECK: dealloc_ref
526+
// CHECK: } // end sil function 'dont_remove_dead_array_overlapping_stores'
527+
sil @dont_remove_dead_array_overlapping_stores : $@convention(thin) (@owned Kl) -> () {
528+
bb0(%0 : $Kl):
529+
%3 = integer_literal $Builtin.Word, 2
530+
%4 = alloc_ref [tail_elems $Kl * %3 : $Builtin.Word] $ArrayStorage
531+
%11 = ref_tail_addr %4 : $ArrayStorage, $Kl
532+
%27 = integer_literal $Builtin.Word, 1
533+
%28 = index_addr %11 : $*Kl, %27 : $Builtin.Word
534+
retain_value %0 : $Kl
535+
store %0 to %28 : $*Kl
536+
store %0 to %11 : $*Kl
537+
%30 = index_addr %11 : $*Kl, %27 : $Builtin.Word
538+
retain_value %0 : $Kl
539+
store %0 to %30 : $*Kl // Overwrites the first store
540+
set_deallocating %4 : $ArrayStorage
541+
%65 = address_to_pointer %11 : $*Kl to $Builtin.RawPointer
542+
%66 = metatype $@thick Kl.Type
543+
%67 = builtin "destroyArray"<Kl>(%66 : $@thick Kl.Type, %65 : $Builtin.RawPointer, %3 : $Builtin.Word) : $()
544+
dealloc_ref %4 : $ArrayStorage
545+
%10 = tuple ()
546+
return %10 : $()
547+
}
548+
549+
// CHECK-LABEL: sil @dont_remove_dead_array_store_to_body
550+
// CHECK: alloc_ref
551+
// CHECK: dealloc_ref
552+
// CHECK: } // end sil function 'dont_remove_dead_array_store_to_body'
553+
sil @dont_remove_dead_array_store_to_body : $@convention(thin) (@owned Kl) -> () {
554+
bb0(%0 : $Kl):
555+
%3 = integer_literal $Builtin.Word, 2
556+
%4 = alloc_ref [tail_elems $Kl * %3 : $Builtin.Word] $ArrayStorage
557+
%5 = ref_element_addr %4 : $ArrayStorage, #ArrayStorage.bodyField
558+
%11 = ref_tail_addr %4 : $ArrayStorage, $Kl
559+
store %0 to %5 : $*Kl // Store non-trivial type to body of ArrayStorage
560+
%27 = integer_literal $Builtin.Word, 1
561+
%28 = index_addr %11 : $*Kl, %27 : $Builtin.Word
562+
retain_value %0 : $Kl
563+
store %0 to %28 : $*Kl
564+
set_deallocating %4 : $ArrayStorage
565+
%65 = address_to_pointer %11 : $*Kl to $Builtin.RawPointer
566+
%66 = metatype $@thick Kl.Type
567+
%67 = builtin "destroyArray"<Kl>(%66 : $@thick Kl.Type, %65 : $Builtin.RawPointer, %3 : $Builtin.Word) : $()
568+
dealloc_ref %4 : $ArrayStorage
569+
%10 = tuple ()
570+
return %10 : $()
571+
}
572+
573+
// CHECK-LABEL: sil @dont_remove_dead_array_out_of_bounds_store
574+
// CHECK: alloc_ref
575+
// CHECK: dealloc_ref
576+
// CHECK: } // end sil function 'dont_remove_dead_array_out_of_bounds_store'
577+
sil @dont_remove_dead_array_out_of_bounds_store: $@convention(thin) (@owned Kl) -> () {
578+
bb0(%0 : $Kl):
579+
%2 = integer_literal $Builtin.Word, 1
580+
%3 = integer_literal $Builtin.Word, 2
581+
%4 = alloc_ref [tail_elems $Kl * %3 : $Builtin.Word] $ArrayStorage
582+
%11 = ref_tail_addr %4 : $ArrayStorage, $Kl
583+
store %0 to %11 : $*Kl
584+
%28 = index_addr %11 : $*Kl, %2 : $Builtin.Word // out-of-bounds store
585+
retain_value %0 : $Kl
586+
store %0 to %28 : $*Kl
587+
set_deallocating %4 : $ArrayStorage
588+
%65 = address_to_pointer %11 : $*Kl to $Builtin.RawPointer
589+
%66 = metatype $@thick Kl.Type
590+
%67 = builtin "destroyArray"<Kl>(%66 : $@thick Kl.Type, %65 : $Builtin.RawPointer, %2 : $Builtin.Word) : $()
591+
dealloc_ref %4 : $ArrayStorage
592+
%10 = tuple ()
593+
return %10 : $()
594+
}
595+
596+
// CHECK-LABEL: sil @dont_remove_dead_array_unknown_bounds
597+
// CHECK: alloc_ref
598+
// CHECK: dealloc_ref
599+
// CHECK: } // end sil function 'dont_remove_dead_array_unknown_bounds'
600+
sil @dont_remove_dead_array_unknown_bounds : $@convention(thin) (@owned Kl, Builtin.Word) -> () {
601+
bb0(%0 : $Kl, %1 : $Builtin.Word):
602+
%3 = integer_literal $Builtin.Word, 2
603+
%4 = alloc_ref [tail_elems $Kl * %3 : $Builtin.Word] $ArrayStorage
604+
%11 = ref_tail_addr %4 : $ArrayStorage, $Kl
605+
store %0 to %11 : $*Kl
606+
%27 = integer_literal $Builtin.Word, 1
607+
%28 = index_addr %11 : $*Kl, %27 : $Builtin.Word
608+
retain_value %0 : $Kl
609+
store %0 to %28 : $*Kl
610+
set_deallocating %4 : $ArrayStorage
611+
%65 = address_to_pointer %11 : $*Kl to $Builtin.RawPointer
612+
%66 = metatype $@thick Kl.Type
613+
%67 = builtin "destroyArray"<Kl>(%66 : $@thick Kl.Type, %65 : $Builtin.RawPointer, %1 : $Builtin.Word) : $()
614+
dealloc_ref %4 : $ArrayStorage
615+
%10 = tuple ()
616+
return %10 : $()
617+
}
618+
619+
// CHECK-LABEL: sil @dont_remove_dead_array_wrong_blocks
620+
// CHECK: alloc_ref
621+
// CHECK: dealloc_ref
622+
// CHECK: } // end sil function 'dont_remove_dead_array_wrong_blocks'
623+
sil @dont_remove_dead_array_wrong_blocks : $@convention(thin) (@owned Kl) -> () {
624+
bb0(%0 : $Kl):
625+
%3 = integer_literal $Builtin.Word, 2
626+
%4 = alloc_ref [tail_elems $Kl * %3 : $Builtin.Word] $ArrayStorage
627+
%11 = ref_tail_addr %4 : $ArrayStorage, $Kl
628+
cond_br undef, bb1, bb2
629+
bb1:
630+
store %0 to %11 : $*Kl
631+
br bb3
632+
bb2:
633+
%27 = integer_literal $Builtin.Word, 1
634+
%28 = index_addr %11 : $*Kl, %27 : $Builtin.Word
635+
store %0 to %28 : $*Kl
636+
br bb3
637+
bb3:
638+
set_deallocating %4 : $ArrayStorage
639+
%65 = address_to_pointer %11 : $*Kl to $Builtin.RawPointer
640+
%66 = metatype $@thick Kl.Type
641+
%67 = builtin "destroyArray"<Kl>(%66 : $@thick Kl.Type, %65 : $Builtin.RawPointer, %3 : $Builtin.Word) : $()
642+
dealloc_ref %4 : $ArrayStorage
643+
%10 = tuple ()
644+
return %10 : $()
645+
}
646+

0 commit comments

Comments
 (0)