Skip to content

Commit 273874c

Browse files
committed
ConstantCapturePropagation: don't propagate keypaths with multiple uses in non-OSSA
We cannot do this because we don't know where to insert the compensating release after the propagated `partial_apply`. A required `strong_retain` may have been moved over the `partial_apply`. Then we would release the keypath too early. Fixes a mis-compile rdar://161321614
1 parent 1262647 commit 273874c

File tree

2 files changed

+38
-6
lines changed

2 files changed

+38
-6
lines changed

SwiftCompilerSources/Sources/Optimizer/FunctionPasses/ConstantCapturePropagation.swift

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -305,7 +305,9 @@ private extension PartialApplyInst {
305305
var nonConstArgs = [Operand]()
306306
var hasKeypath = false
307307
for argOp in argumentOperands {
308-
switch argOp.value.isConstant() {
308+
// In non-OSSA we don't know where to insert the compensating release for a propagated keypath.
309+
// Therefore bail if a keypath has multiple uses.
310+
switch argOp.value.isConstant(requireSingleUse: !parentFunction.hasOwnership && !isOnStack) {
309311
case .constant:
310312
constArgs.append(argOp)
311313
case .constantWithKeypath:
@@ -357,26 +359,33 @@ private enum ConstantKind {
357359
}
358360

359361
private extension Value {
360-
func isConstant() -> ConstantKind {
362+
func isConstant(requireSingleUse: Bool) -> ConstantKind {
361363
// All instructions handled here must also be handled in
362364
// `FunctionSignatureSpecializationMangler::mangleConstantProp`.
365+
let result: ConstantKind
363366
switch self {
364367
case let si as StructInst:
365-
return si.operands.reduce(.constant, { $0.merge(with: $1.value.isConstant()) })
368+
result = si.operands.reduce(.constant, {
369+
$0.merge(with: $1.value.isConstant(requireSingleUse: requireSingleUse))
370+
})
366371
case is ThinToThickFunctionInst, is ConvertFunctionInst, is UpcastInst, is OpenExistentialRefInst:
367-
return (self as! UnaryInstruction).operand.value.isConstant()
372+
result = (self as! UnaryInstruction).operand.value.isConstant(requireSingleUse: requireSingleUse)
368373
case is StringLiteralInst, is IntegerLiteralInst, is FloatLiteralInst, is FunctionRefInst, is GlobalAddrInst:
369-
return .constant
374+
result = .constant
370375
case let keyPath as KeyPathInst:
371376
guard keyPath.operands.isEmpty,
372377
keyPath.hasPattern,
373378
!keyPath.substitutionMap.hasAnySubstitutableParams
374379
else {
375380
return .notConstant
376381
}
377-
return .constantWithKeypath
382+
result = .constantWithKeypath
378383
default:
379384
return .notConstant
380385
}
386+
if requireSingleUse, result == .constantWithKeypath, !uses.ignoreDebugUses.isSingleUse {
387+
return .notConstant
388+
}
389+
return result
381390
}
382391
}

test/SILOptimizer/capture_propagation.sil

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -558,6 +558,29 @@ bb0:
558558
return %7 : $()
559559
}
560560

561+
// CHECK-LABEL: sil @dontPropagateMulitUseKeyPathInNonOSSA :
562+
// CHECK: %0 = keypath
563+
// CHECK: [[U:%.*]] = upcast %0
564+
// CHECK: [[C:%.*]] = function_ref @closureWithKeypath
565+
// CHECK: partial_apply [callee_guaranteed] [[C]]([[U]])
566+
// CHECK: } // end sil function 'dontPropagateMulitUseKeyPathInNonOSSA'
567+
sil @dontPropagateMulitUseKeyPathInNonOSSA : $@convention(thin) () -> () {
568+
bb0:
569+
%0 = keypath $WritableKeyPath<Str, Int>, (root $Str; stored_property #Str.a : $Int)
570+
%c = upcast %0 to $KeyPath<Str, Int>
571+
%1 = function_ref @closureWithKeypath : $@convention(thin) (Str, @guaranteed KeyPath<Str, Int>) -> Int
572+
%2 = partial_apply [callee_guaranteed] %1(%c) : $@convention(thin) (Str, @guaranteed KeyPath<Str, Int>) -> Int
573+
strong_retain %0
574+
%3 = convert_escape_to_noescape %2 : $@callee_guaranteed (Str) -> Int to $@noescape @callee_guaranteed (Str) -> Int
575+
%4 = function_ref @calleeWithKeypath : $@convention(thin) (@noescape @callee_guaranteed (Str) -> Int) -> ()
576+
%5 = apply %4(%3) : $@convention(thin) (@noescape @callee_guaranteed (Str) -> Int) -> ()
577+
strong_release %2 : $@callee_guaranteed (Str) -> Int
578+
strong_release %0
579+
%7 = tuple ()
580+
return %7 : $()
581+
}
582+
583+
561584
// CHECK-LABEL: sil shared @$s18closureWithKeypath{{.*}}main3StrVSiTf3npk_n : $@convention(thin) (Str) -> Int {
562585
// CHECK: [[K:%[0-9]+]] = keypath
563586
// CHECK: [[F:%[0-9]+]] = function_ref @swift_getAtKeyPath

0 commit comments

Comments
 (0)