Skip to content

Commit cd66ca7

Browse files
meg-guptameg-gupta
committed
Update isGuaranteedForwarding api and delete isGuaranteedForwardingPhi api
1 parent 2e0cc04 commit cd66ca7

File tree

9 files changed

+57
-119
lines changed

9 files changed

+57
-119
lines changed

‎include/swift/SIL/OwnershipUtils.h

Lines changed: 18 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -86,33 +86,30 @@ bool canOpcodeForwardGuaranteedValues(Operand *use);
8686
// This is the use-def equivalent of use->getOperandOwnership() ==
8787
// OperandOwnership::GuaranteedForwarding.
8888
inline bool isGuaranteedForwarding(SILValue value) {
89-
assert(value->getOwnershipKind() == OwnershipKind::Guaranteed);
90-
return canOpcodeForwardGuaranteedValues(value);
91-
}
92-
93-
/// Returns true if it is a forwarding phi
94-
inline bool isGuaranteedForwardingPhi(SILValue value) {
9589
if (value->getOwnershipKind() != OwnershipKind::Guaranteed) {
9690
return false;
9791
}
98-
if (isa<BeginBorrowInst>(value) || isa<LoadBorrowInst>(value)) {
99-
return false;
92+
if (canOpcodeForwardGuaranteedValues(value) ||
93+
isa<SILFunctionArgument>(value)) {
94+
return true;
10095
}
10196
auto *phi = dyn_cast<SILPhiArgument>(value);
10297
if (!phi || !phi->isPhi()) {
103-
return true;
98+
return false;
10499
}
105-
bool isGuaranteedForwardingPhi = true;
106-
phi->visitTransitiveIncomingPhiOperands(
107-
[&](auto *phi, auto *operand) -> bool {
108-
if (isa<BeginBorrowInst>(operand->get()) ||
109-
isa<LoadBorrowInst>(operand->get())) {
110-
isGuaranteedForwardingPhi = false;
111-
return false;
112-
}
113-
return true;
114-
});
115-
100+
bool isGuaranteedForwardingPhi = false;
101+
phi->visitTransitiveIncomingPhiOperands([&](auto *, auto *operand) -> bool {
102+
if (canOpcodeForwardGuaranteedValues(operand->get()) ||
103+
isa<SILFunctionArgument>(operand->get())) {
104+
isGuaranteedForwardingPhi = true;
105+
return false;
106+
}
107+
auto *phi = dyn_cast<SILPhiArgument>(operand->get());
108+
if (!phi || !phi->isPhi()) {
109+
return false;
110+
}
111+
return true;
112+
});
116113
return isGuaranteedForwardingPhi;
117114
}
118115

@@ -527,7 +524,7 @@ class BorrowedValueKind {
527524
})) {
528525
return Kind::Invalid;
529526
}
530-
if (isGuaranteedForwardingPhi(value)) {
527+
if (isGuaranteedForwarding(value)) {
531528
return Kind::Invalid;
532529
}
533530
return Kind::Phi;

‎lib/SIL/IR/OperandOwnership.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -430,7 +430,7 @@ OperandOwnership OperandOwnershipClassifier::visitBranchInst(BranchInst *bi) {
430430
bi->getDestBB()->getArgument(getOperandIndex())->getOwnershipKind();
431431

432432
if (destBlockArgOwnershipKind == OwnershipKind::Guaranteed) {
433-
return isGuaranteedForwardingPhi(getValue())
433+
return isGuaranteedForwarding(getValue())
434434
? OperandOwnership::GuaranteedForwardingPhi
435435
: OperandOwnership::Reborrow;
436436
}

‎lib/SIL/Utils/OwnershipUtils.cpp

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1220,11 +1220,12 @@ bool swift::getAllBorrowIntroducingValues(SILValue inputValue,
12201220
if (inputValue->getOwnershipKind() != OwnershipKind::Guaranteed)
12211221
return false;
12221222

1223-
SmallVector<SILValue, 32> worklist;
1224-
worklist.emplace_back(inputValue);
1223+
SmallSetVector<SILValue, 32> worklist;
1224+
worklist.insert(inputValue);
12251225

1226-
while (!worklist.empty()) {
1227-
SILValue value = worklist.pop_back_val();
1226+
// worklist grows in this loop.
1227+
for (unsigned idx = 0; idx < worklist.size(); idx++) {
1228+
SILValue value = worklist[idx];
12281229

12291230
// First check if v is an introducer. If so, stash it and continue.
12301231
if (auto scopeIntroducer = BorrowedValue(value)) {
@@ -1243,21 +1244,28 @@ bool swift::getAllBorrowIntroducingValues(SILValue inputValue,
12431244
// instruction
12441245
if (isGuaranteedForwarding(value)) {
12451246
if (auto *i = value->getDefiningInstruction()) {
1246-
llvm::copy(i->getNonTypeDependentOperandValues(),
1247-
std::back_inserter(worklist));
1247+
for (SILValue opValue : i->getNonTypeDependentOperandValues()) {
1248+
worklist.insert(opValue);
1249+
}
12481250
continue;
12491251
}
12501252

12511253
// Otherwise, we should have a block argument that is defined by a single
12521254
// predecessor terminator.
12531255
auto *arg = cast<SILPhiArgument>(value);
1254-
auto *termInst = arg->getSingleTerminator();
1255-
assert(termInst && termInst->isTransformationTerminator() &&
1256-
OwnershipForwardingMixin::get(termInst)->preservesOwnership());
1257-
assert(termInst->getNumOperands() == 1 &&
1258-
"Transforming terminators should always have a single operand");
1259-
worklist.push_back(termInst->getAllOperands()[0].get());
1260-
continue;
1256+
if (!arg->isPhi()) {
1257+
auto *termInst = arg->getSingleTerminator();
1258+
assert(termInst && termInst->isTransformationTerminator() &&
1259+
OwnershipForwardingMixin::get(termInst)->preservesOwnership());
1260+
assert(termInst->getNumOperands() == 1 &&
1261+
"Transforming terminators should always have a single operand");
1262+
worklist.insert(termInst->getAllOperands()[0].get());
1263+
continue;
1264+
}
1265+
arg->visitIncomingPhiOperands([&](auto *operand) {
1266+
worklist.insert(operand->get());
1267+
return true;
1268+
});
12611269
}
12621270

12631271
// Otherwise, this is an introducer we do not understand. Bail and return

‎lib/SIL/Verifier/SILOwnershipVerifier.cpp

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -280,7 +280,8 @@ bool SILValueOwnershipChecker::gatherUsers(
280280
// this value forwards guaranteed ownership. In such a case, we are going to
281281
// validate it as part of the borrow introducer from which the forwarding
282282
// value originates. So we can just return true and continue.
283-
if (isGuaranteedForwarding(value))
283+
if (value->getOwnershipKind() == OwnershipKind::Guaranteed &&
284+
canOpcodeForwardGuaranteedValues(value))
284285
return true;
285286

286287
// Ok, we have some sort of borrow introducer. We need to recursively validate
@@ -562,10 +563,6 @@ bool SILValueOwnershipChecker::checkValueWithoutLifetimeEndingUses(
562563
if (isGuaranteedForwarding(value)) {
563564
return true;
564565
}
565-
auto *phi = dyn_cast<SILPhiArgument>(value);
566-
if (phi && isGuaranteedForwardingPhi(phi)) {
567-
return true;
568-
}
569566
}
570567

571568
// If we have an unowned value, then again there is nothing left to do.
@@ -683,8 +680,7 @@ bool SILValueOwnershipChecker::checkUses() {
683680
// Check if we are an instruction that forwards guaranteed
684681
// ownership. In such a case, we are a subobject projection. We should not
685682
// have any lifetime ending uses.
686-
if (value->getOwnershipKind() == OwnershipKind::Guaranteed &&
687-
isGuaranteedForwarding(value)) {
683+
if (isGuaranteedForwarding(value)) {
688684
if (!isSubobjectProjectionWithLifetimeEndingUses(value,
689685
lifetimeEndingUsers)) {
690686
return false;

‎test/SIL/OwnershipVerifier/arguments.sil

Lines changed: 3 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -54,40 +54,13 @@ bb1(%2 : @guaranteed $Builtin.NativeObject):
5454

5555
// CHECK-LABEL: Error#: 0. Begin Error in Function: 'leak_along_path'
5656
// CHECK: Error! Found a leak due to a consuming post-dominance failure!
57-
// CHECK: Value: %2 = argument of bb1 : $Builtin.NativeObject
57+
// CHECK: Value: %3 = argument of bb1 : $Builtin.NativeObject
5858
// CHECK: Post Dominating Failure Blocks:
5959
// CHECK: bb3
6060
// CHECK: Error#: 0. End Error in Function: 'leak_along_path'
6161
//
6262
// CHECK-NOT: Error#: {{[0-9][0-9]*}}. End Error in Function: 'leak_along_path'
6363
sil [ossa] @leak_along_path : $@convention(thin) (@guaranteed Builtin.NativeObject) -> () {
64-
bb0(%0 : @guaranteed $Builtin.NativeObject):
65-
br bb1(%0 : $Builtin.NativeObject)
66-
67-
bb1(%1 : @guaranteed $Builtin.NativeObject):
68-
cond_br undef, bb2, bb3
69-
70-
bb2:
71-
end_borrow %1 : $Builtin.NativeObject
72-
br bb4
73-
74-
bb3:
75-
br bb4
76-
77-
bb4:
78-
%9999 = tuple()
79-
return %9999 : $()
80-
}
81-
82-
// CHECK-LABEL: Error#: 0. Begin Error in Function: 'leak_along_path_2'
83-
// CHECK: Error! Found a leak due to a consuming post-dominance failure!
84-
// CHECK: Value: %3 = argument of bb1 : $Builtin.NativeObject
85-
// CHECK: Post Dominating Failure Blocks:
86-
// CHECK: bb3
87-
// CHECK: Error#: 0. End Error in Function: 'leak_along_path_2'
88-
//
89-
// CHECK-NOT: Error#: {{[0-9][0-9]*}}. End Error in Function: 'leak_along_path_2'
90-
sil [ossa] @leak_along_path_2 : $@convention(thin) (@guaranteed Builtin.NativeObject) -> () {
9164
bb0(%0 : @guaranteed $Builtin.NativeObject):
9265
%1 = begin_borrow %0 : $Builtin.NativeObject
9366
br bb1(%1 : $Builtin.NativeObject)
@@ -109,53 +82,16 @@ bb4:
10982

11083
// Make sure that we only flag the subargument leak and not the parent
11184
// argument. Also check for over consuming due to phi nodes.
112-
//
85+
11386
// CHECK-LABEL: Error#: 0. Begin Error in Function: 'leak_along_subarg_path'
11487
// CHECK: Error! Found a leak due to a consuming post-dominance failure!
115-
// CHECK: Value: %6 = argument of bb4 : $Builtin.NativeObject
88+
// CHECK: Value: %7 = argument of bb3 : $Builtin.NativeObject
11689
// CHECK: Post Dominating Failure Blocks:
11790
// CHECK: bb6
11891
// CHECK: Error#: 0. End Error in Function: 'leak_along_subarg_path'
11992
//
12093
// CHECK-NOT: Error#: {{[0-9][0-9]*}}. End Error in Function: 'leak_along_subarg_path'
12194
sil [ossa] @leak_along_subarg_path : $@convention(thin) (@guaranteed Builtin.NativeObject) -> () {
122-
bb0(%0 : @guaranteed $Builtin.NativeObject):
123-
br bb1(%0 : $Builtin.NativeObject)
124-
125-
bb1(%1 : @guaranteed $Builtin.NativeObject):
126-
cond_br undef, bb2, bb3
127-
128-
bb2:
129-
br bb7
130-
131-
bb3:
132-
br bb4(%1 : $Builtin.NativeObject)
133-
134-
bb4(%2 : @guaranteed $Builtin.NativeObject):
135-
cond_br undef, bb5, bb6
136-
137-
bb5:
138-
end_borrow %2 : $Builtin.NativeObject
139-
br bb7
140-
141-
bb6:
142-
br bb7
143-
144-
bb7:
145-
end_borrow %1 : $Builtin.NativeObject
146-
%9999 = tuple()
147-
return %9999 : $()
148-
}
149-
150-
// CHECK-LABEL: Error#: 0. Begin Error in Function: 'leak_along_subarg_path_2'
151-
// CHECK: Error! Found a leak due to a consuming post-dominance failure!
152-
// CHECK: Value: %7 = argument of bb3 : $Builtin.NativeObject
153-
// CHECK: Post Dominating Failure Blocks:
154-
// CHECK: bb6
155-
// CHECK: Error#: 0. End Error in Function: 'leak_along_subarg_path_2'
156-
//
157-
// CHECK-NOT: Error#: {{[0-9][0-9]*}}. End Error in Function: 'leak_along_subarg_path_2'
158-
sil [ossa] @leak_along_subarg_path_2 : $@convention(thin) (@guaranteed Builtin.NativeObject) -> () {
15995
bb0(%0 : @guaranteed $Builtin.NativeObject):
16096
%1 = begin_borrow %0 : $Builtin.NativeObject
16197
br bb1(%1 : $Builtin.NativeObject)

‎test/SIL/OwnershipVerifier/guaranteed_phis_errors.sil

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,3 +192,4 @@ bb0:
192192
%r = tuple ()
193193
return %r : $()
194194
}
195+

‎test/SIL/OwnershipVerifier/use_verifier.sil

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1206,7 +1206,8 @@ bb1:
12061206

12071207
bb2:
12081208
%5 = struct_extract %1 : $NonTrivialStructWithTrivialField, #NonTrivialStructWithTrivialField.owner
1209-
br bb3(%5 : $Builtin.NativeObject)
1209+
%6 = begin_borrow %5 : $Builtin.NativeObject
1210+
br bb3(%6 : $Builtin.NativeObject)
12101211

12111212
bb3(%7 : @guaranteed $Builtin.NativeObject):
12121213
end_borrow %7 : $Builtin.NativeObject
@@ -1366,7 +1367,6 @@ bb2:
13661367
br bb3(%f2thick : $@callee_owned () -> ())
13671368

13681369
bb3(%fUnknown : @guaranteed $@callee_owned () -> ()):
1369-
end_borrow %fUnknown : $@callee_owned () -> ()
13701370
%9999 = tuple()
13711371
return %9999 : $()
13721372
}

‎test/SILOptimizer/redundant_phi_elimination_ossa.sil

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -183,8 +183,6 @@ bb2:
183183
br bb3(%0 : $FakeOptional<Klass>, %0 : $FakeOptional<Klass>)
184184

185185
bb3(%1 : @guaranteed $FakeOptional<Klass>, %2 : @guaranteed $FakeOptional<Klass>):
186-
end_borrow %1 : $FakeOptional<Klass>
187-
end_borrow %2 : $FakeOptional<Klass>
188186
%res = tuple ()
189187
return %res : $()
190188
}
@@ -195,17 +193,18 @@ bb3(%1 : @guaranteed $FakeOptional<Klass>, %2 : @guaranteed $FakeOptional<Klass>
195193
sil [ossa] @test_redundantguaranteedphiarg2 : $@convention(thin) () -> () {
196194
bb0:
197195
%0 = enum $FakeOptional<Klass>, #FakeOptional.none!enumelt
196+
%b = begin_borrow %0 : $FakeOptional<Klass>
198197
cond_br undef, bb1, bb2
199198

200199
bb1:
201-
br bb3(%0 : $FakeOptional<Klass>, %0 : $FakeOptional<Klass>)
200+
br bb3(%0 : $FakeOptional<Klass>, %b : $FakeOptional<Klass>)
202201

203202
bb2:
204-
br bb3(%0 : $FakeOptional<Klass>, %0 : $FakeOptional<Klass>)
203+
br bb3(%0 : $FakeOptional<Klass>, %b : $FakeOptional<Klass>)
205204

206205
bb3(%1 : @owned $FakeOptional<Klass>, %2 : @guaranteed $FakeOptional<Klass>):
207-
destroy_value %1 : $FakeOptional<Klass>
208206
end_borrow %2 : $FakeOptional<Klass>
207+
destroy_value %1 : $FakeOptional<Klass>
209208
%res = tuple ()
210209
return %res : $()
211210
}
@@ -216,13 +215,15 @@ bb3(%1 : @owned $FakeOptional<Klass>, %2 : @guaranteed $FakeOptional<Klass>):
216215
sil [ossa] @test_redundantguaranteedphiarg3 : $@convention(thin) () -> () {
217216
bb0:
218217
%0 = enum $FakeOptional<Klass>, #FakeOptional.none!enumelt
218+
%b1 = begin_borrow %0 : $FakeOptional<Klass>
219+
%b2 = begin_borrow %0 : $FakeOptional<Klass>
219220
cond_br undef, bb1, bb2
220221

221222
bb1:
222-
br bb3(%0 : $FakeOptional<Klass>, %0 : $FakeOptional<Klass>)
223+
br bb3(%b1 : $FakeOptional<Klass>, %b2 : $FakeOptional<Klass>)
223224

224225
bb2:
225-
br bb3(%0 : $FakeOptional<Klass>, %0 : $FakeOptional<Klass>)
226+
br bb3(%b1 : $FakeOptional<Klass>, %b2 : $FakeOptional<Klass>)
226227

227228
bb3(%1 : @guaranteed $FakeOptional<Klass>, %2 : @guaranteed $FakeOptional<Klass>):
228229
cond_br undef, bb4, bb5

‎test/SILOptimizer/simplify_cfg_checkcast.sil

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -383,7 +383,6 @@ bb10(%failBB7 : @guaranteed $Base):
383383
br bb11
384384

385385
bb11:
386-
end_borrow %joined : $Base
387386
%20 = tuple ()
388387
return %20 : $()
389388

0 commit comments

Comments
 (0)