Skip to content

Fix AddressOwnershipLiveRange to include the full range. #80782

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 12, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -501,6 +501,11 @@ enum AddressOwnershipLiveRange : CustomStringConvertible {
}
}

/// Return the live range of the addressable value that reaches 'begin', not including 'begin', which may itself be an
/// access of the address.
///
/// The range ends at the destroy or reassignment of the addressable value.
///
/// Return nil if the live range is unknown.
static func compute(for address: Value, at begin: Instruction,
_ localReachabilityCache: LocalVariableReachabilityCache,
Expand Down Expand Up @@ -624,7 +629,7 @@ extension AddressOwnershipLiveRange {
var reachableUses = Stack<LocalVariableAccess>(context)
defer { reachableUses.deinitialize() }

localReachability.gatherKnownReachableUses(from: assignment, in: &reachableUses)
localReachability.gatherKnownLifetimeUses(from: assignment, in: &reachableUses)

let assignmentInst = assignment.instruction ?? allocation.parentFunction.entryBlock.instructions.first!
var range = InstructionRange(begin: assignmentInst, context)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -930,8 +930,8 @@ extension LifetimeDependenceDefUseWalker {
// of its forwarded address has were visited by LocalVariableAccessWalker and recorded as separate local accesses.
return .continueWalk
case .store:
let si = localAccess.operand!.instruction as! StoringInstruction
assert(si.sourceOperand == initialValue, "the only reachable store should be the current assignment")
// A store does not use the previous in-memory value.
return .continueWalk
case .apply:
return visitAppliedUse(of: localAccess.operand!, by: localAccess.instruction as! FullApplySite)
case .escape:
Expand All @@ -946,7 +946,6 @@ extension LifetimeDependenceDefUseWalker {
case .incomingArgument:
fatalError("Incoming arguments are never reachable")
}
return .continueWalk
}

private mutating func visitAppliedUse(of operand: Operand, by apply: FullApplySite) -> WalkResult {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -630,13 +630,17 @@ struct LocalVariableReachableAccess {

// Find reaching assignments...
extension LocalVariableReachableAccess {
// Gather all fully assigned accesses that reach `instruction`.
// Gather all fully assigned accesses that reach 'instruction'. If 'instruction' is itself a modify access, it is
// ignored and the nearest assignments above 'instruction' are still gathered.
func gatherReachingAssignments(for instruction: Instruction, in accessStack: inout Stack<LocalVariableAccess>)
-> Bool {
var blockList = BasicBlockWorklist(context)
defer { blockList.deinitialize() }

let initialEffect = backwardScanAccesses(before: instruction, accessStack: &accessStack)
var initialEffect: BlockEffect? = nil
if let prev = instruction.previous {
initialEffect = backwardScanAccesses(before: prev, accessStack: &accessStack)
}
if !backwardPropagateEffect(in: instruction.parentBlock, effect: initialEffect, blockList: &blockList,
accessStack: &accessStack) {
return false
Expand All @@ -647,7 +651,7 @@ extension LocalVariableReachableAccess {
// lattice: none -> read -> modify -> escape -> assign
//
// `blockInfo.effect` is the same as `currentEffect` returned by backwardScanAccesses, except when an early escape
// happens after an assign.
// happens below an assign, in which case we report the escape here.
switch currentEffect {
case .none, .read, .modify, .escape:
break
Expand Down Expand Up @@ -695,7 +699,6 @@ extension LocalVariableReachableAccess {
continue
case .assign:
accessStack.push(accessInfo.access)
break
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

was there any functional change here ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, that just looks like a cleanup to avoid confusion with the for-loop break

case .escape:
break
}
Expand All @@ -709,25 +712,31 @@ extension LocalVariableReachableAccess {
extension LocalVariableReachableAccess {
/// This performs a forward CFG walk to find known reachable uses from `assignment`. This ignores aliasing and
/// escapes.
func gatherKnownReachableUses(from assignment: LocalVariableAccess,
///
/// The known live range is the range in which the assigned value is valid and may be used by dependent values. It
/// includes the destroy or reassignment of the local.
func gatherKnownLifetimeUses(from assignment: LocalVariableAccess,
in accessStack: inout Stack<LocalVariableAccess>) {
if let modifyInst = assignment.instruction {
_ = gatherReachableUses(after: modifyInst, in: &accessStack, allowEscape: true)
_ = gatherReachableUses(after: modifyInst, in: &accessStack, lifetime: true)
return
}
gatherKnownReachableUsesFromEntry(in: &accessStack)
gatherKnownLifetimeUsesFromEntry(in: &accessStack)
}

/// This performs a forward CFG walk to find known reachable uses from the function entry. This ignores aliasing and
/// escapes.
private func gatherKnownReachableUsesFromEntry(in accessStack: inout Stack<LocalVariableAccess>) {
private func gatherKnownLifetimeUsesFromEntry(in accessStack: inout Stack<LocalVariableAccess>) {
assert(accessMap.liveInAccess!.kind == .incomingArgument, "only an argument access is live in to the function")
let firstInst = accessMap.function.entryBlock.instructions.first!
_ = gatherReachableUses(onOrAfter: firstInst, in: &accessStack, allowEscape: true)
_ = gatherReachableUses(onOrAfter: firstInst, in: &accessStack, lifetime: true)
}

/// This performs a forward CFG walk to find all reachable uses of `modifyInst`. `modifyInst` may be a `begin_access
/// [modify]` or instruction that initializes the local variable.
///
/// This does not include the destroy or reassignment of the value set by `modifyInst`.
///
/// Returns true if all possible reachable uses were visited. Returns false if any escapes may reach `modifyInst` are
/// reachable from `modifyInst`.
///
Expand All @@ -749,37 +758,40 @@ extension LocalVariableReachableAccess {
if accessInfo.hasEscaped! {
return false
}
return gatherReachableUses(after: modifyInst, in: &accessStack, allowEscape: false)
return gatherReachableUses(after: modifyInst, in: &accessStack, lifetime: false)
}

/// This performs a forward CFG walk to find all uses of this local variable reachable after `begin`.
///
/// If `allowEscape` is true, then this returns false if the walk ended early because of a reachable escape.
/// If `lifetime` is true, then this gathers the full known lifetime, includeing destroys and reassignments ignoring
/// escapes.
///
/// If `lifetime` is false, then this returns `false` if the walk ended early because of a reachable escape.
private func gatherReachableUses(after begin: Instruction, in accessStack: inout Stack<LocalVariableAccess>,
allowEscape: Bool) -> Bool {
lifetime: Bool) -> Bool {
if let term = begin as? TermInst {
for succ in term.successors {
if !gatherReachableUses(onOrAfter: succ.instructions.first!, in: &accessStack, allowEscape: allowEscape) {
if !gatherReachableUses(onOrAfter: succ.instructions.first!, in: &accessStack, lifetime: lifetime) {
return false
}
}
return true
} else {
return gatherReachableUses(onOrAfter: begin.next!, in: &accessStack, allowEscape: allowEscape)
return gatherReachableUses(onOrAfter: begin.next!, in: &accessStack, lifetime: lifetime)
}
}

/// This performs a forward CFG walk to find all uses of this local variable reachable after and including `begin`.
///
/// If `allowEscape` is true, then this returns false if the walk ended early because of a reachable escape.
/// If `lifetime` is true, then this returns false if the walk ended early because of a reachable escape.
private func gatherReachableUses(onOrAfter begin: Instruction, in accessStack: inout Stack<LocalVariableAccess>,
allowEscape: Bool) -> Bool {
lifetime: Bool) -> Bool {
var blockList = BasicBlockWorklist(context)
defer { blockList.deinitialize() }

let initialBlock = begin.parentBlock
let initialEffect = forwardScanAccesses(after: begin, accessStack: &accessStack, allowEscape: allowEscape)
if !allowEscape, initialEffect == .escape {
let initialEffect = forwardScanAccesses(after: begin, accessStack: &accessStack, lifetime: lifetime)
if !lifetime, initialEffect == .escape {
return false
}
forwardPropagateEffect(in: initialBlock, blockInfo: blockMap[initialBlock], effect: initialEffect,
Expand All @@ -795,15 +807,15 @@ extension LocalVariableReachableAccess {
case .none:
break
case .escape:
if !allowEscape {
if !lifetime {
break
}
fallthrough
case .read, .modify, .assign:
let firstInst = block.instructions.first!
currentEffect = forwardScanAccesses(after: firstInst, accessStack: &accessStack, allowEscape: allowEscape)
currentEffect = forwardScanAccesses(after: firstInst, accessStack: &accessStack, lifetime: lifetime)
}
if !allowEscape, currentEffect == .escape {
if !lifetime, currentEffect == .escape {
return false
}
forwardPropagateEffect(in: block, blockInfo: blockInfo, effect: currentEffect, blockList: &blockList,
Expand Down Expand Up @@ -836,9 +848,9 @@ extension LocalVariableReachableAccess {
}

// Check all instructions in this block after and including `begin`. Return a BlockEffect indicating the combined
// effects seen before stopping the scan. An .assign stops the scan. A .escape stops the scan if allowEscape is false.
// effects seen before stopping the scan. An .assign stops the scan. A .escape stops the scan if lifetime is false.
private func forwardScanAccesses(after first: Instruction, accessStack: inout Stack<LocalVariableAccess>,
allowEscape: Bool)
lifetime: Bool)
-> BlockEffect? {
var currentEffect: BlockEffect?
for inst in InstructionList(first: first) {
Expand All @@ -848,9 +860,12 @@ extension LocalVariableReachableAccess {
currentEffect = BlockEffect(for: accessInfo, accessMap.context).meet(currentEffect)
switch currentEffect! {
case .assign:
if lifetime {
accessStack.push(accessInfo.access)
}
return currentEffect
case .escape:
if !allowEscape {
if !lifetime {
log("Local variable: \(accessMap.allocation)\n escapes at \(inst)")
return currentEffect
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,27 +40,40 @@ struct View : ~Escapable {
}
}

struct MutableView : ~Copyable, ~Escapable {
let ptr: UnsafeRawBufferPointer
struct NCMutableContainer : ~Copyable {
let ptr: UnsafeMutableRawBufferPointer
let c: Int
@lifetime(borrow ptr)
init(_ ptr: UnsafeRawBufferPointer, _ c: Int) {
init(_ ptr: UnsafeMutableRawBufferPointer, _ c: Int) {
self.ptr = ptr
self.c = c
}
}

struct MutableView : ~Copyable, ~Escapable {
let ptr: UnsafeMutableRawBufferPointer
@lifetime(borrow ptr)
init(_ ptr: UnsafeMutableRawBufferPointer) {
self.ptr = ptr
}
@lifetime(copy otherBV)
init(_ otherBV: borrowing View) {
init(_ otherBV: borrowing MutableView) {
self.ptr = otherBV.ptr
self.c = otherBV.c
}
init(_ k: borrowing NCContainer) {
init(_ k: borrowing NCMutableContainer) {
self.ptr = k.ptr
self.c = k.c
}
}

extension MutableView {
@lifetime(&self)
mutating public func update() -> Self {
return unsafe MutableView(ptr)
}
}

func use(_ o : borrowing View) {}
func mutate(_ x: inout NCContainer) { }
func mutate(_ x: inout NCMutableContainer) { }
func mutate(_ x: inout View) { }
func consume(_ o : consuming View) {}
func use(_ o : borrowing MutableView) {}
Expand Down Expand Up @@ -125,9 +138,9 @@ func test3(_ a: Array<Int>) {
}
}

func test4(_ a: Array<Int>) {
a.withUnsafeBytes {
var x = NCContainer($0, a.count)
func test4(_ a: inout Array<Int>) {
a.withUnsafeMutableBytes {
var x = NCMutableContainer($0, $0.count)
mutate(&x)
let view = MutableView(x)
use(view)
Expand Down Expand Up @@ -180,11 +193,11 @@ func test7(_ a: UnsafeRawBufferPointer) {
mutate(&x)
}

func test8(_ a: Array<Int>) {
a.withUnsafeBytes {
var x = View($0, a.count)
func test8(_ a: inout Array<Int>) {
a.withUnsafeMutableBytes {
var x = View(UnsafeRawBufferPointer(start: $0.baseAddress!, count: $0.count), $0.count)
mutate(&x)
let view = MutableView(x)
let view = MutableView($0)
use(view)
consume(view)
}
Expand Down Expand Up @@ -245,3 +258,29 @@ func testPointeeDependenceOnMutablePointer(p: UnsafePointer<Int64>) {
_ = ptr.pointee
_ = ptr
}

// CHECK-LABEL: sil hidden @$s31lifetime_dependence_scope_fixup16testReassignment1bySw_tF : $@convention(thin) (UnsafeMutableRawBufferPointer) -> () {
// CHECK: bb0(%0 : $UnsafeMutableRawBufferPointer):
// CHECK: [[VAR:%.*]] = alloc_stack [lexical] [var_decl] $MutableView, var, name "span", type $MutableView
// CHECK: apply %{{.*}}(%0, %{{.*}}) : $@convention(method) (UnsafeMutableRawBufferPointer, @thin MutableView.Type) -> @lifetime(borrow 0) @owned MutableView
// CHECK: [[ACCESS1:%.*]] = begin_access [modify] [static] [[VAR]] : $*MutableView
// CHECK: apply %{{.*}}(%{{.*}}) : $@convention(method) (@inout MutableView) -> @lifetime(borrow 0) @owned MutableView
// CHECK: [[LD1:%.*]] = load %{{.*}} : $*MutableView
// CHECK: apply %{{.*}}([[LD1]]) : $@convention(thin) (@guaranteed MutableView) -> ()
// CHECK: end_access [[ACCESS1]] : $*MutableView
// CHECK: [[ACCESS2:%.*]] = begin_access [modify] [static] [[VAR]] : $*MutableView
// CHECK: apply %{{.*}}(%{{.*}}) : $@convention(method) (@inout MutableView) -> @lifetime(borrow 0) @owned MutableView
// CHECK: [[LD2:%.*]] = load %{{.*}} : $*MutableView
// CHECK: apply %{{.*}}([[LD2]]) : $@convention(thin) (@guaranteed MutableView) -> ()
// CHECK: end_access [[ACCESS2]] : $*MutableView
// CHECK: destroy_addr [[VAR]] : $*MutableView
// CHECK-LABEL: } // end sil function '$s31lifetime_dependence_scope_fixup16testReassignment1bySw_tF'
func testReassignment(b: UnsafeMutableRawBufferPointer) {
var span = MutableView(b)

var sub = span.update()
use(sub)

sub = span.update()
use(sub)
}
39 changes: 39 additions & 0 deletions test/SILOptimizer/lifetime_dependence/scope_fixup.sil
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ struct TrivialHolder {
var pointer: UnsafeRawPointer
}

struct A {
init()
}

struct Holder {
var object: AnyObject
}
Expand All @@ -59,6 +63,9 @@ sil [ossa] @Wrapper_init : $@convention(method) (@owned NE, @thin Wrapper.Type)

sil [ossa] @NCContainer_ne_read : $@yield_once @convention(method) (@guaranteed NCContainer) -> @lifetime(borrow 0) @yields @guaranteed NE

sil @yieldRawSpan : $@yield_once @convention(method) (@in_guaranteed A) -> @lifetime(borrow address_for_deps 0) @yields @guaranteed RawSpan
sil @useRawSpan : $@convention(thin) (@guaranteed RawSpan) -> ()

// NCContainer.wrapper._read:
// var wrapper: Wrapper {
// _read {
Expand Down Expand Up @@ -237,3 +244,35 @@ bb0(%0 : @guaranteed $Holder):
%99 = tuple ()
return %99 : $()
}

// FIXME: extend temporary lifetimes for coroutine access.
//
// CHECK-LABEL: sil [ossa] @testYieldSpan : $@convention(thin) (@in A) -> () {
// CHECK: alloc_stack [lexical] [var_decl] $A
// CHECK: (%{{.*}}, %{{.*}}) = begin_apply %{{.*}}(%{{.*}}) : $@yield_once @convention(method) (@in_guaranteed A) -> @lifetime(borrow address_for_deps 0) @yields @guaranteed RawSpan
// CHECK: [[MD:%.*]] = mark_dependence [unresolved]
// CHECK: [[CP:%.*]] = copy_value [[MD]]
// CHECK: apply %{{.*}}([[CP]]) : $@convention(thin) (@guaranteed RawSpan) -> ()
// CHECK: destroy_value [[CP]]
// CHECK: end_apply
// CHECK: destroy_addr
// CHECK-LABEL: } // end sil function 'testYieldSpan'
sil [ossa] @testYieldSpan : $@convention(thin) (@in A) -> () {
bb0(%0 : $*A):
%1 = alloc_stack [lexical] [var_decl] $A
copy_addr [take] %0 to [init] %1

%3 = function_ref @yieldRawSpan : $@yield_once @convention(method) (@in_guaranteed A) -> @lifetime(borrow address_for_deps 0) @yields @guaranteed RawSpan
(%4, %5) = begin_apply %3(%1) : $@yield_once @convention(method) (@in_guaranteed A) -> @lifetime(borrow address_for_deps 0) @yields @guaranteed RawSpan
%6 = mark_dependence [unresolved] %4 on %5
%7 = copy_value %6
%8 = end_apply %5 as $()

%9 = function_ref @useRawSpan : $@convention(thin) (@guaranteed RawSpan) -> ()
%10 = apply %9(%7) : $@convention(thin) (@guaranteed RawSpan) -> ()
destroy_value %7
destroy_addr %1
dealloc_stack %1
%14 = tuple ()
return %14
}