Skip to content

Commit 188ecdc

Browse files
committed
Fix ARCSequenceOpts handle of is_escaping_closure
Handle is_escaping_closure like an RC-checking instructions. It looks like it "may-decrement" RC because: if the object must be live after the instruction and the subsequent local release, then it expects the refcount to be greater than one. In other words, do not remove a retain/release pair that spans an is_escaping_closure instruction. This would otherwise fail to catch closures that are returned from the function. The following retain/release pair cannot be removed: %me = partial_apply retain %me is_escaping_closure %me release %me return %me Fixes <rdar://problem/52803634>
1 parent 9bd600e commit 188ecdc

File tree

3 files changed

+45
-2
lines changed

3 files changed

+45
-2
lines changed

lib/SILOptimizer/ARC/ARCMatchingSet.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ ARCMatchingSetBuilder::matchIncrementsToDecrements() {
7777

7878
bool BUCodeMotionSafe = (*BURefCountState)->second.isCodeMotionSafe();
7979
LLVM_DEBUG(llvm::dbgs() << " BOTTOM UP CODEMOTIONSAFE: "
80-
<< (BUIsKnownSafe ? "true" : "false") << "\n");
80+
<< (BUCodeMotionSafe ? "true" : "false") << "\n");
8181
Flags.CodeMotionSafe &= BUCodeMotionSafe;
8282

8383
// Now that we know we have an inst, grab the decrement.
@@ -161,7 +161,7 @@ ARCMatchingSetBuilder::matchDecrementsToIncrements() {
161161

162162
bool TDCodeMotionSafe = (*TDRefCountState)->second.isCodeMotionSafe();
163163
LLVM_DEBUG(llvm::dbgs() << " TOP DOWN CODEMOTIONSAFE: "
164-
<< (TDIsKnownSafe ? "true" : "false") << "\n");
164+
<< (TDCodeMotionSafe ? "true" : "false") << "\n");
165165
Flags.CodeMotionSafe &= TDCodeMotionSafe;
166166

167167
// Now that we know we have an inst, grab the decrement.

lib/SILOptimizer/Analysis/ARCAnalysis.cpp

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -459,6 +459,25 @@ valueHasARCDecrementOrCheckInInstructionRange(SILValue Op,
459459
bool
460460
swift::
461461
mayGuaranteedUseValue(SILInstruction *User, SILValue Ptr, AliasAnalysis *AA) {
462+
// Instructions that check the ref count are modeled as both a potential
463+
// decrement and a use.
464+
if (mayCheckRefCount(User)) {
465+
switch (User->getKind()) {
466+
case SILInstructionKind::IsUniqueInst:
467+
// This instruction takes the address of its referent, so there's no way
468+
// for the optimizer to reuse the reference across it (it appears to
469+
// mutate the reference itself). In fact it's operand's RC root would be
470+
// the parent object. This means we can ignore it as a direct RC user.
471+
return false;
472+
case SILInstructionKind::IsEscapingClosureInst:
473+
// FIXME: this is overly conservative. It should return true only of the
474+
// RC identity of the single operand matches Ptr.
475+
return true;
476+
default:
477+
llvm_unreachable("Unexpected check-ref-count instruction.");
478+
}
479+
}
480+
462481
// Only full apply sites can require a guaranteed lifetime. If we don't have
463482
// one, bail.
464483
if (!isa<FullApplySite>(User))

test/SILOptimizer/arcsequenceopts_uniquecheck.sil

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,3 +104,27 @@ bb0(%0 : $*C, %1 : $*C):
104104
strong_release %2 : $C
105105
return %4 : $Builtin.Int1
106106
}
107+
108+
// Similarly, do not remove a retain/release pair that spans an
109+
// is_escaping_closure instruction. This would fail to catch closures
110+
// that are returned from the function.
111+
// <rdar://problem/52803634>
112+
113+
// thunk for @callee_guaranteed () -> ()
114+
sil shared [transparent] [serializable] [reabstraction_thunk] [without_actually_escaping] @$sIg_Ieg_TR : $@convention(thin) (@noescape @callee_guaranteed () -> ()) -> () {
115+
bb0(%0 : $@noescape @callee_guaranteed () -> ()):
116+
%1 = apply %0() : $@noescape @callee_guaranteed () -> ()
117+
return %1 : $()
118+
}
119+
120+
sil hidden [noinline] @testEscapingClosure : $@convention(thin) (@noescape @callee_guaranteed () -> ()) -> @owned @callee_guaranteed () -> () {
121+
bb0(%0 : $@noescape @callee_guaranteed () -> ()):
122+
%1 = function_ref @$sIg_Ieg_TR : $@convention(thin) (@noescape @callee_guaranteed () -> ()) -> ()
123+
%2 = partial_apply [callee_guaranteed] %1(%0) : $@convention(thin) (@noescape @callee_guaranteed () -> ()) -> ()
124+
%3 = mark_dependence %2 : $@callee_guaranteed () -> () on %0 : $@noescape @callee_guaranteed () -> ()
125+
strong_retain %3 : $@callee_guaranteed () -> ()
126+
%5 = is_escaping_closure %3 : $@callee_guaranteed () -> ()
127+
strong_release %3 : $@callee_guaranteed () -> ()
128+
cond_fail %5 : $Builtin.Int1
129+
return %3 : $@callee_guaranteed () -> ()
130+
}

0 commit comments

Comments
 (0)