Skip to content

Conversation

@gottesmm
Copy link
Contributor

@gottesmm gottesmm commented Nov 7, 2023

This PR contains 2 commits that create infrastructure and a final infrastructure that actually implements the subject of this radar. I did this to ease reviewing.

The purpose of this PR is that it ensures that diagnostic passes like TransferNonSendable can discern in between full assignments to tuples vs assignments to projects of the tuple. Before, SILGen would perform tuple assignment by performing it in pieces. This PR introduces a new instruction called tuple_addr_constructor that is equivalent to those in pieces assignments and provides the one memory writing instruction that such diagnostic passes can understand are an assignment over the entire tuple.

I included the full message for the main patch (#3) at the bottom of the PR. Below I included short high level abstracts of the patches:

  1. I added support for the tuple_addr_constructor instruction. This instruction takes in a tuple address (that may have multiple tuple levels) and initializes its tuple leaf elements with the arguments of the instruction. The arguments may be objects or addresses. This mimics how RValue initializes output and was done this way to ensure that we can easily handle complex tuples with arguments and empty tuple members. Semantically it acts like a specialized copy_addr [take]/store. Thus from a memory effect perspective it has side-effects and the behavior of copy_addr/store.
  2. I added a small pass that runs after TransferNonSendable that lowers tuple_addr_constructor. It does this by emitting the relevant tuple_element_addr + copy_addr/store as appropriate. This ensures that I did not need to update huge amounts of the pass pipeline in order to fix this issue.
  3. In this patch I changed RValue::assignInto to use the new instruction and updated DI/TransferNonSendable/other parts of the optimizer to handle this instruction. As part of this, I updated the relevant diagnostic test in TransferNonSendable that show we properly identify tuple assignments vs assignments to their projections.

[region-isolation] When assigning RValues into memory, use tuple_addr_constructor instead of doing it in pieces.

I also included changes to the rest of the SIL optimizer pipeline to ensure that
the part of the optimizer pipeline before we lower tuple_addr_constructor (which
is right after we run TransferNonSendable) work as before.

The reason why I am doing this is that this ensures that diagnostic passes can
tell the difference in between:

x = (a, b, c)

and

x.0 = a
x.1 = b
x.2 = c

This is important for things like TransferNonSendable where assigning over the
entire tuple element is treated differently from if one were to initialize it in
pieces using projections.

rdar://117880194

…nitial a tuple in memory from individual address and object components.

This commit just introduces the instruction. In a subsequent commit, I am going
to add support to SILGen to emit this. This ensures that when we assign into a
tuple var we initialize it with one instruction instead of doing it in pieces.
The problem with doing it in pieces is that when one is emitting diagnostics it
looks semantically like SILGen actually is emitting code for initializing in
pieces which could be an error.
…and eliminates tuple addr constructor.

This will limit the number of passes that need to be updated to handle
tuple_addr_constructor.
@gottesmm gottesmm requested review from kavon and ktoso as code owners November 7, 2023 01:28
@gottesmm
Copy link
Contributor Author

gottesmm commented Nov 7, 2023

@swift-ci smoke test

@gottesmm gottesmm requested a review from slavapestov November 7, 2023 02:57
…_constructor instead of doing it in pieces.

I also included changes to the rest of the SIL optimizer pipeline to ensure that
the part of the optimizer pipeline before we lower tuple_addr_constructor (which
is right after we run TransferNonSendable) work as before.

The reason why I am doing this is that this ensures that diagnostic passes can
tell the difference in between:

```
x = (a, b, c)
```

and

```
x.0 = a
x.1 = b
x.2 = c
```

This is important for things like TransferNonSendable where assigning over the
entire tuple element is treated differently from if one were to initialize it in
pieces using projections.

rdar://117880194
@gottesmm
Copy link
Contributor Author

gottesmm commented Nov 7, 2023

@swift-ci smoke test

@gottesmm gottesmm merged commit 48b4ca0 into swiftlang:main Nov 8, 2023
@gottesmm gottesmm deleted the rdar117880194 branch November 8, 2023 04:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants