Skip to content

Commit 2ce9dde

Browse files
committed
ConditionForwarding: don't violate ownership
Instructions in a block, which is moved, must not use any (non-trivial) value because we don't do liveness analysis. When moving a block, there is no guarantee that the operand value is still alive at the new location. Fixes an ownership violation error rdar://146630743
1 parent 737b5ec commit 2ce9dde

File tree

2 files changed

+54
-5
lines changed

2 files changed

+54
-5
lines changed

lib/SILOptimizer/Transforms/ConditionForwarding.cpp

+19-5
Original file line numberDiff line numberDiff line change
@@ -126,8 +126,25 @@ class ConditionForwarding : public SILFunctionTransform {
126126

127127
/// Returns true if all instructions of block \p BB are safe to be moved
128128
/// across other code.
129-
static bool hasNoRelevantSideEffects(SILBasicBlock *BB) {
129+
static bool hasNoRelevantSideEffects(SILBasicBlock *BB, EnumInst *enumInst) {
130130
for (SILInstruction &I : *BB) {
131+
if (BB->getParent()->hasOwnership() && &I != enumInst) {
132+
// The instruction must not use any (non-trivial) value because we don't
133+
// do liveness analysis. When moving the block, there is no guarantee that
134+
// the operand value is still alive at the new location.
135+
for (Operand *op : I.getRealOperands()) {
136+
SILValue opv = op->get();
137+
// The `enum` is an exception, because it's a forwarded value and we already
138+
// check that it's forwarded to the `switch_enum` at the new location.
139+
if (opv == enumInst)
140+
continue;
141+
// If the value is defined in the block it's a block-local liferange.
142+
if (opv->getParentBlock() == BB)
143+
continue;
144+
if (opv->getOwnershipKind() != OwnershipKind::None)
145+
return false;
146+
}
147+
}
131148
if (I.getMemoryBehavior() == MemoryBehavior::None)
132149
continue;
133150
if (auto *CF = dyn_cast<CondFailInst>(&I)) {
@@ -144,9 +161,6 @@ static bool hasNoRelevantSideEffects(SILBasicBlock *BB) {
144161
return false;
145162
continue;
146163
}
147-
if (isa<BeginBorrowInst>(&I) || isa<EndBorrowInst>(&I)) {
148-
continue;
149-
}
150164
LLVM_DEBUG(llvm::dbgs() << "Bailing out, found inst with side-effects ");
151165
LLVM_DEBUG(I.dump());
152166
return false;
@@ -212,7 +226,7 @@ bool ConditionForwarding::tryOptimize(SwitchEnumInst *SEI) {
212226
CommonBranchBlock = PredPred;
213227

214228
// We cannot move the block across other code if it has side-effects.
215-
if (!hasNoRelevantSideEffects(Pred))
229+
if (!hasNoRelevantSideEffects(Pred, EI))
216230
return false;
217231
PredBlocks.push_back(Pred);
218232
}

test/SILOptimizer/conditionforwarding_ossa.sil

+35
Original file line numberDiff line numberDiff line change
@@ -419,3 +419,38 @@ bb6:
419419
%15 = tuple ()
420420
return %15
421421
}
422+
423+
// CHECK-LABEL: sil [ossa] @copy_from_limited_liferange :
424+
// CHECK: switch_enum
425+
// CHECK-LABEL: } // end sil function 'copy_from_limited_liferange'
426+
sil [ossa] @copy_from_limited_liferange : $@convention(thin) (Builtin.Int1, @guaranteed C) -> () {
427+
bb0(%0 : $Builtin.Int1, %1 : @guaranteed $C):
428+
%2 = begin_borrow [lexical] %1
429+
cond_br %0, bb1, bb2
430+
431+
bb1:
432+
%4 = copy_value %2
433+
%5 = enum $Optional<C>, #Optional.some!enumelt, %4
434+
br bb3(%5)
435+
436+
bb2:
437+
%7 = enum $Optional<C>, #Optional.none!enumelt
438+
br bb3(%7)
439+
440+
bb3(%9 : @owned $Optional<C>):
441+
end_borrow %2
442+
switch_enum %9, case #Optional.some!enumelt: bb5, case #Optional.none!enumelt: bb4
443+
444+
bb4:
445+
br bb6
446+
447+
bb5(%13 : @owned $C):
448+
destroy_value %13
449+
br bb6
450+
451+
bb6:
452+
%16 = tuple ()
453+
return %16
454+
}
455+
456+

0 commit comments

Comments
 (0)