Merge pull request #83995 from nate-chandler/cherrypick/release/6.2.1/rdar158149082

6.2.1: [AllocBoxToStack] Don't destroy in dead-ends.
This commit is contained in:
nate-chandler
2025-08-29 16:35:54 -07:00
committed by GitHub
7 changed files with 473 additions and 25 deletions

View File

@@ -1676,7 +1676,8 @@ public:
}
/// Verifies the lifetime of memory locations in the function.
void verifyMemoryLifetime(CalleeCache *calleeCache);
void verifyMemoryLifetime(CalleeCache *calleeCache,
DeadEndBlocks *deadEndBlocks);
/// Verifies ownership of the function.
/// Since we don't have complete lifetimes everywhere, computes DeadEndBlocks

View File

@@ -12,13 +12,14 @@
#define DEBUG_TYPE "sil-memory-lifetime-verifier"
#include "swift/Basic/Assertions.h"
#include "swift/SIL/MemoryLocations.h"
#include "swift/SIL/BitDataflow.h"
#include "swift/SIL/CalleeCache.h"
#include "swift/SIL/SILBasicBlock.h"
#include "swift/SIL/SILFunction.h"
#include "swift/SIL/ApplySite.h"
#include "swift/SIL/BasicBlockDatastructures.h"
#include "swift/SIL/BasicBlockUtils.h"
#include "swift/SIL/BitDataflow.h"
#include "swift/SIL/CalleeCache.h"
#include "swift/SIL/MemoryLocations.h"
#include "swift/SIL/SILBasicBlock.h"
#include "swift/SIL/SILFunction.h"
#include "llvm/Support/CommandLine.h"
using namespace swift;
@@ -43,6 +44,7 @@ class MemoryLifetimeVerifier {
SILFunction *function;
CalleeCache *calleeCache;
DeadEndBlocks *deadEndBlocks;
MemoryLocations locations;
/// alloc_stack memory locations which are used for store_borrow.
@@ -140,11 +142,12 @@ class MemoryLifetimeVerifier {
}
public:
MemoryLifetimeVerifier(SILFunction *function, CalleeCache *calleeCache) :
function(function),
calleeCache(calleeCache),
locations(/*handleNonTrivialProjections*/ true,
/*handleTrivialLocations*/ true) {}
MemoryLifetimeVerifier(SILFunction *function, CalleeCache *calleeCache,
DeadEndBlocks *deadEndBlocks)
: function(function), calleeCache(calleeCache),
deadEndBlocks(deadEndBlocks),
locations(/*handleNonTrivialProjections*/ true,
/*handleTrivialLocations*/ true) {}
/// The main entry point to verify the lifetime of all memory locations in
/// the function.
@@ -883,7 +886,12 @@ void MemoryLifetimeVerifier::checkBlock(SILBasicBlock *block, Bits &bits) {
}
case SILInstructionKind::DeallocStackInst: {
SILValue opVal = cast<DeallocStackInst>(&I)->getOperand();
requireBitsClear(bits & nonTrivialLocations, opVal, &I);
if (!deadEndBlocks->isDeadEnd(I.getParent())) {
// TODO: rdar://159311784: Maybe at some point the invariant will be
// enforced that values stored into addresses
// don't leak in dead-ends.
requireBitsClear(bits & nonTrivialLocations, opVal, &I);
}
// Needed to clear any bits of trivial locations (which are not required
// to be zero).
locations.clearBits(bits, opVal);
@@ -973,7 +981,8 @@ void MemoryLifetimeVerifier::verify() {
} // anonymous namespace
void SILFunction::verifyMemoryLifetime(CalleeCache *calleeCache) {
MemoryLifetimeVerifier verifier(this, calleeCache);
void SILFunction::verifyMemoryLifetime(CalleeCache *calleeCache,
DeadEndBlocks *deadEndBlocks) {
MemoryLifetimeVerifier verifier(this, calleeCache, deadEndBlocks);
verifier.verify();
}

View File

@@ -7380,7 +7380,7 @@ public:
if (F->hasOwnership() && F->shouldVerifyOwnership() &&
!mod.getASTContext().hadError()) {
F->verifyMemoryLifetime(calleeCache);
F->verifyMemoryLifetime(calleeCache, &getDeadEndBlocks());
}
}

View File

@@ -25,6 +25,8 @@
#include "swift/SIL/SILArgument.h"
#include "swift/SIL/SILBuilder.h"
#include "swift/SIL/SILCloner.h"
#include "swift/SILOptimizer/Analysis/DeadEndBlocksAnalysis.h"
#include "swift/SILOptimizer/Analysis/LoopAnalysis.h"
#include "swift/SILOptimizer/PassManager/Passes.h"
#include "swift/SILOptimizer/PassManager/Transforms.h"
#include "swift/SILOptimizer/Utils/InstOptUtils.h"
@@ -601,7 +603,9 @@ static void hoistMarkUnresolvedNonCopyableValueInsts(
/// rewriteAllocBoxAsAllocStack - Replace uses of the alloc_box with a
/// new alloc_stack, but do not delete the alloc_box yet.
static bool rewriteAllocBoxAsAllocStack(AllocBoxInst *ABI) {
static bool rewriteAllocBoxAsAllocStack(AllocBoxInst *ABI,
DeadEndBlocksAnalysis &deba,
SILLoopAnalysis &la) {
LLVM_DEBUG(llvm::dbgs() << "*** Promoting alloc_box to stack: " << *ABI);
SILValue HeapBox = ABI;
@@ -693,9 +697,31 @@ static bool rewriteAllocBoxAsAllocStack(AllocBoxInst *ABI) {
ABI->getBoxType(), ABI->getModule().Types, 0));
auto Loc = CleanupLocation(ABI->getLoc());
auto *deb = deba.get(ABI->getFunction());
for (auto LastRelease : FinalReleases) {
auto *dbi = dyn_cast<DeallocBoxInst>(LastRelease);
if (!dbi && deb->isDeadEnd(LastRelease->getParent()) &&
!la.get(ABI->getFunction())->getLoopFor(LastRelease->getParent())) {
// "Last" releases in dead-end regions may not actually destroy the box
// and consequently may not actually release the stored value. That's
// because values (including boxes) may be leaked along paths into
// dead-end regions. Thus it is invalid to lower such final releases of
// the box to destroy_addr's/dealloc_box's of the stack-promoted storage.
//
// There is one exception: if the alloc_box is in a dead-end loop. In
// that case SIL invariants require that the final releases actually
// destroy the box; otherwise, a box would leak once per loop. To check
// for this, it is sufficient check that the LastRelease is in a dead-end
// loop: if the alloc_box is not in that loop, then the entire loop is in
// the live range, so no release within the loop would be a "final
// release".
//
// None of this applies to dealloc_box instructions which always destroy
// the box.
continue;
}
SILBuilderWithScope Builder(LastRelease);
if (!isa<DeallocBoxInst>(LastRelease)&& !Lowering.isTrivial()) {
if (!dbi && !Lowering.isTrivial()) {
// If we have a mark_unresolved_non_copyable_value use of our stack box,
// we want to destroy that.
SILValue valueToDestroy = StackBox;
@@ -709,7 +735,6 @@ static bool rewriteAllocBoxAsAllocStack(AllocBoxInst *ABI) {
// instruction we found that isn't an explicit dealloc_box.
Builder.emitDestroyAddrAndFold(Loc, valueToDestroy);
}
auto *dbi = dyn_cast<DeallocBoxInst>(LastRelease);
if (dbi && dbi->isDeadEnd()) {
// Don't bother to create dealloc_stack instructions in dead-ends.
continue;
@@ -1265,7 +1290,9 @@ static void rewriteApplySites(AllocBoxToStackState &pass) {
/// Clone closure bodies and rewrite partial applies. Returns the number of
/// alloc_box allocations promoted.
static unsigned rewritePromotedBoxes(AllocBoxToStackState &pass) {
static unsigned rewritePromotedBoxes(AllocBoxToStackState &pass,
DeadEndBlocksAnalysis &deba,
SILLoopAnalysis &la) {
// First we'll rewrite any ApplySite that we can to remove
// the box container pointer from the operands.
rewriteApplySites(pass);
@@ -1274,7 +1301,7 @@ static unsigned rewritePromotedBoxes(AllocBoxToStackState &pass) {
auto rend = pass.Promotable.rend();
for (auto I = pass.Promotable.rbegin(); I != rend; ++I) {
auto *ABI = *I;
if (rewriteAllocBoxAsAllocStack(ABI)) {
if (rewriteAllocBoxAsAllocStack(ABI, deba, la)) {
++Count;
ABI->eraseFromParent();
}
@@ -1299,7 +1326,9 @@ class AllocBoxToStack : public SILFunctionTransform {
}
if (!pass.Promotable.empty()) {
auto Count = rewritePromotedBoxes(pass);
auto *deba = getAnalysis<DeadEndBlocksAnalysis>();
auto *la = getAnalysis<SILLoopAnalysis>();
auto Count = rewritePromotedBoxes(pass, *deba, *la);
NumStackPromoted += Count;
if (Count) {
if (StackNesting::fixNesting(getFunction()) == StackNesting::Changes::CFG)

View File

@@ -864,3 +864,11 @@ bb0:
%10 = tuple ()
return %10
}
sil [ossa] @storage_leaked_into_dead_ends : $@convention(thin) () -> () {
%t = apply undef() : $@convention(thin) () -> (@owned T)
%t_addr = alloc_stack $T
store %t to [init] %t_addr
dealloc_stack %t_addr
unreachable
}

View File

@@ -14,6 +14,8 @@ struct Bool {
protocol Error {}
class C {}
// CHECK-LABEL: sil @simple_promotion
sil @simple_promotion : $(Int) -> Int {
bb0(%0 : $Int):
@@ -901,7 +903,6 @@ bb2:
// CHECK: bb1:
// CHECK-NEXT: [[STACK2:%[0-9]+]] = alloc_stack $Bool
// CHECK-NEXT: dealloc_stack [[STACK2]]
// CHECK-NEXT: dealloc_stack [[BOX]]
// CHECK-NEXT: unreachable
// CHECK: bb2:
// CHECK: store {{%[0-9]+}}
@@ -1059,10 +1060,8 @@ bb3:
// CHECK-NEXT: [[AI:%[0-9]+]] = alloc_stack $Int
// CHECK-NEXT: cond_br
// CHECK: bb1:
// CHECK-NEXT: dealloc_stack [[AI]]
// CHECK-NEXT: br bb3
// CHECK: bb2:
// CHECK-NEXT: dealloc_stack [[AI]]
// CHECK-NEXT: br bb3
// CHECK: bb3:
// CHECK-NEXT: store {{%[0-9]+}}
@@ -1226,3 +1225,201 @@ bb0:
destroy_value %box : ${ var Builtin.Int32 }
return %value : $Builtin.Int32
}
sil @getC : $@convention(thin) () -> (@owned C)
// CHECK-LABEL: sil @leak_to_inf_loop_1 : {{.*}} {
// CHECK-NOT: destroy_addr
// CHECK-NOT: dealloc_stack
// CHECK-LABEL: } // end sil function 'leak_to_inf_loop_1'
sil @leak_to_inf_loop_1 : $@convention(thin) (@owned C) -> () {
entry(%c : $C):
%box = alloc_box ${ var C }
%c_addr = project_box %box, 0
store %c to %c_addr
strong_retain %box
strong_release %box
br loop
loop:
%getC = function_ref @getC : $@convention(thin) () -> (@owned C)
%c2 = apply %getC() : $@convention(thin) () -> (@owned C)
%c_old = load %c_addr
store %c2 to %c_addr
release_value %c_old
br loop
}
// CHECK-LABEL: sil @leak_to_inf_loop_2 : {{.*}} {
// CHECK: cond_br undef, [[EXIT:bb[0-9]+]], [[TO_LOOP:bb[0-9]+]]
// CHECK: [[EXIT]]
// CHECK: destroy_addr
// CHECK: dealloc_stack
// CHECK: [[TO_LOOP]]
// CHECK-NOT: destroy_addr
// CHECK-NOT: dealloc_stack
// CHECK-LABEL: } // end sil function 'leak_to_inf_loop_2'
sil @leak_to_inf_loop_2 : $@convention(thin) (@owned C) -> () {
entry(%c : $C):
%box = alloc_box ${ var C }
%c_addr = project_box %box, 0
store %c to %c_addr
strong_retain %box
strong_release %box
cond_br undef, exit, to_loop
exit:
strong_release %box
return undef : $()
to_loop:
strong_retain %box
strong_release %box
br loop
loop:
%getC = function_ref @getC : $@convention(thin) () -> (@owned C)
%c2 = apply %getC() : $@convention(thin) () -> (@owned C)
%c_old = load %c_addr
store %c2 to %c_addr
release_value %c_old
br loop
}
// CHECK-LABEL: sil @leak_to_inf_loop_3 : {{.*}} {
// CHECK-NOT: destroy_addr
// CHECK-NOT: dealloc_stack
// CHECK-LABEL: } // end sil function 'leak_to_inf_loop_3'
sil @leak_to_inf_loop_3 : $@convention(thin) (@owned C) -> () {
entry(%c : $C):
%box = alloc_box ${ var C }
%c_addr = project_box %box, 0
store %c to %c_addr
br to_loop
to_loop:
strong_retain %box
strong_release %box
br loop
loop:
%getC = function_ref @getC : $@convention(thin) () -> (@owned C)
%c2 = apply %getC() : $@convention(thin) () -> (@owned C)
%c_old = load %c_addr
store %c2 to %c_addr
release_value %c_old
br loop
}
// CHECK-LABEL: sil @dealloc_box_before_loop
// CHECK: dealloc_stack
// CHECK-LABEL: } // end sil function 'dealloc_box_before_loop'
sil @dealloc_box_before_loop : $@convention(thin) (@owned C) -> () {
entry(%c : $C):
%box = alloc_box ${ var C }
%c_addr = project_box %box, 0
store %c to %c_addr
cond_br undef, exit, to_loop
exit:
strong_release %box
return undef : $()
to_loop:
dealloc_box %box
br loop
loop:
br loop
}
// CHECK-LABEL: sil @alloc_box_in_deadend_loop
// CHECK: alloc_stack
// CHECK: dealloc_stack
// CHECK-LABEL: } // end sil function 'alloc_box_in_deadend_loop'
sil @alloc_box_in_deadend_loop : $@convention(thin) () -> () {
entry:
br loop
loop:
%box = alloc_box ${ var C }
%c_addr = project_box %box, 0
%getC = function_ref @getC : $@convention(thin) () -> (@owned C)
%c = apply %getC() : $@convention(thin) () -> (@owned C)
store %c to %c_addr
strong_release %box
br loop
}
// CHECK-LABEL: sil @alloc_box_in_exiting_loop
// CHECK: br [[EXITING_LOOP:bb[0-9]+]]
// CHECK: [[EXITING_LOOP]]:
// CHECK: alloc_stack
// CHECK: cond_br undef, [[EXIT:bb[0-9]+]], [[LATCH:bb[0-9]+]]
// CHECK: [[LATCH]]:
// CHECK: cond_br undef, [[BACKEDGE:bb[0-9]+]], [[INFINITE_LOOP:bb[0-9]+]]
// CHECK: [[BACKEDGE]]:
// CHECK: dealloc_stack
// CHECK: br [[EXITING_LOOP]]
// CHECK: [[EXIT]]:
// CHECK: dealloc_stack
// CHECK: [[INFINITE_LOOP]]:
// CHECK-NOT: dealloc_stack
// CHECK-LABEL: } // end sil function 'alloc_box_in_exiting_loop'
sil @alloc_box_in_exiting_loop : $@convention(thin) () -> () {
entry:
br exiting_loop
exiting_loop:
%box = alloc_box ${ var C }
%c_addr = project_box %box, 0
cond_br undef, exit, latch
latch:
cond_br undef, backedge, infinite_loop
backedge:
strong_release %box
br exiting_loop
exit:
strong_release %box
return undef : $()
infinite_loop:
%getC = function_ref @getC : $@convention(thin) () -> (@owned C)
%c = apply %getC() : $@convention(thin) () -> (@owned C)
store %c to %c_addr
strong_retain %box
strong_release %box
br infinite_loop
}
// CHECK-LABEL: sil @alloc_box_in_deadend_loop_with_another_deadend
// CHECK: br [[LOOP:bb[0-9]+]]
// CHECK: [[LOOP]]:
// CHECK: alloc_stack
// CHECK: cond_br undef, [[BACKEDGE:bb[0-9]+]], [[DIE:bb[0-9]+]]
// CHECK: [[BACKEDGE]]:
// CHECK: dealloc_stack
// CHECK: br [[LOOP]]
// CHECK: [[DIE]]:
// CHECK-NOT: dealloc_stack
// CHECK-LABEL: } // end sil function 'alloc_box_in_deadend_loop_with_another_deadend'
sil @alloc_box_in_deadend_loop_with_another_deadend : $@convention(thin) () -> () {
entry:
br loop
loop:
%box = alloc_box ${ var C }
cond_br undef, backedge, die
backedge:
strong_release %box
br loop
die:
strong_retain %box
strong_release %box
unreachable
}

View File

@@ -14,6 +14,8 @@ struct Bool {
protocol Error {}
class C {}
// CHECK-LABEL: sil [ossa] @simple_promotion
sil [ossa] @simple_promotion : $@convention(thin) (Int) -> Int {
bb0(%0 : $Int):
@@ -1006,7 +1008,6 @@ bb2:
// CHECK: bb1:
// CHECK-NEXT: [[STACK2:%[0-9]+]] = alloc_stack $Bool
// CHECK-NEXT: dealloc_stack [[STACK2]]
// CHECK-NEXT: dealloc_stack [[BOX]]
// CHECK-NEXT: unreachable
// CHECK: bb2:
// CHECK: store
@@ -1203,3 +1204,206 @@ bb0(%0 : $*T, %1 : $*T, %2 : $*T):
%15 = tuple ()
return %15 : $()
}
sil @getC : $@convention(thin) () -> (@owned C)
// CHECK-LABEL: sil [ossa] @leak_to_inf_loop_1 : {{.*}} {
// CHECK-NOT: destroy_addr
// CHECK-NOT: dealloc_stack
// CHECK-LABEL: } // end sil function 'leak_to_inf_loop_1'
sil [ossa] @leak_to_inf_loop_1 : $@convention(thin) (@owned C) -> () {
entry(%c : @owned $C):
%box = alloc_box ${ var C }
%c_addr = project_box %box, 0
store %c to [init] %c_addr
%copy = copy_value %box
destroy_value %box
br loop
loop:
%getC = function_ref @getC : $@convention(thin) () -> (@owned C)
%c2 = apply %getC() : $@convention(thin) () -> (@owned C)
%c_old = load [take] %c_addr
store %c2 to [init] %c_addr
destroy_value %c_old
br loop
}
// CHECK-LABEL: sil [ossa] @leak_to_inf_loop_2 : {{.*}} {
// CHECK: cond_br undef, [[EXIT:bb[0-9]+]], [[TO_LOOP:bb[0-9]+]]
// CHECK: [[EXIT]]
// CHECK: destroy_addr
// CHECK: dealloc_stack
// CHECK: [[TO_LOOP]]
// CHECK-NOT: destroy_addr
// CHECK-NOT: dealloc_stack
// CHECK-LABEL: } // end sil function 'leak_to_inf_loop_2'
sil [ossa] @leak_to_inf_loop_2 : $@convention(thin) (@owned C) -> () {
entry(%c : @owned $C):
%box = alloc_box ${ var C }
%c_addr = project_box %box, 0
store %c to [init] %c_addr
%copy = copy_value %box
destroy_value %box
cond_br undef, exit, to_loop
exit:
destroy_value %copy
return undef : $()
to_loop:
%copy2 = copy_value %copy
destroy_value %copy
br loop
loop:
%getC = function_ref @getC : $@convention(thin) () -> (@owned C)
%c2 = apply %getC() : $@convention(thin) () -> (@owned C)
%c_old = load [take] %c_addr
store %c2 to [init] %c_addr
destroy_value %c_old
br loop
}
// CHECK-LABEL: sil [ossa] @leak_to_inf_loop_3 : {{.*}} {
// CHECK-NOT: destroy_addr
// CHECK-NOT: dealloc_stack
// CHECK-LABEL: } // end sil function 'leak_to_inf_loop_3'
sil [ossa] @leak_to_inf_loop_3 : $@convention(thin) (@owned C) -> () {
entry(%c : @owned $C):
%box = alloc_box ${ var C }
%c_addr = project_box %box, 0
store %c to [init] %c_addr
br to_loop
to_loop:
%copy = copy_value %box
destroy_value %box
br loop
loop:
%getC = function_ref @getC : $@convention(thin) () -> (@owned C)
%c2 = apply %getC() : $@convention(thin) () -> (@owned C)
%c_old = load [take] %c_addr
store %c2 to [init] %c_addr
destroy_value %c_old
br loop
}
// CHECK-LABEL: sil [ossa] @dealloc_box_before_loop
// CHECK: dealloc_stack
// CHECK-LABEL: } // end sil function 'dealloc_box_before_loop'
sil [ossa] @dealloc_box_before_loop : $@convention(thin) (@owned C) -> () {
entry(%c : @owned $C):
%box = alloc_box ${ var C }
%c_addr = project_box %box, 0
store %c to [init] %c_addr
cond_br undef, exit, to_loop
exit:
destroy_value %box
return undef : $()
to_loop:
dealloc_box %box
br loop
loop:
br loop
}
// CHECK-LABEL: sil [ossa] @alloc_box_in_deadend_loop
// CHECK: alloc_stack
// CHECK: dealloc_stack
// CHECK-LABEL: } // end sil function 'alloc_box_in_deadend_loop'
sil [ossa] @alloc_box_in_deadend_loop : $@convention(thin) () -> () {
entry:
br loop
loop:
%box = alloc_box ${ var C }
%c_addr = project_box %box, 0
%getC = function_ref @getC : $@convention(thin) () -> (@owned C)
%c = apply %getC() : $@convention(thin) () -> (@owned C)
store %c to [init] %c_addr
destroy_value %box
br loop
}
// CHECK-LABEL: sil [ossa] @alloc_box_in_exiting_loop
// CHECK: br [[EXITING_LOOP:bb[0-9]+]]
// CHECK: [[EXITING_LOOP]]:
// CHECK: alloc_stack
// CHECK: cond_br undef, [[EXIT:bb[0-9]+]], [[LATCH:bb[0-9]+]]
// CHECK: [[LATCH]]:
// CHECK: cond_br undef, [[BACKEDGE:bb[0-9]+]], [[INFINITE_LOOP:bb[0-9]+]]
// CHECK: [[BACKEDGE]]:
// CHECK: dealloc_stack
// CHECK: br [[EXITING_LOOP]]
// CHECK: [[EXIT]]:
// CHECK: dealloc_stack
// CHECK: [[INFINITE_LOOP]]:
// CHECK-NOT: dealloc_stack
// CHECK-LABEL: } // end sil function 'alloc_box_in_exiting_loop'
sil [ossa] @alloc_box_in_exiting_loop : $@convention(thin) () -> () {
entry:
br exiting_loop
exiting_loop:
%box = alloc_box ${ var C }
%c_addr = project_box %box, 0
%getC = function_ref @getC : $@convention(thin) () -> (@owned C)
%c = apply %getC() : $@convention(thin) () -> (@owned C)
store %c to [init] %c_addr
cond_br undef, exit, latch
latch:
cond_br undef, backedge, to_infinite_loop
backedge:
destroy_value %box
br exiting_loop
exit:
destroy_value %box
return undef : $()
to_infinite_loop:
br infinite_loop
infinite_loop:
%c2 = apply %getC() : $@convention(thin) () -> (@owned C)
store %c2 to [assign] %c_addr
%copy = copy_value %box
destroy_value %copy
br infinite_loop
}
// CHECK-LABEL: sil [ossa] @alloc_box_in_deadend_loop_with_another_deadend
// CHECK: br [[LOOP:bb[0-9]+]]
// CHECK: [[LOOP]]:
// CHECK: alloc_stack
// CHECK: cond_br undef, [[BACKEDGE:bb[0-9]+]], [[DIE:bb[0-9]+]]
// CHECK: [[BACKEDGE]]:
// CHECK: dealloc_stack
// CHECK: br [[LOOP]]
// CHECK: [[DIE]]:
// CHECK-NOT: dealloc_stack
// CHECK-LABEL: } // end sil function 'alloc_box_in_deadend_loop_with_another_deadend'
sil [ossa] @alloc_box_in_deadend_loop_with_another_deadend : $@convention(thin) () -> () {
entry:
br loop
loop:
%box = alloc_box ${ var C }
cond_br undef, backedge, die
backedge:
destroy_value %box
br loop
die:
%copy = copy_value %box
destroy_value %copy
unreachable
}