Commit Graph

2070 Commits

Author SHA1 Message Date
Michael Gottesman
4c2dc5eac4 Merge pull request #73930 from gottesmm/nonisolatedunsafe-rdar128299305
[region-isolation] Add missing support for nonisolated(unsafe)
2024-05-28 20:46:00 -07:00
Michael Gottesman
74ac12c9d2 [region-isolation] Make temporary alloc_stack that we form for returning values from a non-final class field take on the class method's isolation.
The reason why we are doing this is that otherwise, we have that the alloc_stack
formed for the result is disconnected and despite the fact that we merge it into
the actor region of the class method, we do not have that the alloc_stack
specifically is marked when we attempt to squelch Please.

This patch fixes that problem by detecting when an alloc_stack is being used as
a temporary for an out parameter and makes the alloc_stack initially isolated as
appropriate. It only does this in the specific cases where we can pattern match
it which in my limited testing has handled everything.
2024-05-28 17:31:08 -07:00
Tim Kientzle
65f78e6268 Merge pull request #73675 from tbkka/tbkka-assertions-v2
New assertions support
2024-05-28 17:25:21 -07:00
Michael Gottesman
2de13df909 [region-isolation] Use SILIsolationInfo::initializeTrackableValue instead of SILIsolationInfo::mergeIsolationRegionInfo to fix last issue
When merging SILIsolationInfo for regions, we want to drop
nonisolated(unsafe). This is important since nonisolated(unsafe) should only
apply to the specific "value" that it belongs to, not the entire region.

This creates a problem since in a few places in the code base we initialize a
value (producing a disconnected value) and then initialize it by merging in an
actor isolation. This no longer work since we will then always have
nonisolated(unsafe) stripped, so no values would ever be considered to be
nonisolated(unsafe). After analyzing the use case, I realized that these were
just initialization patterns and in this commit, I added a specific
initialization operation called SILIsolationInfo::initializeTrackableValue and
eliminated those calls to SILIsolationInfo::mergeIsolationRegionInfo.

Since SILIsolationInfo no longer has any merge operation on it, I then
eliminated that code in this commit. This completes the behavior split that I
put into the type system in the last commit. Specifically, I defined a
composition type called SILDynamicMergedIsolationInfo. It represents a
SILIsolationInfo that has been merged... that is why I called it the
DynamicMergedIsolationInfo. It could probably use a better name = (.

This fixes one of the last weird test case that I wrote where we were not letting through valid
nonisolated(unsafe) code.

At the same time, I discovered an additional issue (which can be seen in the
TODOs in this commit), where we are being too conservative around a non-Sendable
class var field. I am going to fix that in the next commit.

rdar://128299305
2024-05-27 21:42:15 -07:00
Michael Gottesman
1bef011e48 [region-isolation] Add the ability for the analysis to emit "unknown error" partition ops in case we detect a case we cannot pattern match.
DISCUSSION: The analysis itself is unable to emit errors. So we achieve the same
functionality by in such cases emitting a partition op that signals to our user
that when they process that partition op they should emit an "unknown pattern"
error at the partition op's instructions.

I have wanted this for a long time, but I never got around to it.
2024-05-27 21:42:04 -07:00
Michael Gottesman
741244e16b [region-isolation] Split in the type system SILIsolationInfo that has been merged and those that haven't.
Specifically, I introduced a new composition type called
SILDynamicMergedIsolationInfo that just contains a
SILIsolationInfo. Importantly, whenever one merges a SILIsolationInfo with
another SILIsolationInfo, one gets back a SILDynamicMergedIsolationInfo.

The reason why I am doing this is that we drop nonisolated(unsafe) when merging
so I want to ensure that parts of the code that use merging (where the dropping
occurs) and normal SILIsolationInfo where we do not want to merge is
distinguished.
2024-05-27 21:41:54 -07:00
Michael Gottesman
3a1f58a72a [region-isolation] Make sure that nonisolated(unsafe) works in all cases.
I made sure we match what we get without region isolation by turning off region
isolation in one of the test runs on the test for this.

There is one problem where for non-final classes with nonisolated(unsafe) var
fields, we currently do not properly squelch since I need to do more
infrastructure work. I am going to do that in the next commit.

rdar://128299305
2024-05-27 21:41:32 -07:00
Michael Gottesman
89a2cfce0b [region-isolation] Initialize TrackableValueState's regionInfo with a .none instead of a disconnected region.
The design change here is that instead of just initializing the regionInfo with
disconnected, we set it as .none and if we see .none, just return a newly
construct disconnected isolation region info when getIsolationRegionInfo() is
called.

This enables us to provide a setIsolationRegionInfo() helper for
RegionAnalysisValueMap::getTrackableValue that does not perform a merge. This is
important since for nonisolated(unsafe), we want to not have nonisolated(unsafe)
propagate through merging. So if we use merging to initialize the internal
regionInfo state of a SILIsolationInfo, we will never have a SILIsolationInfo
with that bit set since it will be lost in the merge. So we need some sort of
other assignment operator. Noting that we should only compute a value's
SILIsolationInfo once in RegionAnalysisValueMap before we cache it in the map,
it made sense to just represent it as an optional that way we can guarantee that
the regionInfo is only ever set exactly once by that routine.
2024-05-27 21:28:34 -07:00
Michael Gottesman
3f26d08ee4 [region-isolation] Add the ability in SILIsolationInfo to represent a disconnected value that is nonisolated(unsafe). 2024-05-27 21:25:44 -07:00
Michael Gottesman
4b29089f8b [region-isolation] Add a little bit of documentation to ActorInstance. 2024-05-27 21:21:52 -07:00
Michael Gottesman
3597006200 [region-isolation] Move SILIsolationInfo out of PartitionUtils into its own header/cpp file.
SILIsolationInfo is conceptually a layer that composes with PartitionUtils so it
makes sense for it to be in a different file.
2024-05-27 20:37:08 -07:00
Ellie Shin
5ccc4cd394 SIL function can be serialized with different kinds: [serialized] or
[serialized_for_package] if Package CMO is enabled. The latter kind
allows a function to be serialized even if it contains loadable types,
if Package CMO is enabled. Renamed IsSerialized_t as SerializedKind_t.

The tri-state serialization kind requires validating inlinability
depending on the serialization kinds of callee vs caller; e.g. if the
callee is [serialized_for_package], the caller must be _not_ [serialized].
Renamed `hasValidLinkageForFragileInline` as `canBeInlinedIntoCaller`
that takes in its caller's SerializedKind as an argument. Another argument
`assumeFragileCaller` is also added to ensure that the calle sites of
this function know the caller is serialized unless it's called for SIL
inlining optimization passes.

The [serialized_for_package] attribute is allowed for SIL function, global var,
v-table, and witness-table.

Resolves rdar://128406520
2024-05-23 15:53:02 -07:00
Kshitij Jain
64da348ee9 Merge pull request #73688 from jkshtj/main
[Autodiff] Adds logic to rewrite call-sites using functions specialized by the closure-spec optimization
2024-05-23 14:21:31 -07:00
Kshitij
12faf79911 [Autodiff] Adds logic to rewrite call-sites using functions specialized by the closure-spec optimization 2024-05-21 12:02:28 -07:00
Slava Pestov
a1462ef184 SIL: Promote removeDeadBlock() from SILOptimizer to a method on SILBasicBlock 2024-05-21 13:52:58 -04:00
Michael Gottesman
d759ec97ea Merge pull request #73696 from gottesmm/rdar128216574
[sending] Add support for 'sending'
2024-05-18 05:42:41 -04:00
Tim Kientzle
f09fb7dfa4 Update a couple of files to pull assertion helpers from the new header 2024-05-16 12:50:23 -07:00
Erik Eckstein
cc78c8f094 Optimizer: add Context.canMakeStaticObjectReadOnly API 2024-05-16 21:34:35 +02:00
Michael Gottesman
e3e78ad6bb [sending] Change the internals of sending to be based around 'sending' instead of 'transferring'.
We still only parse transferring... but this sets us up for adding the new
'sending' syntax by first validating that this internal change does not mess up
the current transferring impl since we want both to keep working for now.

rdar://128216574
2024-05-16 12:20:45 -07:00
Kshitij Jain
c37d31cc64 Merge pull request #73596 from jkshtj/specialize
[Autodiff] Adds logic to generate specialized functions in the closure-spec pass
2024-05-14 18:55:23 -07:00
Kshitij
ce2161ee43 [Autodiff] Fixes build issues on Linux 2024-05-13 19:25:18 -07:00
Kshitij
74166a4ab6 [Autodiff] Moves bridging code accesses in closure-spec opt behind APIs
Addresses some other surfacial feedback as well.
2024-05-13 15:37:30 -07:00
Michael Gottesman
fe2dc11cb9 [region-isolation] When computing isolation for isolated parameters, use getAnyActor instead of getNominalOrBoundGenericNominal().
This ensures that we can properly compute isolation for generic types that
conform to AnyActor.

I found this by playing with test cases from the previous commit. We would not
find an actor type for the actor instance isolation and would fall back along an
incorrect path.

rdar://128021548
2024-05-13 13:43:51 -07:00
Kshitij
ab751d57ab [Autodiff] Adds logic to generate specialized functions in the closure-spec pass 2024-05-13 11:16:42 -07:00
Michael Gottesman
35a5a2546a [region-isolation] Fix isolation inference for init accessors.
rdar://127844737
2024-05-10 15:33:44 -07:00
Michael Gottesman
50c2d678f2 [region-isolation] When inferring isolation for an argument, handle non-self isolated parameters as well as self parameters that are actor isolated.
As part of this I went through how we handled inference and rather than using a
grab-bag getActorIsolation that was confusing to use, I created split APIs for
specific use cases (actor instance, global actor, just an apply expr crossing)
that makes it clearer inside the SILIsolationInfo::get* APIs what we are
actually trying to model. I found a few issues as a result and fixed most of
them if they were small. I also fixed one bigger one around computed property
initializers in the next commit. There is a larger change I didn't fix around allowing function
ref/partial_apply with isolated self parameters have a delayed flow sensitive
actor isolation... this will be fixed in a subsequent commit.

This also fixes a bunch of cases where we were printing actor-isolated instead
of 'self' isolated.

rdar://127295657
2024-05-10 15:33:44 -07:00
Michael Gottesman
9bfb3b7ee7 [region-isolation] Some small gardening updates in preparation for the next commit.
Specifically, I added a few helper methods and improved the logging printing.
This all makes the next commit a more focused commit.
2024-05-10 15:33:44 -07:00
Kshitij Jain
01654fd323 Merge pull request #71775 from jkshtj/main
[Autodiff] Adds part of the Autodiff specific closure-specialization optimization pass
2024-05-10 08:51:01 -07:00
Daniel Rodríguez Troitiño
c6745e6d50 Fix Swift Compiler Modules support in Debug mode (#73506)
The new `swift-driver` seems to enqueue a `wrapmodule` job which uses
the given `module-name` to form the output file name when not doing
optmizations (seems to happen only for `-Onone` in my testing). Since the
CMake functions macros are using the module name also as the explicit output
name, this clashes and ends up in an unhelpful error message from the driver.

```
SwiftDriverExecution/MultiJobExecutor.swift:207: Fatal error: multiple
producers for output ... SwiftCompilerSources/Basic.o: Wrapping Swift
module Basic & Compiling Basic SourceLoc.swift
```

This was reported in https://forums.swift.org/t/debug-swift-build-fails/71380

The changes use a different output object name (by using `.object.o`
suffix) which does not clash with what the `swift-driver` does
automatically. The code around the output objects and the static
libraries have to change slightly to handle this case.

Additionally, the resulting library when in `Debug` is now declaring its
dependency on `swiftSwiftOnoneSupport`, to avoid linking errors when the
libraries are used in the final binaries.

Debug mode seems to enable PURE_BRIDGING_MODE, which seems to skip
transitively including some C headers that files like
`Utilities/Test.swift` depend on. To avoid errors building, add the
missing include in a new `#else` branch.

I think CI will not test the `Debug` mode, so the only thing that it can prove is
that these changes do not break the `Release` mode.
2024-05-08 15:35:29 -07:00
Kshitij
fd609846ae Makes the swift-based closure-spec pass an experimental frontend feature 2024-05-02 09:14:05 -07:00
Kshitij
c8375c06ae [Autodiff] Adds part of the closure-specialization optimization pass
Changes in this CR add part of the, Swift based, Autodiff specific
closure specialization optimization pass. The pass does not modify any
code nor does it even exist in any of the optimization pipelines. The
rationale for pushing this partially complete optimization pass upstream
is to keep up with the breaking changes in the underlying Swift based
compiler infrastructure.
2024-05-02 09:14:05 -07:00
Kshitij
a7f8d6c647 [Autodiff] Adds bridging code in preparation for the Swift based Autodiff closure-spec pass 2024-05-02 09:14:05 -07:00
Emil Pedersen
a31d15c438 Merge pull request #72964 from Snowy1803/mem2reg-salvage-store
[DebugInfo] Salvage all stores
2024-04-25 16:52:37 -07:00
Emil Pedersen
523a769e34 [DebugInfo] [Mem2Reg] Move debug info promotion to salvageDebugInfo 2024-04-23 13:12:21 -07:00
Michael Gottesman
c4f7076baf [region-isolation] When RegionIsolation is enabled, delay the preconcurrency import not used diagnostic to the SIL pipeline.
The reason why I am doing this is that I am going to be adding support for
preconcurrency imports to TransferNonSendable. That implies that we can have
preconcurrency import suppression in the SIL pipeline and thus that emitting the
diagnostic in Sema is too early.

To do this, I introduced a new module pass called
DiagnoseUnnecessaryPreconcurrencyImports that runs after the SILFunction pass
TransferNonSendable. The reason why I use a module pass is to ensure that
TransferNonSendable has run on all functions before we attempt to emit these
diagnostics. Then in that pass, we iterate over all of the modules functions and
construct a uniqued array of SourceFiles for these functions. Then we iterate
over the uniqued SourceFiles and use the already constructed Sema machinery to
emit the diagnostic using the source files.

rdar://126928265
2024-04-23 12:42:43 -05:00
Michael Gottesman
1113a61f8d [region-isolation] Stub out getIsolationInfo so the mocking unit tests succeed.
Specifically, the partition unit tests pass in bogus instructions/operands so we
cannot call /any/ methods on them. So I created stubed out helpers on the
evaluator that in the case of mocking just return a default initialized
SILIsolationInfo().
2024-04-19 15:28:48 -07:00
Michael Gottesman
0e21c58619 [region-isolation] Disconnected regions in global actor isolated function are not accessible again after call to nonisolated async function
I also fixed the nonisolated(unsafe) issue.

rdar://126667934
rdar://126174959
2024-04-19 13:52:58 -07:00
Michael Gottesman
3c29997cd1 [region-isolation] Out of an abundance of caution convert isActor -> isAnyActor(). 2024-04-17 13:07:56 -07:00
Michael Gottesman
2e5b3bc257 [region-isolation] Do not treat functions/class_methods that are isolated to a #isolated as being globally isolated to that.
Instead, we need to consider the isolation at the apply site.

rdar://126285681
rdar://125078448
2024-04-11 16:00:55 -07:00
Michael Gottesman
51ef67d7df [variable-name-utils] Refactor inferName/inferNameAndRoot helpers onto VariableNameInferrer.
I have been using these in TransferNonSendable and they are useful in terms of
reducing the amount of code that one has to type to use this API. I am going to
need to use it in SILIsolationInfo, so it makes sense to move it into
SILOptimizer/Utils.

NFCI.
2024-04-11 15:41:18 -07:00
Michael Gottesman
d8f39f70d9 [region-isolation] Begin tracking in SILIsolationInfo the actorInstance that a value is isolated to if we are dealing with an actor instance.
This will let us distinguish in between values derived from two actor instances
of the same type and to emit better errors.
2024-04-11 15:41:18 -07:00
Michael Gottesman
b407b21e9a [region-isolation] Finish moving isolation computation into SILIsolationInfo::get. 2024-04-11 15:41:18 -07:00
Michael Gottesman
513ab78602 [region-isolation] Move SILIsolationInfo determining code for ref_element_addr and global_addr onto SILIsolationInfo and call that instead. 2024-04-11 15:41:18 -07:00
Michael Gottesman
b80faee2a7 [region-isolation] Rename Partition::{fresh_label,freshLabel}. 2024-04-11 15:41:18 -07:00
Michael Gottesman
c9fe8ff935 [region-isolation] Eliminate unnecessary using TrackableValueID = Element.
Having two artificial typedefs for the same wrapped value is just confusing.
Better to just have one and make the code simpler to understand.
2024-04-11 15:41:18 -07:00
Michael Gottesman
beaab91376 [region-isolation] Add a small helper to TrackableValue to print it's isolation info.
I am finding that I am calling that a bunch so this just makes it a little more
convenient.
2024-04-11 15:41:18 -07:00
Michael Gottesman
df4fb64ea1 Merge pull request #72955 from gottesmm/rdar126170014
[region-isolation] Include the region -> transferring operand map in the dataflow convergence.
2024-04-10 17:20:55 -07:00
Michael Gottesman
d2bdec2acd [region-isolation] Include the region -> transferring operand map in the dataflow convergence.
The reason why I am doing this is that really we are running two different
dataflow equations at the same time... one for propagating tracking transferring
sets and the other for propagating regions. Since at the source level the two
dataflow problems are very interrelated, I was unable to come up with an example
where we fail to iterate because of this, but I would like to be sure that we do
not hit one, so I am fixing this here.

rdar://126170014
2024-04-10 10:36:42 -07:00
Michael Gottesman
ca8179aa7c [region-isolation] Track operand info in a separate map rather than inline in a TransferringOperand data structure.
This is backing out an approach that I thought would be superior, but ended up
causing problems.

Originally, we mapped a region number to an immutable pointer set containing
Operand * where the region was tranferred. This worked great for a time... until
I began to need to propagate other information from the transferring code in the
analysis to the actual diagnostic emitter.

To be able to do that, my thought was to make a wrapper type around Operand
called TransferringOperand that contained the operand and the other information
I needed. This seemed to provide me what I wanted but I later found that since
the immutable pointer set was tracking TransferringOperands which were always
newly wrapped with an Operand *, we actually always created new pointer
sets. This is of course wasteful from a memory perspective, but also prevents me
from tracking transferring operand sets during the dataflow since we would never
converge.

In this commit, I fix that issue by again tracking just an Operand * in the
TransferringOperandSet and instead map each operand to a state structure which
we merge dataflow state into whenever we visit it. This provides us with
everything we need to in the next commit to including a region -> transferring
operand set equality check in our dataflow equations and always converge.
2024-04-10 10:30:10 -07:00
Erik Eckstein
e14c1d1f62 SIL, Optimizer: update and handle borrowed-from instructions
Compute, update and handle borrowed-from instruction in various utilities and passes.
Also, used borrowed-from to simplify `gatherBorrowIntroducers` and `gatherEnclosingValues`.
Replace those utilities by `Value.getBorrowIntroducers` and `Value.getEnclosingValues`, which return a lazily computed Sequence of borrowed/enclosing values.
2024-04-10 13:38:10 +02:00