Skip to content

Commit 7bf0010

Browse files
eecksteineeckstein
committed
SIL: remove computeIsReborrow
This API computes the re-borrow flags for guaranteed phis based on the presence of forwarding instructions in the incoming values. This is not correct in all cases, because optimizations can optimize away forwarding instructions. Fixes a verifier crash: rdar://139280579
1 parent 8462459 commit 7bf0010

File tree

12 files changed

+83
-143
lines changed

12 files changed

+83
-143
lines changed

‎include/swift/SIL/OwnershipUtils.h‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -57,14 +57,6 @@ bool canOpcodeForwardInnerGuaranteedValues(SILValue value);
5757
/// the operation may be trivially rewritten with Guaranteed ownership.
5858
bool canOpcodeForwardInnerGuaranteedValues(Operand *use);
5959

60-
bool computeIsScoped(SILArgument *arg);
61-
62-
bool computeIsReborrow(SILArgument *arg);
63-
64-
// This is the use-def equivalent of use->getOperandOwnership() ==
65-
// OperandOwnership::GuaranteedForwarding.
66-
bool computeIsGuaranteedForwarding(SILValue value);
67-
6860
/// Is the opcode that produces \p value capable of forwarding owned values?
6961
///
7062
/// This may be true even if the current instance of the instruction is not a

‎lib/SIL/Parser/ParseSIL.cpp‎

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -65,11 +65,6 @@ static llvm::cl::opt<bool>
6565
ParseIncompleteOSSA("parse-incomplete-ossa",
6666
llvm::cl::desc("Parse OSSA with incomplete lifetimes"));
6767

68-
static llvm::cl::opt<bool> DisablePopulateOwnershipFlags(
69-
"disable-populate-ownership-flags",
70-
llvm::cl::desc("Disable populating ownership flags"),
71-
llvm::cl::init(false));
72-
7368
//===----------------------------------------------------------------------===//
7469
// SILParserState implementation
7570
//===----------------------------------------------------------------------===//
@@ -7237,16 +7232,6 @@ bool SILParser::parseSILBasicBlock(SILBuilder &B) {
72377232
return false;
72387233
}
72397234

7240-
static void populateOwnershipFlags(SILFunction *func) {
7241-
for (auto &bb : *func) {
7242-
for (auto &arg : bb.getArguments()) {
7243-
if (computeIsReborrow(arg)) {
7244-
arg->setReborrow(true);
7245-
}
7246-
}
7247-
}
7248-
}
7249-
72507235
/// decl-sil: [[only in SIL mode]]
72517236
/// 'sil' sil-linkage '@' identifier ':' sil-type decl-sil-body?
72527237
/// decl-sil-body:
@@ -7426,13 +7411,6 @@ bool SILParserState::parseDeclSIL(Parser &P) {
74267411
return true;
74277412
} while (P.Tok.isNot(tok::r_brace) && P.Tok.isNot(tok::eof));
74287413

7429-
// OSSA uses flags to represent "reborrow", "escaping" etc.
7430-
// These flags are populated here as a shortcut to rewriting all
7431-
// existing OSSA tests with flags.
7432-
if (!DisablePopulateOwnershipFlags) {
7433-
populateOwnershipFlags(FunctionState.F);
7434-
}
7435-
74367414
SourceLoc RBraceLoc;
74377415
P.parseMatchingToken(tok::r_brace, RBraceLoc, diag::expected_sil_rbrace,
74387416
LBraceLoc);

‎lib/SIL/Utils/OwnershipUtils.cpp‎

Lines changed: 0 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -146,64 +146,6 @@ bool swift::canOpcodeForwardOwnedValues(Operand *use) {
146146
return false;
147147
}
148148

149-
bool swift::computeIsReborrow(SILArgument *arg) {
150-
if (arg->getOwnershipKind() != OwnershipKind::Guaranteed) {
151-
return false;
152-
}
153-
if (isa<SILFunctionArgument>(arg)) {
154-
return false;
155-
}
156-
return !computeIsGuaranteedForwarding(arg);
157-
}
158-
159-
bool swift::computeIsScoped(SILArgument *arg) {
160-
if (arg->getOwnershipKind() == OwnershipKind::Owned) {
161-
return true;
162-
}
163-
return computeIsReborrow(arg);
164-
}
165-
166-
// This is the use-def equivalent of use->getOperandOwnership() ==
167-
// OperandOwnership::GuaranteedForwarding.
168-
bool swift::computeIsGuaranteedForwarding(SILValue value) {
169-
if (value->getOwnershipKind() != OwnershipKind::Guaranteed) {
170-
return false;
171-
}
172-
value = lookThroughBorrowedFromDef(value);
173-
// NOTE: canOpcodeForwardInnerGuaranteedValues returns true for transformation
174-
// terminator results.
175-
if (canOpcodeForwardInnerGuaranteedValues(value) ||
176-
isa<SILFunctionArgument>(value)) {
177-
return true;
178-
}
179-
// If not a phi, return false
180-
auto *phi = dyn_cast<SILPhiArgument>(value);
181-
if (!phi || !phi->isPhi()) {
182-
return false;
183-
}
184-
// For a phi, if we find GuaranteedForwarding phi operand on any incoming
185-
// path, we return true. Additional verification is added to ensure
186-
// GuaranteedForwarding phi operands are found on zero or all paths in the
187-
// OwnershipVerifier.
188-
bool isGuaranteedForwardingPhi = false;
189-
phi->visitTransitiveIncomingPhiOperands([&](auto *, auto *op) -> bool {
190-
assert(op->get()->getOwnershipKind().isCompatibleWith(
191-
OwnershipKind::Guaranteed));
192-
auto opValue = lookThroughBorrowedFromDef(op->get());
193-
if (canOpcodeForwardInnerGuaranteedValues(opValue) ||
194-
isa<SILFunctionArgument>(opValue)) {
195-
isGuaranteedForwardingPhi = true;
196-
return false;
197-
}
198-
auto *phi = dyn_cast<SILPhiArgument>(opValue);
199-
if (!phi || !phi->isPhi()) {
200-
return false;
201-
}
202-
return true;
203-
});
204-
return isGuaranteedForwardingPhi;
205-
}
206-
207149
BorrowedFromInst *swift::getBorrowedFromUser(SILValue v) {
208150
for (auto *use : v->getUses()) {
209151
if (auto *bfi = dyn_cast<BorrowedFromInst>(use->getUser())) {

‎lib/SIL/Verifier/SILVerifier.cpp‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1277,11 +1277,6 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
12771277
CurArgument = arg;
12781278
checkLegalType(arg->getFunction(), arg, nullptr);
12791279

1280-
// Ensure flags on the argument are not stale.
1281-
require(!arg->getFunction()->hasOwnership() ||
1282-
computeIsReborrow(arg) == arg->isReborrow(),
1283-
"Stale reborrow flag");
1284-
12851280
if (checkLinearLifetime) {
12861281
checkValueBaseOwnership(arg);
12871282
}

‎lib/SILOptimizer/SemanticARC/OwnershipLiveRange.cpp‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,6 @@ static SILValue convertIntroducerToGuaranteed(OwnedValueIntroducer introducer) {
281281
case OwnedValueIntroducerKind::Phi: {
282282
auto *phiArg = cast<SILPhiArgument>(introducer.value);
283283
phiArg->setOwnershipKind(OwnershipKind::Guaranteed);
284-
phiArg->setReborrow(computeIsReborrow(phiArg));
285284
return phiArg;
286285
}
287286
case OwnedValueIntroducerKind::Struct: {

‎lib/SILOptimizer/Utils/SILSSAUpdater.cpp‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -247,8 +247,6 @@ SILValue SILSSAUpdater::getValueInMiddleOfBlock(SILBasicBlock *block) {
247247
addNewEdgeValueToBranch(pair.first->getTerminator(), block, pair.second,
248248
deleter);
249249
}
250-
// Set the reborrow flag on the newly created phi.
251-
phiArg->setReborrow(computeIsReborrow(phiArg));
252250

253251
if (insertedPhis)
254252
insertedPhis->push_back(phiArg);
@@ -359,9 +357,6 @@ class SSAUpdaterTraits<SILSSAUpdater> {
359357
auto *ti = predBlock->getTerminator();
360358

361359
changeEdgeValue(ti, phiBlock, phiArgIndex, value);
362-
363-
// Set the reborrow flag.
364-
phi->setReborrow(computeIsReborrow(phi));
365360
}
366361

367362
/// Check if an instruction is a PHI.

‎test/SIL/OwnershipVerifier/arguments.sil‎

Lines changed: 9 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ bb1(%1 : @guaranteed $Builtin.NativeObject):
3838

3939
// CHECK-LABEL: Error#: 0. Begin Error in Function: 'no_end_borrow_error2'
4040
// CHECK: Non trivial values, non address values, and non guaranteed function args must have at least one lifetime ending use?!
41-
// CHECK: Value: %3 = argument of bb1 : $Builtin.NativeObject
41+
// CHECK: Value: %1 = begin_borrow %0 : $Builtin.NativeObject
4242
// CHECK-LABEL: Error#: 0. End Error in Function: 'no_end_borrow_error2'
4343
//
4444
// CHECK-NOT: Error#: {{[0-9][0-9]*}}. End Error in Function: 'no_end_borrow_error2'
@@ -272,8 +272,8 @@ bb6:
272272
// CHECK: User: end_borrow %5 : $Builtin.NativeObject // id: %13
273273
// CHECK: Block: bb2
274274
// CHECK: Consuming Users:
275-
// CHECK: end_borrow %5 : $Builtin.NativeObject // id: %13
276-
// CHECK: end_borrow %5 : $Builtin.NativeObject // id: %7
275+
// CHECK: end_borrow %5 : $Builtin.NativeObject
276+
// CHECK: end_borrow %5 : $Builtin.NativeObject
277277
// CHECK: Error#: 6. End Error in Function: 'bad_order_add_a_level'
278278
//
279279
// CHECK-LABEL: Error#: 7. Begin Error in Function: 'bad_order_add_a_level'
@@ -282,36 +282,18 @@ bb6:
282282
// CHECK: Post Dominating Failure Blocks:
283283
// CHECK: bb3
284284
// CHECK: Error#: 7. End Error in Function: 'bad_order_add_a_level'
285-
286-
// CHECK: Error#: 9. Begin Error in Function: 'bad_order_add_a_level'
287-
// CHECK: Error! Found a leak due to a consuming post-dominance failure!
288-
// CHECK: Value: %5 = argument of bb2 : $Builtin.NativeObject // users: %13, %9, %7
289-
// CHECK: Post Dominating Failure Blocks:
290-
// CHECK: bb3
291-
// CHECK-LABEL: Error#: 9. End Error in Function: 'bad_order_add_a_level'
292285
//
293-
// CHECK-LABEL: Error#: 10. Begin Error in Function: 'bad_order_add_a_level'
286+
// CHECK-LABEL: Error#: 9. Begin Error in Function: 'bad_order_add_a_level'
294287
// CHECK: Found outside of lifetime use!
295288
// CHECK: Value: %5 = argument of bb2 : $Builtin.NativeObject // users: %13, %9, %7
296-
// CHECK: User: br bb4(%9 : $Builtin.NativeObject) // id: %10
289+
// CHECK: User: %9 = begin_borrow %5 : $Builtin.NativeObject
297290
// CHECK: Block: bb3
298-
// CHECK-LABEL: Error#: 10. End Error in Function: 'bad_order_add_a_level'
299-
//
300-
// We found a use that we didn't visit. For our purposes, we consider this to be
301-
// an outside lifetime use rather than use after freer to unite it with errors
302-
// from uses that are not reachable from the definition of the value.
303-
//
304-
// CHECK-LABEL: Error#: 11. Begin Error in Function: 'bad_order_add_a_level'
305-
// CHECK: Found outside of lifetime use!
306-
// CHECK: Value: %5 = argument of bb2 : $Builtin.NativeObject // users: %13, %9, %7
307-
// CHECK: User: %9 = begin_borrow %5 : $Builtin.NativeObject // user: %10
308-
// CHECK: Block: bb3
309-
// CHECK: Error#: 11. End Error in Function: 'bad_order_add_a_level'
291+
// CHECK-LABEL: Error#: 9. End Error in Function: 'bad_order_add_a_level'
310292
//
311-
// CHECK-LABEL: Error#: 12. Begin Error in Function: 'bad_order_add_a_level'
293+
// CHECK-LABEL: Error#: 10. Begin Error in Function: 'bad_order_add_a_level'
312294
// CHECK: Non trivial values, non address values, and non guaranteed function args must have at least one lifetime ending use?!
313-
// CHECK: Value: %11 = argument of bb4 : $Builtin.NativeObject
314-
// CHECK: Error#: 12. End Error in Function: 'bad_order_add_a_level'
295+
// CHECK: Value: %9 = begin_borrow %5 : $Builtin.NativeObject
296+
// CHECK: Error#: 10. End Error in Function: 'bad_order_add_a_level'
315297
sil [ossa] @bad_order_add_a_level : $@convention(thin) (@owned Builtin.NativeObject) -> () {
316298
bb0(%0 : @owned $Builtin.NativeObject):
317299
%1 = begin_borrow %0 : $Builtin.NativeObject

‎test/SILOptimizer/borrow_introducer_unit.sil‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -126,21 +126,16 @@ none(%phi : @guaranteed $FakeOptional<C>):
126126
return %retval : $()
127127
}
128128

129-
// An unreachable phi currently looks like a reborrow.
130-
//
131-
// TODO: When we have a reborrow flag, an unreachable phi can be
132-
// either a reborrow or forwarded, as long as it has no end_borrow.
129+
// An unreachable phi can be either a reborrow or forwarded, as long as it has no end_borrow.
133130

134131
// CHECK-LABEL: introducer_unreachable: find_borrow_introducers with: %phiCycle
135132
// CHECK: } // end sil function 'introducer_unreachable'
136133
// CHECK: Introducers:
137-
// CHECK-NEXT: argument of bb1 : $C
138134
// CHECK-NEXT: introducer_unreachable: find_borrow_introducers with: %phiCycle
139135

140136
// CHECK-LABEL: introducer_unreachable: borrow_introducers with: %phiCycle
141137
// CHECK: } // end sil function 'introducer_unreachable'
142138
// CHECK: Borrow introducers for: %1 = argument of bb1 : $C
143-
// CHECK-NEXT: argument of bb1 : $C
144139
// CHECK-NEXT: introducer_unreachable: borrow_introducers with: %phiCycle
145140
sil [ossa] @introducer_unreachable : $@convention(thin) () -> () {
146141
entry:

‎test/SILOptimizer/borrowed_from_updater.sil‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -64,13 +64,8 @@ bb1(%phi : @guaranteed $FakeOptional<C>):
6464
return %retval : $()
6565
}
6666

67-
// An unreachable phi currently looks like a reborrow.
68-
//
69-
// TODO: When we have a reborrow flag, an unreachable phi can be
70-
// either a reborrow or forwarded, as long as it has no end_borrow.
71-
7267
// CHECK-LABEL: sil [ossa] @introducer_unreachable : $@convention(thin) () -> () {
73-
// CHECK: bb1([[A:%.*]] : @reborrow @guaranteed $C):
68+
// CHECK: bb1([[A:%.*]] : @guaranteed $C):
7469
// CHECK-NEXT: [[B:%.*]] = borrowed [[A]] : $C from ()
7570
// CHECK: } // end sil function 'introducer_unreachable'
7671
sil [ossa] @introducer_unreachable : $@convention(thin) () -> () {

‎test/SILOptimizer/enclosing_def_unit.sil‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -222,18 +222,19 @@ bb3(%reborrow_inner3 : @guaranteed $C):
222222

223223
// CHECK-LABEL: enclosing_def_cycle: find_enclosing_defs with: %forward
224224
// CHECK: sil [ossa] @enclosing_def_cycle : $@convention(thin) (@guaranteed C) -> () {
225-
// CHECK: bb1([[REBORROW:%.*]] : @reborrow @guaranteed $C,
225+
// CHECK: bb1([[REBORROW:%.*]] : @guaranteed $C,
226226
// CHECK: } // end sil function 'enclosing_def_cycle'
227227
// CHECK: Enclosing Defs:
228228
// CHECK-NEXT: [[REBORROW]] = argument of bb1 : $C
229229
// CHECK-NEXT: enclosing_def_cycle: find_enclosing_defs with: %forward
230230

231231
// CHECK-LABEL: enclosing_def_cycle: enclosing_values with: %forward
232232
// CHECK: sil [ossa] @enclosing_def_cycle : $@convention(thin) (@guaranteed C) -> () {
233-
// CHECK: bb1([[REBORROW:%.*]] : @reborrow @guaranteed $C,
233+
// CHECK: bb1([[REBORROW:%.*]] : @guaranteed $C,
234234
// CHECK: } // end sil function 'enclosing_def_cycle'
235235
// CHECK: Enclosing values for: %{{.*}} = argument of bb1 : $C
236-
// CHECK-NEXT: [[REBORROW]] = argument of bb1 : $C
236+
// CHECK-NEXT: %1 = begin_borrow %0
237+
// CHECK-NEXT: %0 = argument of bb0
237238
// CHECK-NEXT: enclosing_def_cycle: enclosing_values with: %forward
238239
sil [ossa] @enclosing_def_cycle : $@convention(thin) (@guaranteed C) -> () {
239240
entry(%0 : @guaranteed $C):

0 commit comments

Comments
 (0)