Skip to content

Commit bc16701

Browse files
Merge pull request swiftlang#78319 from nate-chandler/cherrypick/release/6.1/rdar141560546
6.1: [DCE] Don't complete lifetimes of erased values.
2 parents 7b4ba26 + 9bc4a9b commit bc16701

File tree

2 files changed

+78
-22
lines changed

2 files changed

+78
-22
lines changed

lib/SILOptimizer/Transforms/DeadCodeElimination.cpp

Lines changed: 41 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -12,19 +12,20 @@
1212

1313
#define DEBUG_TYPE "sil-dce"
1414
#include "swift/Basic/Assertions.h"
15+
#include "swift/Basic/BlotSetVector.h"
1516
#include "swift/SIL/BasicBlockBits.h"
1617
#include "swift/SIL/DebugUtils.h"
1718
#include "swift/SIL/MemAccessUtils.h"
1819
#include "swift/SIL/NodeBits.h"
20+
#include "swift/SIL/OSSALifetimeCompletion.h"
1921
#include "swift/SIL/OwnershipUtils.h"
2022
#include "swift/SIL/SILArgument.h"
2123
#include "swift/SIL/SILBasicBlock.h"
2224
#include "swift/SIL/SILBuilder.h"
2325
#include "swift/SIL/SILFunction.h"
2426
#include "swift/SIL/SILUndef.h"
25-
#include "swift/SIL/OSSALifetimeCompletion.h"
26-
#include "swift/SILOptimizer/Analysis/DominanceAnalysis.h"
2727
#include "swift/SILOptimizer/Analysis/DeadEndBlocksAnalysis.h"
28+
#include "swift/SILOptimizer/Analysis/DominanceAnalysis.h"
2829
#include "swift/SILOptimizer/PassManager/Passes.h"
2930
#include "swift/SILOptimizer/PassManager/Transforms.h"
3031
#include "swift/SILOptimizer/Utils/BasicBlockOptUtils.h"
@@ -94,7 +95,7 @@ static bool seemsUseful(SILInstruction *I) {
9495
if (isa<DebugValueInst>(I))
9596
return isa<SILFunctionArgument>(I->getOperand(0))
9697
|| isa<SILUndef>(I->getOperand(0));
97-
98+
9899

99100
// Don't delete allocation instructions in DCE.
100101
if (isa<AllocRefInst>(I) || isa<AllocRefDynamicInst>(I)) {
@@ -131,7 +132,7 @@ class DCE {
131132
DominanceInfo *DT;
132133
DeadEndBlocks *deadEndBlocks;
133134
llvm::DenseMap<SILBasicBlock *, ControllingInfo> ControllingInfoMap;
134-
llvm::SmallVector<SILValue> valuesToComplete;
135+
SmallBlotSetVector<SILValue, 8> valuesToComplete;
135136

136137
// Maps instructions which produce a failing condition (like overflow
137138
// builtins) to the actual cond_fail instructions which handle the failure.
@@ -212,7 +213,7 @@ class DCE {
212213
markLive();
213214
return removeDead();
214215
}
215-
216+
216217
bool mustInvalidateCalls() const { return CallsChanged; }
217218
bool mustInvalidateBranches() const { return BranchesChanged; }
218219
};
@@ -637,7 +638,7 @@ void DCE::endLifetimeOfLiveValue(Operand *op, SILInstruction *insertPt) {
637638
// If DCE is going to delete the block in which we have to insert a
638639
// compensating lifetime end, let complete lifetimes utility handle it.
639640
if (!LiveBlocks.contains(insertPt->getParent())) {
640-
valuesToComplete.push_back(lookThroughBorrowedFromDef(value));
641+
valuesToComplete.insert(lookThroughBorrowedFromDef(value));
641642
return;
642643
}
643644

@@ -728,9 +729,23 @@ bool DCE::removeDead() {
728729
endLifetimeOfLiveValue(predOp, insertPt);
729730
}
730731
}
731-
erasePhiArgument(&BB, i, /*cleanupDeadPhiOps=*/true,
732-
InstModCallbacks().onCreateNewInst(
733-
[&](auto *inst) { markInstructionLive(inst); }));
732+
erasePhiArgument(
733+
&BB, i, /*cleanupDeadPhiOps=*/true,
734+
InstModCallbacks()
735+
.onCreateNewInst([&](auto *inst) { markInstructionLive(inst); })
736+
.onDelete([&](auto *inst) {
737+
for (auto result : inst->getResults()) {
738+
result = lookThroughBorrowedFromDef(result);
739+
if (valuesToComplete.count(result)) {
740+
valuesToComplete.erase(result);
741+
}
742+
}
743+
inst->replaceAllUsesOfAllResultsWithUndef();
744+
if (isa<ApplyInst>(inst))
745+
CallsChanged = true;
746+
++NumDeletedInsts;
747+
inst->eraseFromParent();
748+
}));
734749
Changed = true;
735750
BranchesChanged = true;
736751
}
@@ -744,7 +759,8 @@ bool DCE::removeDead() {
744759
// We want to replace dead terminators with unconditional branches to
745760
// the nearest post-dominator that has useful instructions.
746761
if (auto *termInst = dyn_cast<TermInst>(Inst)) {
747-
SILBasicBlock *postDom = nearestUsefulPostDominator(Inst->getParent());
762+
SILBasicBlock *postDom =
763+
nearestUsefulPostDominator(termInst->getParent());
748764
if (!postDom)
749765
continue;
750766

@@ -754,12 +770,13 @@ bool DCE::removeDead() {
754770
}
755771
}
756772
LLVM_DEBUG(llvm::dbgs() << "Replacing branch: ");
757-
LLVM_DEBUG(Inst->dump());
773+
LLVM_DEBUG(termInst->dump());
758774
LLVM_DEBUG(llvm::dbgs()
759775
<< "with jump to: BB" << postDom->getDebugID() << "\n");
760776

761-
replaceBranchWithJump(Inst, postDom);
762-
Inst->eraseFromParent();
777+
replaceBranchWithJump(termInst, postDom);
778+
++NumDeletedInsts;
779+
termInst->eraseFromParent();
763780
BranchesChanged = true;
764781
Changed = true;
765782
continue;
@@ -777,6 +794,12 @@ bool DCE::removeDead() {
777794
}
778795
}
779796
}
797+
for (auto result : Inst->getResults()) {
798+
result = lookThroughBorrowedFromDef(result);
799+
if (valuesToComplete.count(result)) {
800+
valuesToComplete.erase(result);
801+
}
802+
}
780803
Inst->replaceAllUsesOfAllResultsWithUndef();
781804

782805
if (isa<ApplyInst>(Inst))
@@ -789,7 +812,9 @@ bool DCE::removeDead() {
789812

790813
OSSALifetimeCompletion completion(F, DT, *deadEndBlocks);
791814
for (auto value : valuesToComplete) {
792-
completion.completeOSSALifetime(value,
815+
if (!value.has_value())
816+
continue;
817+
completion.completeOSSALifetime(*value,
793818
OSSALifetimeCompletion::Boundary::Liveness);
794819
}
795820

@@ -846,9 +871,9 @@ void DCE::computeLevelNumbers(PostDomTreeNode *root) {
846871
auto entry = workList.pop_back_val();
847872
PostDomTreeNode *node = entry.first;
848873
unsigned level = entry.second;
849-
874+
850875
insertControllingInfo(node->getBlock(), level);
851-
876+
852877
for (PostDomTreeNode *child : *node) {
853878
workList.push_back({child, level + 1});
854879
}

test/SILOptimizer/dead_code_elimination_nontrivial_ossa.sil

Lines changed: 37 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -514,7 +514,7 @@ bb1(%newborrow : @guaranteed $Klass, %borrow2 : @guaranteed $Klass):
514514
// Here %newborrowo and %newborrowi are both dead phis.
515515
// First end_borrow for the incoming value of %newborrowi is added
516516
// It is non straight forward to find the insert pt for the end_borrow of the incoming value of %newborrowo
517-
// This may not be important once CanonicalizeOSSALifetime supports rewrite of multi-block borrows.
517+
// This may not be important once CanonicalizeOSSALifetime supports rewrite of multi-block borrows.
518518
sil [ossa] @dce_nestedborrowlifetime3 : $@convention(thin) (@guaranteed Klass) -> () {
519519
bb0(%0 : @guaranteed $Klass):
520520
%borrowo = begin_borrow %0 : $Klass
@@ -752,7 +752,7 @@ exit:
752752
sil hidden [ossa] @add_end_borrow_after_destroy_value : $@convention(thin) (Builtin.Int1, @owned Klass, @owned Klass) -> () {
753753
bb0(%condition : $Builtin.Int1, %instance_1 : @owned $Klass, %instance_2 : @owned $Klass):
754754
%lifetime_1 = begin_borrow %instance_1 : $Klass
755-
755+
756756
%copy_1 = copy_value %lifetime_1 : $Klass
757757
%stack_addr = alloc_stack $Klass
758758
store %copy_1 to [init] %stack_addr : $*Klass
@@ -1124,7 +1124,7 @@ sil @use_klass : $@convention(thin) (@guaranteed Klass) -> ()
11241124
sil [ossa] @test_forwarded_phi1 : $@convention(thin) (@owned Wrapper1) -> () {
11251125
bb0(%0 : @owned $Wrapper1):
11261126
%outer = begin_borrow %0 : $Wrapper1
1127-
br bb1
1127+
br bb1
11281128

11291129
bb1:
11301130
%ex1 = struct_extract %outer : $Wrapper1, #Wrapper1.val1
@@ -1144,13 +1144,13 @@ bb3(%phi3 : @guaranteed $Klass, %phi4 : @guaranteed $Wrapper1):
11441144
}
11451145

11461146
// CHECK-LABEL: sil [ossa] @test_forwarded_phi2 :
1147-
// CHECK: br bb3
1147+
// CHECK: br bb3
11481148
// CHECK: end_borrow
11491149
// CHECK-LABEL: } // end sil function 'test_forwarded_phi2'
11501150
sil [ossa] @test_forwarded_phi2 : $@convention(thin) (@owned Wrapper1) -> () {
11511151
bb0(%0 : @owned $Wrapper1):
11521152
%outer = begin_borrow %0 : $Wrapper1
1153-
br bb1
1153+
br bb1
11541154

11551155
bb1:
11561156
br bb2(%outer : $Wrapper1)
@@ -1202,7 +1202,7 @@ bb3(%phi2 : @guaranteed $Klass, %phi3 : @guaranteed $Wrapper1):
12021202
sil [ossa] @test_loadborrow_dep : $@convention(thin) (@in Wrapper1) -> () {
12031203
bb0(%0 : $*Wrapper1):
12041204
%outer = load_borrow %0 : $*Wrapper1
1205-
br bb1
1205+
br bb1
12061206

12071207
bb1:
12081208
%ex1 = struct_extract %outer : $Wrapper1, #Wrapper1.val1
@@ -1412,3 +1412,34 @@ bb5(%16 : @reborrow $FakeOptional<Klass>, %17 : @reborrow $FakeOptional<Klass>):
14121412
return %23 : $()
14131413
}
14141414

1415+
struct String {
1416+
var guts: Builtin.AnyObject
1417+
}
1418+
1419+
sil [_semantics "string.makeUTF8"] [ossa] @makeString : $@convention(thin) (Builtin.RawPointer, Builtin.Word) -> @owned String
1420+
1421+
// CHECK-LABEL: sil [ossa] @uncompletedDeadStrings
1422+
// CHECK-NOT: apply
1423+
// CHECK-LABEL: } // end sil function 'uncompletedDeadStrings'
1424+
sil [ossa] @uncompletedDeadStrings : $@convention(thin) () -> () {
1425+
%first_ptr = string_literal utf8 "first"
1426+
%first_len = integer_literal $Builtin.Word, 5
1427+
%makeString = function_ref @makeString : $@convention(thin) (Builtin.RawPointer, Builtin.Word) -> @owned String
1428+
%first = apply %makeString(%first_ptr, %first_len) : $@convention(thin) (Builtin.RawPointer, Builtin.Word) -> @owned String
1429+
cond_br undef, nope, yep
1430+
1431+
nope:
1432+
destroy_value %first : $String
1433+
%second_ptr = string_literal utf8 "second"
1434+
%second_len = integer_literal $Builtin.Word, 6
1435+
%second = apply %makeString(%second_ptr, %second_len) : $@convention(thin) (Builtin.RawPointer, Builtin.Word) -> @owned String
1436+
br this(%second : $String)
1437+
1438+
yep:
1439+
br this(%first : $String)
1440+
1441+
this(%string : @owned $String):
1442+
destroy_value %string : $String
1443+
%retval = tuple ()
1444+
return %retval : $()
1445+
}

0 commit comments

Comments
 (0)