We may see undef closure captures in ClosureLifetimeFixup since
it is a mandatory pass that runs on invalid code as well.
This could sometimes hang the compiler while running `insertDeallocOfCapturedArguments`
in release builds.
This change adds a bailout when we see an undef closure capture to avoid running into this issue.
The InstructionDeleter can remove instructions including their destroys and then insert compensating destroys at a new place.
This is effectively destroy-hoisting which doesn't respect deinit-barriers. Therefore it's not done for lexical lifetimes.
However, since https://github.com/swiftlang/swift/pull/85334, the optimizer should treat _all_ lifetimes as fixed and not only lexical lifetimes.
This change adds a `assumeFixedLifetimes` flag to InstructionDeleter which is on by default.
Only mandatory passes (like OSLogOptimization) should turn this off.
Specifically, improved debug info retention in:
* tryReplaceRedundantInstructionPair,
* splitAggregateLoad,
* TempLValueElimination,
* Mem2Reg,
* ConstantFolding.
The changes to Mem2Reg allow debug info to be retained in the case tested by
self-nostorage.swift in -O builds, so we have just enabled -O in that file
instead of writing a new test for it.
We attempted to add a case to salvageDebugInfo for unchecked_enum_data, but it
caused crashes in Linux CI that we were not able to reproduce.
We cannot compute the liverange of a value if it bit-wise escapes.
This fixes a mis-compile in copy-propagation which hoists a destroy_value over a use of the escaped value when lexical liveranges are disabled.
The test case is a simplified SIL sequence from the stdlib core where this problem shows up, because we build the stdlib core with disabled lexical liveranges.
This peephole optimization didn't consider that an alloc_stack of an enum can be overridden by another value.
The fix is to remove this peephole optimization at all because it is already covered by `optimizeEnum` in alloc_stack simplification.
Fixes a miscompile
https://github.com/swiftlang/swift/issues/85687
rdar://165374568
This code was not consulting SILFunctionConventions, so it was
passing things indirect when they are not in sil-opaque-values.
This fixes callers as well to ensure they pass arguments and
results correctly in both modes.
We are creating/relying on a contract between the AST and SIL... that SILDeclRef
should accurately describe the method/accessor that a class_method is from. By
doing this we eliminate pattern matching on the AST which ties this code too
tightly to the AST and makes it brittle in the face of AST changes. This also
fixes an issue where we were not handling setters correctly.
I am doing this now since it is natural to fix it along side fixing the
ref_element_addr issue in the previous commit since they are effectively doing
the same thing.
rdar://153207557
The two pieces of code are fundamentally doing the same thing so I can reuse the
code. I am doing the refactoring as a separate change so that it is easier to
review.
The previous algorithm was doing an iterative forward data flow analysis
followed by a reverse data flow analysis. I suspect the history here is that
it was a reverse analysis, and that didn't really work for infinite loops,
and so complexity accumulated.
The new algorithm is quite straightforward and relies on the allocations
being properly jointly post-dominated, just not nested. We simply walk
forward through the blocks in consistent-with-dominance order, maintaining
the stack of active allocations and deferring deallocations that are
improperly nested until we deallocate the allocations above it. The only
real subtlety is that we have to delay walking into dead-end regions until
we've seen all of the edges into them, so that we can know whether we have
a coherent stack state in them. If the state is incoherent, we need to
remove any deallocations of previous allocations because we cannot talk
correctly about what's on top of the stack.
The reason I'm doing this, besides it just being a simpler and hopefully
faster algorithm, is that modeling some of the uses of the async stack
allocator properly requires builtins that cannot just be semantically
reordered. That should be somewhat easier to handle with the new approach,
although really (1) we should not have runtime functions that need this and
(2) we're going to need a conservatively-correct solution that's different
from this anyway because hoisting allocations is *also* limited in its own
way.
I've attached a rather pedantic proof of the correctness of the algorithm.
The thing that concerns me most about the rewritten pass is that it isn't
actually validating joint post-dominance on input, so if you give it bad
input, it might be a little mystifying to debug the verifier failures.
This instruction can be used to disable ownership verification on it's result and
will be allowed only in raw SIL.
Sometimes SILGen can produce invalid ownership SSA, that cannot be resolved until
mandatory passes run. We have a few ways to piecewise disable verification.
With unchecked_ownership instruction we can provide a uniform way to disable ownership
verification for a value.
Specifically given a nominal type like the following:
```swift
@MainActor
struct Foo {
@CustomActor var ns: NonSendableKlass
}
```
the isolation for ns should be CustomActor not MainActor.
rdar://160603379
The caller is allowed to assume that the 'inout sending' parameters are not in
the same region on return so can be sent to different isolation domains safely.
To enforce that we have to ensure on return that the two are /actually/ not in
the same region.
rdar://138519484
This instruction converts Builtin.ImplicitActor to Optional<any Actor>. In the
process of doing so, it masks out the bits we may have stolen from the witness
table pointer of Builtin.ImplicitActor. The bits that we mask out are the bottom
two bits of the top nibble of the TBI space on platforms that support TBI (that
is bit 60,61 on arm64). On platforms that do not support TBI, we just use the
bottom two tagged pointer bits (0,1).
By using an instruction, we avoid having to represent the bitmasking that we are
performing at the SIL level and can instead just make the emission of the
bitmasking an IRGen detail. It also allows us to move detection if we are
compiling for AArch64 to be an IRGen flag instead of a LangOpts flag.
The instruction is a guaranteed forwarding instruction since we want to treat
its result as a borrowed projection from the Builtin.ImplicitActor.
The previous commit in this PR exposed that we were not handling this correctly.
Specifically, we incorrectly started to error in SwiftFoundation.
rdar://162629359
Specifically, this code was added because otherwise we would in swift 5 +
strict-concurrency mode emit two warnings, one at the AST level and one at the
SIL level. Once we are in swift-6 mode, this does not happen since we stop
compiling at the AST level since we will emit the AST level diagnostic as an
error.
To do this, we tried to pattern match what the AST was erroring upon and treat
the parameter as disconnected instead of being isolated. Sadly, this resulted in
us treating certain closure cases incorrectly and not emit a diagnostic
(creating a concurrency hole).
Given that this behavior results in a bad diagnostic only to avoid emitting two
diagnostics in a mode which is not going to last forever... it really doesn't
make sense to keep it. We really need a better way to handle these sorts of
issues. Perhaps a special semantic parameter put on the function that squelches
certain errors. But that is something for another day. The specific case it
messes up is:
```
class NonSendable {
func action() async {}
}
@MainActor
final class Foo {
let value = NonSendable()
func perform() {
Task { [value] in
await value.action() // Should emit error but do not.
}
}
}
```
In this case, we think that value is sending... when it isnt and we should emit
an error.
rdar://146378329
A dead `destructure_struct` with an owned argument can appear for a non-copyable or non-escapable struct which has only trivial elements.
The instruction is not trivially dead because it ends the lifetime of its operand.
Fixes an ownership verification error.
The intent for `@inline(always)` is to act as an optimization control.
The user can rely on inlining to happen or the compiler will emit an error
message.
Because function values can be dynamic (closures, protocol/class lookup)
this guarantee can only be upheld for direct function references.
In cases where the optimizer can resolve dynamic function values the
attribute shall be respected.
rdar://148608854
