Skip to content

Commit 44a2661

Browse files
atrickatrick
authored
Merge pull request #72764 from atrick/fix-address-yield
Fix LifetimeDependenceDefUseWalker for address yields.
2 parents 5a546ed + 1b6be74 commit 44a2661

File tree

6 files changed

+119
-35
lines changed

6 files changed

+119
-35
lines changed

‎SwiftCompilerSources/Sources/Optimizer/Utilities/AddressUtils.swift

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,6 @@ extension AddressUseVisitor {
127127
is InitEnumDataAddrInst, is UncheckedTakeEnumDataAddrInst,
128128
is InitExistentialAddrInst, is OpenExistentialAddrInst,
129129
is ProjectBlockStorageInst, is UncheckedAddrCastInst,
130-
is UnconditionalCheckedCastAddrInst,
131130
is MarkUninitializedInst, is DropDeinitInst,
132131
is CopyableToMoveOnlyWrapperAddrInst,
133132
is MoveOnlyWrapperToCopyableAddrInst,

‎SwiftCompilerSources/Sources/Optimizer/Utilities/LifetimeDependenceUtils.swift

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1022,12 +1022,14 @@ extension LifetimeDependenceDefUseWalker {
10221022
assert(!mdi.isUnresolved && !mdi.isNonEscaping,
10231023
"should be handled as a dependence by AddressUseVisitor")
10241024
}
1025-
if operand.instruction is ReturnInst, !operand.value.isEscapable {
1026-
return returnedDependence(result: operand)
1027-
}
1028-
if operand.instruction is YieldInst, !operand.value.isEscapable {
1029-
return yieldedDependence(result: operand)
1025+
if operand.instruction is YieldInst {
1026+
if operand.value.isEscapable {
1027+
return leafUse(of: operand)
1028+
} else {
1029+
return yieldedDependence(result: operand)
1030+
}
10301031
}
1032+
// Escaping an address
10311033
return escapingDependence(on: operand)
10321034
}
10331035

‎SwiftCompilerSources/Sources/SIL/Instruction.swift

Lines changed: 33 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -432,7 +432,9 @@ final public class DebugStepInst : Instruction {}
432432

433433
final public class SpecifyTestInst : Instruction {}
434434

435-
final public class UnconditionalCheckedCastAddrInst : Instruction {
435+
final public class UnconditionalCheckedCastAddrInst : Instruction, SourceDestAddrInstruction {
436+
public var isTakeOfSrc: Bool { true }
437+
public var isInitializationOfDest: Bool { true }
436438
public override var mayTrap: Bool { true }
437439
}
438440

@@ -527,9 +529,6 @@ final public class DeallocStackInst : Instruction, UnaryInstruction, Deallocatio
527529
}
528530
}
529531

530-
final public class DeallocPackInst : Instruction, UnaryInstruction, Deallocation {}
531-
final public class DeallocPackMetadataInst : Instruction, Deallocation {}
532-
533532
final public class DeallocStackRefInst : Instruction, UnaryInstruction, Deallocation {
534533
public var allocRef: AllocRefInstBase { operand.value as! AllocRefInstBase }
535534
}
@@ -701,12 +700,6 @@ class ValueMetatypeInst : SingleValueInstruction, UnaryInstruction {}
701700
final public
702701
class ExistentialMetatypeInst : SingleValueInstruction, UnaryInstruction {}
703702

704-
final public class OpenPackElementInst : SingleValueInstruction {}
705-
final public class PackLengthInst : SingleValueInstruction {}
706-
final public class DynamicPackIndexInst : SingleValueInstruction {}
707-
final public class PackPackIndexInst : SingleValueInstruction {}
708-
final public class ScalarPackIndexInst : SingleValueInstruction {}
709-
710703
final public class ObjCProtocolInst : SingleValueInstruction {}
711704

712705
public class GlobalAccessInstruction : SingleValueInstruction {
@@ -1093,20 +1086,6 @@ final public class ObjectInst : SingleValueInstruction {
10931086
final public class VectorInst : SingleValueInstruction {
10941087
}
10951088

1096-
final public class TuplePackExtractInst: SingleValueInstruction {
1097-
public var indexOperand: Operand { operands[0] }
1098-
public var tupleOperand: Operand { operands[1] }
1099-
}
1100-
1101-
final public class TuplePackElementAddrInst: SingleValueInstruction {
1102-
public var indexOperand: Operand { operands[0] }
1103-
public var tupleOperand: Operand { operands[1] }
1104-
}
1105-
1106-
final public class PackElementGetInst: SingleValueInstruction {}
1107-
1108-
final public class PackElementSetInst: SingleValueInstruction {}
1109-
11101089
final public class DifferentiableFunctionInst: SingleValueInstruction {}
11111090

11121091
final public class LinearFunctionInst: SingleValueInstruction {}
@@ -1140,9 +1119,6 @@ final public class AllocVectorInst : SingleValueInstruction, Allocation, UnaryIn
11401119
public var capacity: Value { operand.value }
11411120
}
11421121

1143-
final public class AllocPackInst : SingleValueInstruction, Allocation {}
1144-
final public class AllocPackMetadataInst : SingleValueInstruction, Allocation {}
1145-
11461122
public class AllocRefInstBase : SingleValueInstruction, Allocation {
11471123
final public var isObjC: Bool { bridged.AllocRefInstBase_isObjc() }
11481124

@@ -1361,6 +1337,36 @@ final public class DestructureTupleInst : MultipleValueInstruction, UnaryInstruc
13611337
public var `tuple`: Value { operand.value }
13621338
}
13631339

1340+
//===----------------------------------------------------------------------===//
1341+
// parameter pack instructions
1342+
//===----------------------------------------------------------------------===//
1343+
1344+
final public class AllocPackInst : SingleValueInstruction, Allocation {}
1345+
final public class AllocPackMetadataInst : SingleValueInstruction, Allocation {}
1346+
1347+
final public class DeallocPackInst : Instruction, UnaryInstruction, Deallocation {}
1348+
final public class DeallocPackMetadataInst : Instruction, Deallocation {}
1349+
1350+
final public class OpenPackElementInst : SingleValueInstruction {}
1351+
final public class PackLengthInst : SingleValueInstruction {}
1352+
final public class DynamicPackIndexInst : SingleValueInstruction {}
1353+
final public class PackPackIndexInst : SingleValueInstruction {}
1354+
final public class ScalarPackIndexInst : SingleValueInstruction {}
1355+
1356+
final public class TuplePackExtractInst: SingleValueInstruction {
1357+
public var indexOperand: Operand { operands[0] }
1358+
public var tupleOperand: Operand { operands[1] }
1359+
}
1360+
1361+
final public class TuplePackElementAddrInst: SingleValueInstruction {
1362+
public var indexOperand: Operand { operands[0] }
1363+
public var tupleOperand: Operand { operands[1] }
1364+
}
1365+
1366+
final public class PackElementGetInst: SingleValueInstruction {}
1367+
1368+
final public class PackElementSetInst: Instruction {}
1369+
13641370
//===----------------------------------------------------------------------===//
13651371
// terminator instructions
13661372
//===----------------------------------------------------------------------===//

‎lib/SIL/Verifier/SILVerifier.cpp

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -556,6 +556,17 @@ struct ImmutableAddressUseVerifier {
556556
return false;
557557
}
558558

559+
/// Handle instructions that move a value from one address into another
560+
/// address.
561+
bool isConsumingOrMutatingMoveAddrUse(Operand *use) {
562+
assert(use->getUser()->getNumOperands() == 2);
563+
auto opIdx = use->getOperandNumber();
564+
if (opIdx == CopyLikeInstruction::Dest)
565+
return true;
566+
assert(opIdx == CopyLikeInstruction::Src);
567+
return false;
568+
}
569+
559570
bool isAddrCastToNonConsuming(SingleValueInstruction *i) {
560571
// Check if any of our uses are consuming. If none of them are consuming, we
561572
// are good to go.
@@ -685,6 +696,12 @@ struct ImmutableAddressUseVerifier {
685696
}
686697
return true;
687698
}
699+
case SILInstructionKind::UnconditionalCheckedCastAddrInst:
700+
case SILInstructionKind::UncheckedRefCastAddrInst:
701+
if (isConsumingOrMutatingMoveAddrUse(use)) {
702+
return true;
703+
}
704+
break;
688705
case SILInstructionKind::CheckedCastAddrBranchInst:
689706
switch (cast<CheckedCastAddrBranchInst>(inst)->getConsumptionKind()) {
690707
case CastConsumptionKind::BorrowAlways:

‎test/SILOptimizer/lifetime_dependence_diagnostics.swift

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,29 @@ func bv_copy(_ bv: borrowing BV) -> dependsOn(bv) BV {
2323
}
2424

2525
struct NCInt: ~Copyable {
26-
var value: Int
26+
var i: Int
27+
}
28+
29+
public struct NEInt: ~Escapable {
30+
var i: Int
31+
32+
// Test yielding an address.
33+
// CHECK-LABEL: sil hidden @$s4test5NEIntV5ipropSivM : $@yield_once @convention(method) (@inout NEInt) -> @yields @inout Int {
34+
// CHECK: bb0(%0 : $*NEInt):
35+
// CHECK: [[A:%.*]] = begin_access [modify] [static] %0 : $*NEInt
36+
// CHECK: [[E:%.*]] = struct_element_addr [[A]] : $*NEInt, #NEInt.i
37+
// CHECK: yield [[E]] : $*Int, resume bb1, unwind bb2
38+
// CHECK: end_access [[A]] : $*NEInt
39+
// CHECK: end_access [[A]] : $*NEInt
40+
// CHECK-LABEL: } // end sil function '$s4test5NEIntV5ipropSivM'
41+
var iprop: Int {
42+
_read { yield i }
43+
_modify { yield &i }
44+
}
45+
46+
init(owner: borrowing NCInt) -> dependsOn(owner) Self {
47+
self.i = owner.i
48+
}
2749
}
2850

2951
func takeClosure(_: () -> ()) {}
@@ -56,5 +78,5 @@ func bv_borrow_borrow(bv: borrowing BV) -> dependsOn(scoped bv) BV {
5678
// because lifetime dependence does not expect a dependence directly on an 'inout' address without any 'begin_access'
5779
// marker.
5880
func ncint_capture(ncInt: inout NCInt) {
59-
takeClosure { _ = ncInt.value }
81+
takeClosure { _ = ncInt.i }
6082
}

‎test/SILOptimizer/ownership_liveness_unit.sil

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,44 @@ bb0(%0 : @guaranteed $C):
274274
return %99 : $()
275275
}
276276

277+
// CHECK-LABEL: testInteriorUnconditionalAddrCast: interior-liveness with: %1
278+
// CHECK: Interior liveness: %1 = argument of bb0 : $D
279+
// CHECK-NEXT: bb0: LiveWithin
280+
// CHECK-NEXT: regular user: [[FIELD:%.*]] = ref_element_addr %1 : $D, #D.object
281+
// CHECK-NEXT: regular user: unconditional_checked_cast_addr C in [[FIELD]] : $*C to D in %0 : $*D
282+
// CHECK-NEXT: regular user: copy_addr [take] %4 to [init] [[FIELD]] : $*C
283+
// CHECK-NEXT: regular user: unchecked_ref_cast_addr C in [[FIELD]] : $*C to D in %0 : $*D
284+
// CHECK-NEXT: Complete liveness
285+
// CHECK-NEXT: Unenclosed phis {
286+
// CHECK-NEXT: }
287+
// CHECK-NEXT: last user: unchecked_ref_cast_addr C in [[FIELD]] : $*C to D in %0 : $*D
288+
// CHECK-NEXT: testInteriorUnconditionalAddrCast: interior-liveness with: %1
289+
290+
// CHECK-LABEL: testInteriorUnconditionalAddrCast: interior_liveness_swift with: %1
291+
// CHECK: Interior liveness: %1 = argument of bb0 : $D
292+
// CHECK-NEXT: begin: [[FIELD]] = ref_element_addr %1 : $D, #D.object
293+
// CHECK-NEXT: ends: unchecked_ref_cast_addr C in [[FIELD]] : $*C to D in %0 : $*D
294+
// CHECK-NEXT: exits:
295+
// CHECK-NEXT: interiors: copy_addr [take] %4 to [init] [[FIELD]] : $*C
296+
// CHECK-NEXT: unconditional_checked_cast_addr C in [[FIELD]] : $*C to D in %0 : $*D
297+
// CHECK-NEXT: [[FIELD]] = ref_element_addr %1 : $D, #D.object
298+
// CHECK-NEXT: Unenclosed phis {
299+
// CHECK-NEXT: }
300+
// CHECK-NEXT: last user: unchecked_ref_cast_addr C in [[FIELD]] : $*C to D in %0 : $*D
301+
// CHECK-NEXT: testInteriorUnconditionalAddrCast: interior_liveness_swift with: %1
302+
sil [ossa] @testInteriorUnconditionalAddrCast : $@convention(thin) (@guaranteed D) -> @out D {
303+
bb0(%0 : $*D, %1 : @guaranteed $D):
304+
specify_test "interior-liveness %1"
305+
specify_test "interior_liveness_swift %1"
306+
%c1 = ref_element_addr %1 : $D, #D.object
307+
unconditional_checked_cast_addr C in %c1 : $*C to D in %0 : $*D
308+
%c2 = unchecked_addr_cast %0 : $*D to $*C
309+
copy_addr [take] %c2 to [init] %c1 : $*C
310+
unchecked_ref_cast_addr C in %c1 : $*C to D in %0 : $*D
311+
%99 = tuple()
312+
return %99 : $()
313+
}
314+
277315
// CHECK-LABEL: testInteriorReborrow: interior-liveness with: %borrow
278316
// CHECK: Complete liveness
279317
// CHECK-NEXT: Unenclosed phis {

0 commit comments

Comments
 (0)