From 0d0b6b153bb59ca4ef974501e430b45a649ba31b Mon Sep 17 00:00:00 2001 From: Andrew Trick Date: Mon, 7 Jul 2025 14:01:44 -0700 Subject: [PATCH 1/4] Fix AllocBoxToStack to handle mark_dependence[_addr] Handle the easy case in which the promoted box is the base operand of a mark_dep. This requires nothing more than setting the base operand to the new stack address. Required for move-only checking. Otherwise, we get an incredibly bizarre compiler error: noncopyable 'foo' cannot be consumed when captured by an escaping closure Fixes rdar://150398673 ([nonescapable] allocbox-to-stack fails causing lifetime diagnostics to fail) --- include/swift/SIL/SILInstruction.h | 15 ++++ .../Transforms/AllocBoxToStack.cpp | 13 ++++ test/SILOptimizer/allocbox_to_stack_ne.sil | 68 +++++++++++++++++++ 3 files changed, 96 insertions(+) create mode 100644 test/SILOptimizer/allocbox_to_stack_ne.sil diff --git a/include/swift/SIL/SILInstruction.h b/include/swift/SIL/SILInstruction.h index 5cc1b87bf63c7..89908bbc6eeb1 100644 --- a/include/swift/SIL/SILInstruction.h +++ b/include/swift/SIL/SILInstruction.h @@ -8964,6 +8964,21 @@ class MarkDependenceInstruction { return SILValue(); } + void setBase(SILValue newVal) { + if (inst) { + switch (inst->getKind()) { + case SILInstructionKind::MarkDependenceInst: + cast(inst)->setBase(newVal); + break; + case SILInstructionKind::MarkDependenceAddrInst: + cast(inst)->setBase(newVal); + break; + default: + break; + } + } + } + SILValue getDependent() const { if (inst) { switch (inst->getKind()) { diff --git a/lib/SILOptimizer/Transforms/AllocBoxToStack.cpp b/lib/SILOptimizer/Transforms/AllocBoxToStack.cpp index c780a9be1f342..a9bc266195a8e 100644 --- a/lib/SILOptimizer/Transforms/AllocBoxToStack.cpp +++ b/lib/SILOptimizer/Transforms/AllocBoxToStack.cpp @@ -400,6 +400,13 @@ static SILInstruction *recursivelyFindBoxOperandsPromotableToAddress( isa(User)) continue; + // mark_dependence base value uses will be directly converted to base + // address uses. + if (auto mdi = MarkDependenceInstruction(User)) { + if (Op->get() == mdi.getBase()) + continue; + } + // If our user instruction is a copy_value or a mark_uninitialized, visit // the users recursively. if (isa(User) || isa(User) || @@ -738,6 +745,12 @@ static bool rewriteAllocBoxAsAllocStack(AllocBoxInst *ABI) { Inst->eraseFromParent(); continue; } + // mark_dependence base value uses will be directly converted to base + // address uses. + if (auto mdi = MarkDependenceInstruction(User)) { + mdi.setBase(StackBox); + continue; + } assert(isa(User) || isa(User) || isa(User) || isa(User) || diff --git a/test/SILOptimizer/allocbox_to_stack_ne.sil b/test/SILOptimizer/allocbox_to_stack_ne.sil new file mode 100644 index 0000000000000..2a5219585a3b8 --- /dev/null +++ b/test/SILOptimizer/allocbox_to_stack_ne.sil @@ -0,0 +1,68 @@ +// RUN: %target-sil-opt -enable-experimental-feature LifetimeDependence -allocbox-to-stack %s | %FileCheck %s + +// REQUIRES: swift_feature_LifetimeDependence + +import Swift +import Builtin + +public struct View : ~Escapable { + let _ptr: UnsafeRawPointer +} + +sil [ossa] @initFn : $@convention(method) (UnsafeRawPointer, @thin View.Type) -> @lifetime(borrow 0) @owned View +sil [ossa] @initFnAddr : $@convention(thin) (UnsafeRawPointer, @thin View.Type) -> @lifetime(borrow 0) @out View + +// CHECK-LABEL: sil [ossa] @testBoxedParam : +// CHECK-NOT: alloc_box +// CHECK-LABEL: } // end sil function 'testBoxedParam' +sil [ossa] @testBoxedParam : $@convention(thin) (@owned View) -> @lifetime(copy 0) @owned View { +bb0(%0 : @noImplicitCopy @_eagerMove @owned $View): + %1 = alloc_box ${ var @moveOnly View }, var, name "that" + %2 = begin_borrow [var_decl] %1 + %3 = project_box %2, 0 + %4 = moveonlywrapper_to_copyable_addr %3 + store %0 to [init] %4 + %6 = metatype $@thin View.Type + %7 = begin_access [read] [static] %3 + %8 = mark_unresolved_non_copyable_value [no_consume_or_assign] %7 + %9 = struct_element_addr %8, #View._ptr + %10 = load_borrow %9 + %11 = moveonlywrapper_to_copyable [guaranteed] %10 + end_borrow %10 + end_access %7 + %14 = function_ref @initFn : $@convention(method) (UnsafeRawPointer, @thin View.Type) -> @lifetime(borrow 0) @owned View + %15 = apply %14(%11, %6) : $@convention(method) (UnsafeRawPointer, @thin View.Type) -> @lifetime(borrow 0) @owned View + %16 = mark_dependence [unresolved] %15 on %2 + end_borrow %2 + destroy_value %1 + return %16 +} + +// CHECK-LABEL: sil [ossa] @testBoxedParamAddr : +// CHECK-NOT: alloc_box +// CHECK-LABEL: } // end sil function 'testBoxedParamAddr' +sil [ossa] @testBoxedParamAddr : $@convention(thin) (@owned View) -> () { +bb0(%0 : @noImplicitCopy @_eagerMove @owned $View): + %1 = alloc_box ${ var @moveOnly View }, var, name "that" + %2 = begin_borrow [var_decl] %1 + %3 = project_box %2, 0 + %4 = moveonlywrapper_to_copyable_addr %3 + store %0 to [init] %4 + %6 = metatype $@thin View.Type + %7 = begin_access [read] [static] %3 + %8 = mark_unresolved_non_copyable_value [no_consume_or_assign] %7 + %9 = struct_element_addr %8, #View._ptr + %10 = load_borrow %9 + %11 = moveonlywrapper_to_copyable [guaranteed] %10 + end_borrow %10 + end_access %7 + %14 = alloc_stack $View + %15 = function_ref @initFnAddr : $@convention(thin) (UnsafeRawPointer, @thin View.Type) -> @lifetime(borrow 0) @out View + %16 = apply %15(%14, %11, %6) : $@convention(thin) (UnsafeRawPointer, @thin View.Type) -> @lifetime(borrow 0) @out View + mark_dependence_addr [unresolved] %14 on %2 + dealloc_stack %14 + end_borrow %2 + destroy_value %1 + %99 = tuple () + return %99 +} From 749f186200e5e86518f954c1649e85b81b29c61f Mon Sep 17 00:00:00 2001 From: Andrew Trick Date: Mon, 7 Jul 2025 14:05:27 -0700 Subject: [PATCH 2/4] Update SwiftCompilerSources AllocBoxToStack to handle mark_dep. Handle the easy case in which the promoted box is the base operand of a mark_dep. This requires nothing more than setting the base operand to the new stack address. Required for move-only checking. Otherwise, we get an incredibly bizarre compiler error: noncopyable 'foo' cannot be consumed when captured by an escaping closure The legacy pass was updated first for 6.2. --- .../Optimizer/FunctionPasses/AllocBoxToStack.swift | 9 +++++++++ test/SILOptimizer/allocbox_to_stack_ne.sil | 1 + 2 files changed, 10 insertions(+) diff --git a/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/AllocBoxToStack.swift b/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/AllocBoxToStack.swift index 416350b9b8d17..fae1f76f70504 100644 --- a/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/AllocBoxToStack.swift +++ b/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/AllocBoxToStack.swift @@ -154,6 +154,12 @@ private func canPromote(allocBox: AllocBoxInst) -> (promotableArguments: [Functi fallthrough case is MarkUninitializedInst, is CopyValueInst, is MoveValueInst: worklist.pushIfNotVisited(use.instruction as! SingleValueInstruction) + case let markDep as MarkDependenceInstruction: + if markDep.baseOperand == use { + // mark_dependence base value uses will be directly converted to base address uses. + break + } + return nil case let apply as ApplySite: if apply.isCallee(operand: use) { // Calling the closure does not escape the closure value. @@ -226,6 +232,9 @@ private struct FunctionSpecializations { // First, replace the instruction with the original `box`, which adds more uses to `box`. // In a later iteration those additional uses will be handled. (user as! SingleValueInstruction).replace(with: box, context) + case let markDep as MarkDependenceInstruction: + assert(markDep.baseOperand == use) + markDep.baseOperand.set(to: stack, context) case let apply as ApplySite: specialize(apply: apply, context) default: diff --git a/test/SILOptimizer/allocbox_to_stack_ne.sil b/test/SILOptimizer/allocbox_to_stack_ne.sil index 2a5219585a3b8..a7dd3ddcfc605 100644 --- a/test/SILOptimizer/allocbox_to_stack_ne.sil +++ b/test/SILOptimizer/allocbox_to_stack_ne.sil @@ -1,4 +1,5 @@ // RUN: %target-sil-opt -enable-experimental-feature LifetimeDependence -allocbox-to-stack %s | %FileCheck %s +// RUN: %target-sil-opt -enable-experimental-feature LifetimeDependence -legacy-allocbox-to-stack %s | %FileCheck %s // REQUIRES: swift_feature_LifetimeDependence From 078770caf6ea070dfbee98c540b2f836a3dc6692 Mon Sep 17 00:00:00 2001 From: Andrew Trick Date: Tue, 8 Jul 2025 16:34:34 -0700 Subject: [PATCH 3/4] [NFC] add LifetimeDiagnostics internal tracing Make the actual errors more obvious in the trace output since diagnostics messages are not printed in-line with tracing. --- .../FunctionPasses/LifetimeDependenceDiagnostics.swift | 2 ++ 1 file changed, 2 insertions(+) diff --git a/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LifetimeDependenceDiagnostics.swift b/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LifetimeDependenceDiagnostics.swift index 69694a06100c2..0d55d469d74c0 100644 --- a/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LifetimeDependenceDiagnostics.swift +++ b/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LifetimeDependenceDiagnostics.swift @@ -264,6 +264,8 @@ private struct DiagnoseDependence { } onError() + log(" Error: lifetime-dependence violation - escaping use") + // Identify the escaping variable. let escapingVar = LifetimeVariable(usedBy: operand, context) if let varDecl = escapingVar.varDecl { From a281754e990242cf87d7a4d7f4eed97e3e94bc35 Mon Sep 17 00:00:00 2001 From: Andrew Trick Date: Tue, 8 Jul 2025 16:35:52 -0700 Subject: [PATCH 4/4] Add a SIL test for LifetimeDependenceDiagnostics alloc_stack support --- .../verify_diagnostics.sil | 33 +++++++++++++++++-- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/test/SILOptimizer/lifetime_dependence/verify_diagnostics.sil b/test/SILOptimizer/lifetime_dependence/verify_diagnostics.sil index 4c7e44db6406e..75ead981e3a04 100644 --- a/test/SILOptimizer/lifetime_dependence/verify_diagnostics.sil +++ b/test/SILOptimizer/lifetime_dependence/verify_diagnostics.sil @@ -52,13 +52,14 @@ sil @useNE : $@convention(thin) (@guaranteed NE) -> () sil @reborrowNE : $@convention(thin) (@guaranteed NE) -> @lifetime(borrow 0) @owned NE sil @initHolder : $@convention(thin) () -> @out Holder +sil @getHolderNE : $@convention(thin) (@guaranteed Holder) -> @lifetime(borrow 0) @owned NE sil @getNE : $@convention(thin) (@in_guaranteed Holder) -> @lifetime(borrow address_for_deps 0) @owned NE sil @getNEIndirect : $@convention(thin) (@in_guaranteed Holder) -> @lifetime(borrow address_for_deps 0) @out NE sil @initTrivialHolder : $@convention(thin) () -> @out TrivialHolder sil @getTrivialNE : $@convention(thin) (@in_guaranteed TrivialHolder) -> @lifetime(borrow address_for_deps 0) @owned NE -sil @makeHolder: $@convention(method) (@thin Holder.Type) -> @owned Holder // user: %6 +sil @makeHolder: $@convention(thin) () -> @owned Holder sil @getGeneric : $@convention(thin) <τ_0_0 where τ_0_0 : ~Escapable> (@guaranteed Holder, @thick τ_0_0.Type) -> @lifetime(borrow 0) @out τ_0_0 // user: %12 // Test returning a owned dependence on a trivial value @@ -196,9 +197,8 @@ sil [ossa] @testBorrowAddress : $@convention(thin) (@th bb0(%0 : $*T, %1 : $@thick T.Type): debug_value %1, let, name "type", argno 1 %3 = alloc_stack [lexical] [var_decl] $Holder, var, name "holder", type $Holder - %4 = metatype $@thin Holder.Type - %5 = function_ref @makeHolder : $@convention(method) (@thin Holder.Type) -> @owned Holder + %5 = function_ref @makeHolder : $@convention(thin) () -> @owned Holder %6 = apply %5(%4) : $@convention(method) (@thin Holder.Type) -> @owned Holder store %6 to [init] %3 %8 = alloc_stack [lexical] [var_decl] $T, let, name "result" @@ -246,3 +246,30 @@ bb0(%0 : $*NE, %1 : $*Holder): %18 = tuple () return %18 } + +// Test dependence on an alloc_stack variable. +// +//!!! +sil private [ossa] @testOnStackHolder : $@convention(thin) (@guaranteed Holder) -> () { +bb0(%0 : @guaranteed $Holder): + %6 = alloc_stack [lexical] [var_decl] $Holder, let, name "c", type $Holder + + %10 = function_ref @makeHolder : $@convention(thin) () -> @owned Holder + %11 = apply %10() : $@convention(thin) () -> @owned Holder + store %11 to [init] %6 + %13 = load_borrow %6 + + %14 = function_ref @getHolderNE : $@convention(thin) (@guaranteed Holder) -> @lifetime(borrow 0) @owned NE + %15 = apply %14(%13) : $@convention(thin) (@guaranteed Holder) -> @lifetime(borrow 0) @owned NE + %16 = mark_dependence [unresolved] %15 on %6 + %17 = move_value [var_decl] %16 + end_borrow %13 + + %20 = function_ref @useNE : $@convention(thin) (@guaranteed NE) -> () + %21 = apply %20(%17) : $@convention(thin) (@guaranteed NE) -> () + destroy_value %17 + destroy_addr %6 + dealloc_stack %6 + %26 = tuple () + return %26 +}