From 05790dc14b5cca6fde26e55caf86e21cfe502c65 Mon Sep 17 00:00:00 2001 From: Andrew Trick Date: Fri, 29 Mar 2024 15:40:29 -0700 Subject: [PATCH 1/4] Fix SILVerifier and AddressUtils handling of addr casts. These core utils did not handle UnconditionalCheckedCastAddrInst or UncheckedRefCastAddrInst correctly. (cherry picked from commit a1bb9f401a15436af44911e8924fe810befb7b26) --- .../Optimizer/Utilities/AddressUtils.swift | 1 - .../Sources/SIL/Instruction.swift | 4 +- lib/SIL/Verifier/SILVerifier.cpp | 17 +++++++++ test/SILOptimizer/ownership_liveness_unit.sil | 38 +++++++++++++++++++ 4 files changed, 58 insertions(+), 2 deletions(-) diff --git a/SwiftCompilerSources/Sources/Optimizer/Utilities/AddressUtils.swift b/SwiftCompilerSources/Sources/Optimizer/Utilities/AddressUtils.swift index 4baa1611490ee..2473411e433b4 100644 --- a/SwiftCompilerSources/Sources/Optimizer/Utilities/AddressUtils.swift +++ b/SwiftCompilerSources/Sources/Optimizer/Utilities/AddressUtils.swift @@ -127,7 +127,6 @@ extension AddressUseVisitor { is InitEnumDataAddrInst, is UncheckedTakeEnumDataAddrInst, is InitExistentialAddrInst, is OpenExistentialAddrInst, is ProjectBlockStorageInst, is UncheckedAddrCastInst, - is UnconditionalCheckedCastAddrInst, is MarkUninitializedInst, is DropDeinitInst, is CopyableToMoveOnlyWrapperAddrInst, is MoveOnlyWrapperToCopyableAddrInst, diff --git a/SwiftCompilerSources/Sources/SIL/Instruction.swift b/SwiftCompilerSources/Sources/SIL/Instruction.swift index ec134d81187b7..5b70f85013a23 100644 --- a/SwiftCompilerSources/Sources/SIL/Instruction.swift +++ b/SwiftCompilerSources/Sources/SIL/Instruction.swift @@ -432,7 +432,9 @@ final public class DebugStepInst : Instruction {} final public class SpecifyTestInst : Instruction {} -final public class UnconditionalCheckedCastAddrInst : Instruction { +final public class UnconditionalCheckedCastAddrInst : Instruction, SourceDestAddrInstruction { + public var isTakeOfSrc: Bool { true } + public var isInitializationOfDest: Bool { true } public override var mayTrap: Bool { true } } diff --git a/lib/SIL/Verifier/SILVerifier.cpp b/lib/SIL/Verifier/SILVerifier.cpp index 8c9aa2a27cbfa..28271a716952c 100644 --- a/lib/SIL/Verifier/SILVerifier.cpp +++ b/lib/SIL/Verifier/SILVerifier.cpp @@ -556,6 +556,17 @@ struct ImmutableAddressUseVerifier { return false; } + /// Handle instructions that move a value from one address into another + /// address. + bool isConsumingOrMutatingMoveAddrUse(Operand *use) { + assert(use->getUser()->getNumOperands() == 2); + auto opIdx = use->getOperandNumber(); + if (opIdx == CopyLikeInstruction::Dest) + return true; + assert(opIdx == CopyLikeInstruction::Src); + return false; + } + bool isAddrCastToNonConsuming(SingleValueInstruction *i) { // Check if any of our uses are consuming. If none of them are consuming, we // are good to go. @@ -679,6 +690,12 @@ struct ImmutableAddressUseVerifier { } return true; } + case SILInstructionKind::UnconditionalCheckedCastAddrInst: + case SILInstructionKind::UncheckedRefCastAddrInst: + if (isConsumingOrMutatingMoveAddrUse(use)) { + return true; + } + break; case SILInstructionKind::CheckedCastAddrBranchInst: switch (cast(inst)->getConsumptionKind()) { case CastConsumptionKind::BorrowAlways: diff --git a/test/SILOptimizer/ownership_liveness_unit.sil b/test/SILOptimizer/ownership_liveness_unit.sil index 03cd0f67f2827..f92820ee7875c 100644 --- a/test/SILOptimizer/ownership_liveness_unit.sil +++ b/test/SILOptimizer/ownership_liveness_unit.sil @@ -274,6 +274,44 @@ bb0(%0 : @guaranteed $C): return %99 : $() } +// CHECK-LABEL: testInteriorUnconditionalAddrCast: interior-liveness with: %1 +// CHECK: Interior liveness: %1 = argument of bb0 : $D +// CHECK-NEXT: bb0: LiveWithin +// CHECK-NEXT: regular user: [[FIELD:%.*]] = ref_element_addr %1 : $D, #D.object +// CHECK-NEXT: regular user: unconditional_checked_cast_addr C in [[FIELD]] : $*C to D in %0 : $*D +// CHECK-NEXT: regular user: copy_addr [take] %4 to [init] [[FIELD]] : $*C +// CHECK-NEXT: regular user: unchecked_ref_cast_addr C in [[FIELD]] : $*C to D in %0 : $*D +// CHECK-NEXT: Complete liveness +// CHECK-NEXT: Unenclosed phis { +// CHECK-NEXT: } +// CHECK-NEXT: last user: unchecked_ref_cast_addr C in [[FIELD]] : $*C to D in %0 : $*D +// CHECK-NEXT: testInteriorUnconditionalAddrCast: interior-liveness with: %1 + +// CHECK-LABEL: testInteriorUnconditionalAddrCast: interior_liveness_swift with: %1 +// CHECK: Interior liveness: %1 = argument of bb0 : $D +// CHECK-NEXT: begin: [[FIELD]] = ref_element_addr %1 : $D, #D.object +// CHECK-NEXT: ends: unchecked_ref_cast_addr C in [[FIELD]] : $*C to D in %0 : $*D +// CHECK-NEXT: exits: +// CHECK-NEXT: interiors: copy_addr [take] %4 to [init] [[FIELD]] : $*C +// CHECK-NEXT: unconditional_checked_cast_addr C in [[FIELD]] : $*C to D in %0 : $*D +// CHECK-NEXT: [[FIELD]] = ref_element_addr %1 : $D, #D.object +// CHECK-NEXT: Unenclosed phis { +// CHECK-NEXT: } +// CHECK-NEXT: last user: unchecked_ref_cast_addr C in [[FIELD]] : $*C to D in %0 : $*D +// CHECK-NEXT: testInteriorUnconditionalAddrCast: interior_liveness_swift with: %1 +sil [ossa] @testInteriorUnconditionalAddrCast : $@convention(thin) (@guaranteed D) -> @out D { +bb0(%0 : $*D, %1 : @guaranteed $D): + specify_test "interior-liveness %1" + specify_test "interior_liveness_swift %1" + %c1 = ref_element_addr %1 : $D, #D.object + unconditional_checked_cast_addr C in %c1 : $*C to D in %0 : $*D + %c2 = unchecked_addr_cast %0 : $*D to $*C + copy_addr [take] %c2 to [init] %c1 : $*C + unchecked_ref_cast_addr C in %c1 : $*C to D in %0 : $*D + %99 = tuple() + return %99 : $() +} + // CHECK-LABEL: testInteriorReborrow: interior-liveness with: %borrow // CHECK: Complete liveness // CHECK-NEXT: Unenclosed phis { From c58ba970f59ba50b8268bad37d8ebf7ec219f404 Mon Sep 17 00:00:00 2001 From: Andrew Trick Date: Mon, 1 Apr 2024 16:49:19 -0700 Subject: [PATCH 2/4] SwiftCompilerSources: fix PackElementSetInst It is not a SingleValueInstruction. (cherry picked from commit 66dcf6a35c3eddae48d20b8ae56740f2d668e0f4) --- SwiftCompilerSources/Sources/SIL/Instruction.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/SwiftCompilerSources/Sources/SIL/Instruction.swift b/SwiftCompilerSources/Sources/SIL/Instruction.swift index 5b70f85013a23..463cd9ace5ccb 100644 --- a/SwiftCompilerSources/Sources/SIL/Instruction.swift +++ b/SwiftCompilerSources/Sources/SIL/Instruction.swift @@ -1107,7 +1107,7 @@ final public class TuplePackElementAddrInst: SingleValueInstruction { final public class PackElementGetInst: SingleValueInstruction {} -final public class PackElementSetInst: SingleValueInstruction {} +final public class PackElementSetInst: Instruction {} final public class DifferentiableFunctionInst: SingleValueInstruction {} From ed417d07d00b78beacd1d7b9f677065d05c853d4 Mon Sep 17 00:00:00 2001 From: Andrew Trick Date: Mon, 1 Apr 2024 17:27:18 -0700 Subject: [PATCH 3/4] SwiftCompilerSources: organize parameter pack instructions (cherry picked from commit 6fcec2ff96d82fba932cbf9c0855b0349f93b379) --- .../Sources/SIL/Instruction.swift | 56 ++++++++++--------- 1 file changed, 30 insertions(+), 26 deletions(-) diff --git a/SwiftCompilerSources/Sources/SIL/Instruction.swift b/SwiftCompilerSources/Sources/SIL/Instruction.swift index 463cd9ace5ccb..5288c4604c348 100644 --- a/SwiftCompilerSources/Sources/SIL/Instruction.swift +++ b/SwiftCompilerSources/Sources/SIL/Instruction.swift @@ -529,9 +529,6 @@ final public class DeallocStackInst : Instruction, UnaryInstruction, Deallocatio } } -final public class DeallocPackInst : Instruction, UnaryInstruction, Deallocation {} -final public class DeallocPackMetadataInst : Instruction, Deallocation {} - final public class DeallocStackRefInst : Instruction, UnaryInstruction, Deallocation { public var allocRef: AllocRefInstBase { operand.value as! AllocRefInstBase } } @@ -703,12 +700,6 @@ class ValueMetatypeInst : SingleValueInstruction, UnaryInstruction {} final public class ExistentialMetatypeInst : SingleValueInstruction, UnaryInstruction {} -final public class OpenPackElementInst : SingleValueInstruction {} -final public class PackLengthInst : SingleValueInstruction {} -final public class DynamicPackIndexInst : SingleValueInstruction {} -final public class PackPackIndexInst : SingleValueInstruction {} -final public class ScalarPackIndexInst : SingleValueInstruction {} - final public class ObjCProtocolInst : SingleValueInstruction {} public class GlobalAccessInstruction : SingleValueInstruction { @@ -1095,20 +1086,6 @@ final public class ObjectInst : SingleValueInstruction { final public class VectorInst : SingleValueInstruction { } -final public class TuplePackExtractInst: SingleValueInstruction { - public var indexOperand: Operand { operands[0] } - public var tupleOperand: Operand { operands[1] } -} - -final public class TuplePackElementAddrInst: SingleValueInstruction { - public var indexOperand: Operand { operands[0] } - public var tupleOperand: Operand { operands[1] } -} - -final public class PackElementGetInst: SingleValueInstruction {} - -final public class PackElementSetInst: Instruction {} - final public class DifferentiableFunctionInst: SingleValueInstruction {} final public class LinearFunctionInst: SingleValueInstruction {} @@ -1142,9 +1119,6 @@ final public class AllocVectorInst : SingleValueInstruction, Allocation, UnaryIn public var capacity: Value { operand.value } } -final public class AllocPackInst : SingleValueInstruction, Allocation {} -final public class AllocPackMetadataInst : SingleValueInstruction, Allocation {} - public class AllocRefInstBase : SingleValueInstruction, Allocation { final public var isObjC: Bool { bridged.AllocRefInstBase_isObjc() } @@ -1363,6 +1337,36 @@ final public class DestructureTupleInst : MultipleValueInstruction, UnaryInstruc public var `tuple`: Value { operand.value } } +//===----------------------------------------------------------------------===// +// parameter pack instructions +//===----------------------------------------------------------------------===// + +final public class AllocPackInst : SingleValueInstruction, Allocation {} +final public class AllocPackMetadataInst : SingleValueInstruction, Allocation {} + +final public class DeallocPackInst : Instruction, UnaryInstruction, Deallocation {} +final public class DeallocPackMetadataInst : Instruction, Deallocation {} + +final public class OpenPackElementInst : SingleValueInstruction {} +final public class PackLengthInst : SingleValueInstruction {} +final public class DynamicPackIndexInst : SingleValueInstruction {} +final public class PackPackIndexInst : SingleValueInstruction {} +final public class ScalarPackIndexInst : SingleValueInstruction {} + +final public class TuplePackExtractInst: SingleValueInstruction { + public var indexOperand: Operand { operands[0] } + public var tupleOperand: Operand { operands[1] } +} + +final public class TuplePackElementAddrInst: SingleValueInstruction { + public var indexOperand: Operand { operands[0] } + public var tupleOperand: Operand { operands[1] } +} + +final public class PackElementGetInst: SingleValueInstruction {} + +final public class PackElementSetInst: Instruction {} + //===----------------------------------------------------------------------===// // terminator instructions //===----------------------------------------------------------------------===// From 0954b1b15820f534db041359ed54ef64bf355966 Mon Sep 17 00:00:00 2001 From: Andrew Trick Date: Fri, 29 Mar 2024 15:42:23 -0700 Subject: [PATCH 4/4] Fix LifetimeDependenceDefUseWalker for address yields. Do not treat address yields as escapes. Fixes rdar://125752476 (`UnsafeRawPointer` property in non-escapable type doesn't compile) (cherry picked from commit 1b6be740e857fc39943a726f31f80c57e74d096e) --- .../Utilities/LifetimeDependenceUtils.swift | 12 +++++---- .../lifetime_dependence_diagnostics.swift | 26 +++++++++++++++++-- 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/SwiftCompilerSources/Sources/Optimizer/Utilities/LifetimeDependenceUtils.swift b/SwiftCompilerSources/Sources/Optimizer/Utilities/LifetimeDependenceUtils.swift index 10c5a48e5850f..47a5eb1ad3ea3 100644 --- a/SwiftCompilerSources/Sources/Optimizer/Utilities/LifetimeDependenceUtils.swift +++ b/SwiftCompilerSources/Sources/Optimizer/Utilities/LifetimeDependenceUtils.swift @@ -1022,12 +1022,14 @@ extension LifetimeDependenceDefUseWalker { assert(!mdi.isUnresolved && !mdi.isNonEscaping, "should be handled as a dependence by AddressUseVisitor") } - if operand.instruction is ReturnInst, !operand.value.isEscapable { - return returnedDependence(result: operand) - } - if operand.instruction is YieldInst, !operand.value.isEscapable { - return yieldedDependence(result: operand) + if operand.instruction is YieldInst { + if operand.value.isEscapable { + return leafUse(of: operand) + } else { + return yieldedDependence(result: operand) + } } + // Escaping an address return escapingDependence(on: operand) } diff --git a/test/SILOptimizer/lifetime_dependence_diagnostics.swift b/test/SILOptimizer/lifetime_dependence_diagnostics.swift index e27f1d5fb8cd3..fe59019c84a5f 100644 --- a/test/SILOptimizer/lifetime_dependence_diagnostics.swift +++ b/test/SILOptimizer/lifetime_dependence_diagnostics.swift @@ -23,7 +23,29 @@ func bv_copy(_ bv: borrowing BV) -> dependsOn(bv) BV { } struct NCInt: ~Copyable { - var value: Int + var i: Int +} + +public struct NEInt: ~Escapable { + var i: Int + + // Test yielding an address. + // CHECK-LABEL: sil hidden @$s4test5NEIntV5ipropSivM : $@yield_once @convention(method) (@inout NEInt) -> @yields @inout Int { + // CHECK: bb0(%0 : $*NEInt): + // CHECK: [[A:%.*]] = begin_access [modify] [static] %0 : $*NEInt + // CHECK: [[E:%.*]] = struct_element_addr [[A]] : $*NEInt, #NEInt.i + // CHECK: yield [[E]] : $*Int, resume bb1, unwind bb2 + // CHECK: end_access [[A]] : $*NEInt + // CHECK: end_access [[A]] : $*NEInt + // CHECK-LABEL: } // end sil function '$s4test5NEIntV5ipropSivM' + var iprop: Int { + _read { yield i } + _modify { yield &i } + } + + init(owner: borrowing NCInt) -> dependsOn(owner) Self { + self.i = owner.i + } } func takeClosure(_: () -> ()) {} @@ -56,5 +78,5 @@ func bv_borrow_borrow(bv: borrowing BV) -> dependsOn(scoped bv) BV { // because lifetime dependence does not expect a dependence directly on an 'inout' address without any 'begin_access' // marker. func ncint_capture(ncInt: inout NCInt) { - takeClosure { _ = ncInt.value } + takeClosure { _ = ncInt.i } }