Skip to content

Commit 9dba1c1

Browse files
committed
Temporary fix for OSSA RAUW utilities.
This fix unblocks unrelated optimizer commits. A unit test will be introduced with those commits. The RAUW utility does not correctly handle reborrows. It is in the middle of being rewritten. For now, simply bail out since this isn't an important case to optimize.
1 parent 93585a9 commit 9dba1c1

File tree

3 files changed

+32
-10
lines changed

3 files changed

+32
-10
lines changed

lib/SIL/Utils/OwnershipUtils.cpp

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,9 @@ bool swift::canOpcodeForwardOwnedValues(Operand *use) {
9090
// Skip over nested borrow scopes. Their scope-ending instructions are their use
9191
// points. Transitively find all nested scope-ending instructions by looking
9292
// through nested reborrows. Nested reborrows are not use points.
93+
//
94+
// FIXME: handle inner reborrows, which aren't dominated by
95+
// guaranteedValue. Audit all users to handle reborrows.
9396
bool swift::findInnerTransitiveGuaranteedUses(
9497
SILValue guaranteedValue, SmallVectorImpl<Operand *> *usePoints) {
9598

@@ -183,13 +186,18 @@ bool swift::findInnerTransitiveGuaranteedUses(
183186
break;
184187
}
185188
case OperandOwnership::Borrow:
186-
// FIXME: visitExtendedScopeEndingUses can't return false here once dead
189+
// FIXME: Use visitExtendedScopeEndingUses and audit all clients to handle
190+
// reborrows.
191+
//
192+
// FIXME: visit[Extended]ScopeEndingUses can't return false here once dead
187193
// borrows are disallowed.
188-
if (!BorrowingOperand(use).visitExtendedScopeEndingUses(
189-
[&](Operand *endUse) {
190-
leafUse(endUse);
191-
return true;
192-
})) {
194+
if (!BorrowingOperand(use).visitScopeEndingUses([&](Operand *endUse) {
195+
if (endUse->getOperandOwnership() == OperandOwnership::Reborrow) {
196+
foundPointerEscape = true;
197+
}
198+
leafUse(endUse);
199+
return true;
200+
})) {
193201
// Special case for dead borrows. This is dangerous because clients
194202
// don't expect a begin_borrow to be in the use list.
195203
leafUse(use);

lib/SILOptimizer/Utils/OwnershipOptUtils.cpp

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -392,18 +392,27 @@ bool OwnershipRAUWHelper::hasValidRAUWOwnership(SILValue oldValue,
392392
static bool canFixUpOwnershipForRAUW(SILValue oldValue, SILValue newValue,
393393
OwnershipFixupContext &context) {
394394
switch (oldValue.getOwnershipKind()) {
395-
case OwnershipKind::Guaranteed:
395+
case OwnershipKind::Guaranteed: {
396396
// Check that the old lifetime can be extended and record the necessary
397397
// book-keeping in the OwnershipFixupContext.
398398
context.clear();
399399

400400
// Check that no transitive uses have a PointerEscape, and record the leaf
401401
// uses for liveness extension.
402-
if (!findExtendedTransitiveGuaranteedUses(oldValue,
403-
context.guaranteedUsePoints))
402+
//
403+
// FIXME: Use findExtendedTransitiveGuaranteedUses and switch the
404+
// implementation of borrowCopyOverGuaranteedUses to
405+
// GuaranteedOwnershipExtension. Utils then, reborrows are considered
406+
// pointer escapes, causing findTransitiveGuaranteedUses to return false. So
407+
// they can be ignored.
408+
auto visitReborrow = [&](Operand *reborrow) {};
409+
if (!findTransitiveGuaranteedUses(oldValue, context.guaranteedUsePoints,
410+
visitReborrow)) {
404411
return false;
412+
}
405413
return OwnershipRAUWHelper::hasValidRAUWOwnership(
406414
oldValue, newValue, context.guaranteedUsePoints);
415+
}
407416
default: {
408417
SmallVector<Operand *, 8> ownedUsePoints;
409418
// If newValue is lexical, find the uses of oldValue so that it can be

test/SILOptimizer/cse_ossa_nontrivial.sil

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -609,10 +609,15 @@ struct RecurStruct {
609609
}
610610

611611
// Test to make sure we do not crash on a use after free when the branch instructions in bb1 and bb2 are deleted
612+
//
613+
// FIXME: Fix OwnershipRAUW to handle guaranteed uses of the old CSE'd
614+
// value that are reborrows. The reborrowed uses are not dominated by
615+
// the old value, but they are still within the new values borrow scope.
616+
//
612617
// CHECK-LABEL: sil [ossa] @test_useafterfreeinrauw :
613618
// CHECK: bb0
614619
// CHECK: struct_extract
615-
// CHECK-NOT: struct_extract
620+
// FIXME-CHECK-NOT: struct_extract
616621
// CHECK: cond_br
617622
// CHECK-LABEL: } // end sil function 'test_useafterfreeinrauw'
618623
sil [ossa] @test_useafterfreeinrauw : $@convention(thin) (@guaranteed RecurStruct) -> @owned NonTrivialStruct {

0 commit comments

Comments
 (0)