[SIL-opaque] Handled forward captures. #1

nate-chandler · 2022-03-21T18:41:34Z

A let that isn't defined yet can be captured by a local function to which a closure is formed. Don't use an address type unless we're using lowered addresses.

Merge the AddressLowering pass from its old development branch and update it so we can begin incrementally enabling it under a flag. This has been reimplemented for simplicity. There's no point in looking at the old code.

This could happen as a result of specialization or concrete address-only values. For now, it's just tested by SIL unit tests.

Mostly documentation and typos.

Compute the latestOpeningInst, not the firstOpeningInst.

In classic compiler terminology, this is a "phi copy" algorithm. But the documentation now tries to clearly distinguish between "semantics copies" vs. moves, where moves are "storage copies".

Explain high-level objectives and terminology with more precision.

Avoid attempting to coalesce enum payloads.

CHECK lines still need to be updated for OSSA

Anywhere that code is not obviously inserted immediately adjacent to the origin instruction.

The function attribute's name changed on main.

Temporarily map storage to a fake load_borrow.

Add comments. Add a basic dominance sanity check.

For borrowing a projection.

nate-chandler · 2022-03-21T19:11:01Z

Without opaque values:

// opaque_values_silgen.forward_capture_generic_let<A>(with: A) -> Swift.Array<A>
sil hidden [ossa] @$s20opaque_values_silgen27forward_capture_generic_let4withSayxGx_tlF : $@convention(thin) <U> (@in_guaranteed U) -> @owned Array<U> {
// %0 "u"                                         // users: %4, %1
bb0(%0 : @guaranteed $U):
  debug_value %0 : $U, let, name "u", argno 1     // id: %1
  // function_ref f #1 <A>() -> () in opaque_values_silgen.forward_capture_generic_let<A>(with: A) -> Swift.Array<A>
  %2 = function_ref @$s20opaque_values_silgen27forward_capture_generic_let4withSayxGx_tlF1fL_yylF : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> () // user: %3
  %3 = apply %2<U>(undef) : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> ()
  %4 = copy_value %0 : $U                         // users: %8, %5
  %5 = begin_borrow [lexical] %4 : $U             // users: %7, %6
  debug_value %5 : $U, let, name "x"              // id: %6
  end_borrow %5 : $U                              // id: %7
  destroy_value %4 : $U                           // id: %8
  unreachable                                     // id: %9
} // end sil function '$s20opaque_values_silgen27forward_capture_generic_let4withSayxGx_tlF'

// f #1 <A>() -> () in opaque_values_silgen.forward_capture_generic_let<A>(with: A) -> Swift.Array<A>
sil private [ossa] @$s20opaque_values_silgen27forward_capture_generic_let4withSayxGx_tlF1fL_yylF : $@convention(thin) <U> (@in_guaranteed U) -> () {
// %0 "x"                                         // users: %3, %1
bb0(%0 : @guaranteed $U):
  debug_value %0 : $U, let, name "x", argno 1     // id: %1
  // function_ref g #1 <A>() -> () in opaque_values_silgen.forward_capture_generic_let<A>(with: A) -> Swift.Array<A>
  %2 = function_ref @$s20opaque_values_silgen27forward_capture_generic_let4withSayxGx_tlF1gL_yylF : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> () // user: %3
  %3 = apply %2<U>(%0) : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> ()
  %4 = tuple ()                                   // user: %5
  return %4 : $()                                 // id: %5
} // end sil function '$s20opaque_values_silgen27forward_capture_generic_let4withSayxGx_tlF1fL_yylF'

// g #1 <A>() -> () in opaque_values_silgen.forward_capture_generic_let<A>(with: A) -> Swift.Array<A>
sil private [ossa] @$s20opaque_values_silgen27forward_capture_generic_let4withSayxGx_tlF1gL_yylF : $@convention(thin) <U> (@in_guaranteed U) -> () {
// %0 "x"                                         // users: %3, %1
bb0(%0 : @guaranteed $U):
  debug_value %0 : $U, let, name "x", argno 1     // id: %1
  // function_ref opaque_values_silgen.take<A>(A) -> ()
  %2 = function_ref @$s20opaque_values_silgen4takeyyxlF : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> () // user: %3
  %3 = apply %2<U>(%0) : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> ()
  %4 = tuple ()                                   // user: %5
  return %4 : $()                                 // id: %5
} // end sil function '$s20opaque_values_silgen27forward_capture_generic_let4withSayxGx_tlF1gL_yylF'

With opaque values and this patch:

// opaque_values_silgen.forward_capture_generic_let<A>(with: A) -> Swift.Array<A>
sil hidden [ossa] @$s20opaque_values_silgen27forward_capture_generic_let4withSayxGx_tlF : $@convention(thin) <U> (@in_guaranteed U) -> @owned Array<U> {
// %0 "u"                                         // users: %4, %1
bb0(%0 : @guaranteed $U):
  debug_value %0 : $U, let, name "u", argno 1     // id: %1
  // function_ref f #1 <A>() -> () in opaque_values_silgen.forward_capture_generic_let<A>(with: A) -> Swift.Array<A>
  %2 = function_ref @$s20opaque_values_silgen27forward_capture_generic_let4withSayxGx_tlF1fL_yylF : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> () // user: %3
  %3 = apply %2<U>(undef) : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> ()
  %4 = copy_value %0 : $U                         // users: %8, %5
  %5 = begin_borrow [lexical] %4 : $U             // users: %7, %6
  debug_value %5 : $U, let, name "x"              // id: %6
  end_borrow %5 : $U                              // id: %7
  destroy_value %4 : $U                           // id: %8
  unreachable                                     // id: %9
} // end sil function '$s20opaque_values_silgen27forward_capture_generic_let4withSayxGx_tlF'

// f #1 <A>() -> () in opaque_values_silgen.forward_capture_generic_let<A>(with: A) -> Swift.Array<A>
sil private [ossa] @$s20opaque_values_silgen27forward_capture_generic_let4withSayxGx_tlF1fL_yylF : $@convention(thin) <U> (@in_guaranteed U) -> () {
// %0 "x"                                         // users: %3, %1
bb0(%0 : @guaranteed $U):
  debug_value %0 : $U, let, name "x", argno 1     // id: %1
  // function_ref g #1 <A>() -> () in opaque_values_silgen.forward_capture_generic_let<A>(with: A) -> Swift.Array<A>
  %2 = function_ref @$s20opaque_values_silgen27forward_capture_generic_let4withSayxGx_tlF1gL_yylF : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> () // user: %3
  %3 = apply %2<U>(%0) : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> ()
  %4 = tuple ()                                   // user: %5
  return %4 : $()                                 // id: %5
} // end sil function '$s20opaque_values_silgen27forward_capture_generic_let4withSayxGx_tlF1fL_yylF'

// g #1 <A>() -> () in opaque_values_silgen.forward_capture_generic_let<A>(with: A) -> Swift.Array<A>
sil private [ossa] @$s20opaque_values_silgen27forward_capture_generic_let4withSayxGx_tlF1gL_yylF : $@convention(thin) <U> (@in_guaranteed U) -> () {
// %0 "x"                                         // users: %3, %1
bb0(%0 : @guaranteed $U):
  debug_value %0 : $U, let, name "x", argno 1     // id: %1
  // function_ref opaque_values_silgen.take<A>(A) -> ()
  %2 = function_ref @$s20opaque_values_silgen4takeyyxlF : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> () // user: %3
  %3 = apply %2<U>(%0) : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> ()
  %4 = tuple ()                                   // user: %5
  return %4 : $()                                 // id: %5
} // end sil function '$s20opaque_values_silgen27forward_capture_generic_let4withSayxGx_tlF1gL_yylF'

A let that isn't defined yet can be captured by a local function to which a closure is formed. Don't use an address type unless we're using lowered addresses.

While trying to reuse the liveness-points analysis originally in DI for injecting actor hops for more general purposes, Pavel and I discovered that the point at which we are injecting the hops might not have fully-computed the liveness information. That appears to be the case because we were computing the fully-initialized points before having processed destroy/releases of TheMemory. While this most likely had no influence on the actor hop injection, it does affect what the outgoing AvailabilitySet contains for a block. In particular, for this example: ```swift struct X { init(cond: Bool) { var _storage: (name: String, age: Int) _storage.name = "" if cond { _storage.age = 30 } else { _storage.age = 40 } } } ``` But because we are determine the full initialization points before processing the destroy, the liveness analysis doesn't iterate to correctly determine the out-availability of block 1 and 3 (corresponding to the then and else blocks in the example above). Here's the debug output showing that issue: ``` *** Definite Init looking at: %5 = mark_uninitialized [var] %4 : $*(name: String, age: Int) // users: %37, %12, %22, %32 Get liveness 0, #1 at assign %11 to %13 : $*String // id: %14 Get liveness 1, #1 at assign %21 to %23 : $*Int // id: %24 Get liveness for block 1 Iteration 0 Result: (yn) Get liveness 1, #1 at assign %31 to %33 : $*Int // id: %34 Get liveness for block 3 add block 2 to worklist Iteration 0 Block 2 out: (yn) Iteration 1 Block 2 out: (yn) Result: (yn) full-init-finder: rejecting bb0 b/c non-Yes OUT avail full-init-finder: rejecting bb1 b/c non-Yes OUT avail full-init-finder: rejecting bb2 b/c no non-load uses. full-init-finder: rejecting bb3 b/c non-Yes OUT avail full-init-finder: rejecting bb4 b/c no non-load uses. Get liveness 0, #2 at destroy_addr %5 : $*(name: String, age: Int) // id: %37 Get liveness for block 4 add block 3 to worklist add block 1 to worklist Iteration 0 Block 1 out: (yy) Block 3 out: (yy) Iteration 1 Block 1 out: (yy) Block 3 out: (yy) Result: (yy) ``` So, this patch basically just sinks the computation so it happens after, so that we force the incremental liveness analysis to also consider the liveness at the point of the destroy, but before having done any other transformations or modifications to the CFG to handle a destroy of something partially initialized.

Update bridging

Co-authored-by: Karoy Lorentey <[email protected]>

[Inclusive-Language] change comment with "sanity" to "soundness"

Local symbols

Add a new demangler option which excludes a closure's type signature. This will be used in lldb. Closures are not subject to overloading, and so the signature will never be used to disambiguate. A demangled closure is uniquely identifiable by its index(s) and parent. Where opaque types are involved, the concrete type signature can be quite complex. This demangling option allows callers to avoid printing the underlying complex nested concrete types. Example: before: `closure #1 (Swift.Int) -> () in closure #1 (Swift.Int) -> () in main` after: `closure #1 in closure #1 in main`

…wiftlang#73353) Add a new demangler option which excludes a closure's type signature. This will be used in lldb. Closures are not subject to overloading, and so the signature will never be used to disambiguate. A demangled closure is uniquely identifiable by its index(s) and parent. Where opaque types are involved, the concrete type signature can be quite complex. This demangling option allows callers to avoid printing the underlying complex nested concrete types. Example: before: `closure #1 (Swift.Int) -> () in closure #1 (Swift.Int) -> () in main` after: `closure #1 in closure #1 in main`

This inserts a suitably named function into the stack trace whenever a dynamic cast failure involves a NULL source or target type. Very often, crash logs include backtraces with function names but no log output; with this change, such a backtrace might look like the following -- note `TARGET_TYPE_NULL` in the function name here to mark the missing type information: ``` frame #0: __pthread_kill + 8 frame #1: pthread_kill + 288 frame #2: abort + 128 frame swiftlang#3: swift::fatalErrorv() frame swiftlang#4: swift::fatalError() frame swiftlang#5: swift_dynamicCastFailure_TARGET_TYPE_NULL() frame swiftlang#6: swift::swift_dynamicCastFailure() frame swiftlang#7: ::swift_dynamicCast() ``` Resolves rdar://130630157

…n's demangled name when processing it in RegionAnalysis. Just trying to improve logging to speed up triaging further. This is useful so that I can quickly find specific closures we process by using the closure numbering (e.x.: closure #1 in XXXX).

Two are fixes needed in most of the `RawSpan` and `Span` initializers. For example: ``` let baseAddress = buffer.baseAddress let span = RawSpan(_unchecked: baseAddress, byteCount: buffer.count) // As a trivial value, 'baseAddress' does not formally depend on the // lifetime of 'buffer'. Make the dependence explicit. self = _overrideLifetime(span, borrowing: buffer) ``` Fix #1. baseAddress needs to be a variable `span` has a lifetime dependence on `baseAddress` via its initializer. Therefore, the lifetime of `baseAddress` needs to include the call to `_overrideLifetime`. The override sets the lifetime dependency of its result, not its argument. It's argument still needs to be non-escaping when it is passed in. Alternatives: - Make the RawSpan initializer `@_unsafeNonescapableResult`. Any occurrence of `@_unsafeNonescapableResult` actually signals a bug. We never want to expose this annotation. In addition to being gross, it would totally disable enforcement of the initialized span. But we really don't want to side-step `_overrideLifetime` where it makes sense. We want the library author to explicitly indicate that they understand exactly which dependence is unsafe. And we do want to eventually expose the `_overrideLifetime` API, which needs to be well understood, supported, and tested. - Add lifetime annotations to a bunch of `UnsafePointer`-family APIs so the compiler can see that the resulting pointer is derived from self, where self is an incoming `Unsafe[Buffer]Pointer`. This would create a massive lifetime annotation burden on the `UnsafePointer`-family APIs, which don't really have anything to do with lifetime dependence. It makes more sense for the author of `Span`-like APIs to reason about pointer lifetimes. Fix #2. `_overrideLifetime` changes the lifetime dependency of span to be on an incoming argument rather than a local variables. This makes it legal to escape the function (by assigning it to self). Remember that self is implicitly returned, so the `@lifetime(borrow buffer)` tells the compiler that `self` is valid within `buffer`'s borrow scope.

Two are fixes needed in most of the `RawSpan` and `Span` initializers. For example: ``` let baseAddress = buffer.baseAddress let span = RawSpan(_unchecked: baseAddress, byteCount: buffer.count) // As a trivial value, 'baseAddress' does not formally depend on the // lifetime of 'buffer'. Make the dependence explicit. self = _overrideLifetime(span, borrowing: buffer) ``` Fix #1. baseAddress needs to be a variable `span` has a lifetime dependence on `baseAddress` via its initializer. Therefore, the lifetime of `baseAddress` needs to include the call to `_overrideLifetime`. The override sets the lifetime dependency of its result, not its argument. It's argument still needs to be non-escaping when it is passed in. Alternatives: - Make the RawSpan initializer `@_unsafeNonescapableResult`. Any occurrence of `@_unsafeNonescapableResult` actually signals a bug. We never want to expose this annotation. In addition to being gross, it would totally disable enforcement of the initialized span. But we really don't want to side-step `_overrideLifetime` where it makes sense. We want the library author to explicitly indicate that they understand exactly which dependence is unsafe. And we do want to eventually expose the `_overrideLifetime` API, which needs to be well understood, supported, and tested. - Add lifetime annotations to a bunch of `UnsafePointer`-family APIs so the compiler can see that the resulting pointer is derived from self, where self is an incoming `Unsafe[Buffer]Pointer`. This would create a massive lifetime annotation burden on the `UnsafePointer`-family APIs, which don't really have anything to do with lifetime dependence. It makes more sense for the author of `Span`-like APIs to reason about pointer lifetimes. Fix #2. `_overrideLifetime` changes the lifetime dependency of span to be on an incoming argument rather than a local variable. This makes it legal to escape the function (by assigning it to self). Remember that self is implicitly returned, so the `@lifetime(borrow buffer)` tells the compiler that `self` is valid within `buffer`'s borrow scope.

This is the missing check for "rule #1" in the isolated conformances proposal, which states that an isolated conformance can only be referenced within the same isolation domain as the conformance. For example, a main-actor-isolated conformance can only be used within main-actor code.

…g#81643) which generates IR without a llvm.trap function All the normal CI generated this: ``` ret i32 %1 } ; Function Attrs: cold noreturn nounwind memory(inaccessiblemem: write) declare void @llvm.trap() #1 attributes #0 = { "frame-pointer"= ``` But the test-simulator CI doesn't for some reason, so just check for the closing brace instead.

atrick added 28 commits March 18, 2022 14:00

Move -enable-sil-opaque-value to SILOptions.

db4bfcc

Add emitLoad/emitStore to OpaqueValue type lowering.

0897a69

SILModule::hasLoweredAddress

4ccb2e9

Add support for indirect tuple-type results.

4ceed0e

This could happen as a result of specialization or concrete address-only values. For now, it's just tested by SIL unit tests.

[SIL-opaque] Code review suggestions

bbf6177

Mostly documentation and typos.

Fix alloc_stack placement for open_existential.

862c7bc

Compute the latestOpeningInst, not the firstOpeningInst.

Rename "phi copy" to "phi move" for consistency with documentation.

222477a

In classic compiler terminology, this is a "phi copy" algorithm. But the documentation now tries to clearly distinguish between "semantics copies" vs. moves, where moves are "storage copies".

[SIL-opaque] avoid handling operands past 64k

2a9c305

[SIL-opaque] More file-level documentation

7c663a4

Explain high-level objectives and terminology with more precision.

[SIL-opaque] Add section-level comment explaining storage allocation

5181705

[SIL-opaque] remove cleanupAfterCall helper

f164c37

[SIL-opaque] add a test case for phi coalescing

bb0dbf3

Avoid attempting to coalesce enum payloads.

[SIL-opaque] in-depth top-level documentation for phi coalescing.

ba3e613

[SIL-opaque] minor NFC review feedback

752173c

[SIL-opaque] rename materialization functions.

04f4bcd

[SIL-opaque] rename initializeComposingUse

906bee3

[SIL-opaque] Removed [Unconditional]CheckedCastValue

97eec75

[SIL-opaque] Cleanup and reenable SILGen unit tests

1a0a92a

CHECK lines still need to be updated for OSSA

[SIL-opaque] Various SILGen fixes

d4bc86a

Add a test file for combined SILGen + AddressLowering

23516ec

[SIL-opaque] use generated SILLocations

06948a8

Anywhere that code is not obviously inserted immediately adjacent to the origin instruction.

Remove an unused test file (opaque_value_silgen_todo).

95add90

Update SILGen/opaque_values_silgen_lib.swift for [serialized]

390ab4d

The function attribute's name changed on main.

[SIL-opaque] Add @in_guaranteed function argument support.

434faee

Temporarily map storage to a fake load_borrow.

[SIL-opaque] Add an assert for open_existential_value.

d6f0c73

Add comments. Add a basic dominance sanity check.

[SIL-opaque] [NFC] clang-format AddressLowering.cpp

2d53e03

[SIL-opaque] Add address lowering test case.

757a9d2

For borrowing a projection.

nate-chandler force-pushed the opv-silgen/pr/forward-capture-let branch from 49931d7 to 25704eb Compare March 21, 2022 19:10

[SIL-opaque] Handled forward captures.

3266969

A let that isn't defined yet can be captured by a local function to which a closure is formed. Don't use an address type unless we're using lowered addresses.

nate-chandler force-pushed the opv-silgen/pr/forward-capture-let branch from 25704eb to 3266969 Compare March 21, 2022 19:13

atrick force-pushed the opv-silgen branch 3 times, most recently from 9258297 to 05fb8c7 Compare March 23, 2022 01:10

atrick deleted the branch atrick:opv-silgen March 23, 2022 19:02

atrick closed this Mar 23, 2022

nate-chandler deleted the opv-silgen/pr/forward-capture-let branch July 5, 2023 23:33

atrick pushed a commit that referenced this pull request Nov 14, 2023

Merge pull request #1 from Genaro-Chris/Genaro-Chris-patch-1

49ea89d

Update bridging

atrick pushed a commit that referenced this pull request Jan 20, 2024

Take doc suggestion from Karoy #1

f28ebc9

Co-authored-by: Karoy Lorentey <[email protected]>

atrick pushed a commit that referenced this pull request Feb 5, 2024

Merge pull request #1 from Serena748/Serena748/inclusive-language

141b4d0

[Inclusive-Language] change comment with "sanity" to "soundness"

atrick pushed a commit that referenced this pull request Feb 22, 2024

Merge pull request #1 from skrtks/local-symbols

b97c636

Local symbols

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[SIL-opaque] Handled forward captures. #1

[SIL-opaque] Handled forward captures. #1

Uh oh!

nate-chandler commented Mar 21, 2022

Uh oh!

nate-chandler commented Mar 21, 2022

Uh oh!

Uh oh!

[SIL-opaque] Handled forward captures. #1

[SIL-opaque] Handled forward captures. #1

Uh oh!

Conversation

nate-chandler commented Mar 21, 2022

Uh oh!

nate-chandler commented Mar 21, 2022

Uh oh!

Uh oh!