[SIL-opaque] Don't override arg value category. #2
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
Previously, when emitting block arguments, the value category of the SILType was overridden to be address for indirect arguments. With opaque types, that distinction is made later during AddressLowering. So only do that when opaque types are disabled.
|
Without opaque values: With opaque values and this patch: |
atrick
force-pushed
the
opv-silgen
branch
3 times, most recently
from
9258297 to
05fb8c7
Compare
atrick
pushed a commit
that referenced
this pull request
Sep 26, 2022
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.
atrick
pushed a commit
that referenced
this pull request
Sep 18, 2023
# This is the 1st commit message: utils: update the build-windows-toolchain.bat to extract the toolchain Fetch a prebuilt toolchain to build the toolchain. This is required to enable the macro support on Windows. # The commit message #2 will be skipped: # build: build SwiftSyntax before the toolchain build # # Perform a build of Swift Syntax prior to the build of the toolchain so # that we can enable the early swift syntax parser builds. This is a # prerequisite for enabling macros on Windows. # The commit message swiftlang#3 will be skipped: # # This is a combination of 5 commits. # # This is the 1st commit message: # # build: wire up the early swift-syntax build to the build # # This enables the early swift syntax build to get us macro support on # Windows. # # # The commit message #2 will be skipped: # # # Update build-windows-toolchain.bat # # # The commit message swiftlang#3 will be skipped: # # # Update build-windows-toolchain.bat # # # The commit message swiftlang#4 will be skipped: # # # Update build-windows-toolchain.bat # # # The commit message swiftlang#5 will be skipped: # # # Update build-windows-toolchain.bat
atrick
pushed a commit
that referenced
this pull request
Jan 20, 2024
Co-authored-by: Karoy Lorentey <[email protected]>
atrick
pushed a commit
that referenced
this pull request
Jul 13, 2024
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
atrick
added a commit
that referenced
this pull request
Dec 4, 2024
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.
atrick
added a commit
that referenced
this pull request
Dec 4, 2024
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.
atrick
added a commit
that referenced
this pull request
Dec 4, 2024
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.
atrick
added a commit
that referenced
this pull request
Dec 12, 2024
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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, when emitting block arguments, the value category of the SILType was overridden to be address for indirect arguments. With opaque types, that distinction is made later during AddressLowering. So only do that when opaque types are disabled.