Skip to content

5.9: [TempRValueOpt] Fold new destroy_addrs into loads. #65070

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

nate-chandler
Copy link
Contributor

@nate-chandler nate-chandler commented Apr 11, 2023

Cherry-pick of #65052 .

To avoid introducing new copies--which is illegal for move-only values-- don't rewrite load [take]s and copy_addr [take]s as load [copy]s and copy_addrs respectively and introduce new destroy_addrs after them. Instead, get the effect of folding such a newly created destroy_addr into the preceding rewritten load [copy] or copy_addr. Get that effect by neither modifying the copy_addr [take] or load [take] nor adding a subsequent destroy_addr.

An example for each kind (load [take] and copy_addr [take]):

// Input 1 (`load [take]`)
  copy_addr [take] %src to [init] %tmp
  %val = load [take] %src
// Old Output 1
  %val = load [copy] %src
  destroy_addr %src
// New Output 2
  %val = load [take] %src
// Input 2 (`copy_addr [take]`)
  copy_addr [take] %src to [init] %tmp
  copy_addr [take] %src to [init] %dst
// Old Output 2
  copy_addr %src to [init] %dst
  destroy_addr %src
// New Output 2
  copy_addr [take] %src to [init] %dst

rdar://107839979

In preparation for "folding" an "inserted destroy" into a load [copy] or
copy_addr, rename the variable that indicates whether the copyInst's
source must be deinitialized after its last "load".
As a separate commit to make the behavior change more obvious.
To avoid introducing new copies--which is illegal for move-only values--
don't rewrite `load [take]`s and `copy_addr [take]`s as `load [copy]`s
and `copy_addr`s respectively and introduce new `destroy_addr`s after
them.  Instead, get the effect of folding such a newly created
`destroy_addr` into the preceding rewritten `load [copy]` or
`copy_addr`.  Get that effect by neither modifying the `copy_addr [take]`
or `load [take]` nor adding a subsequent `destroy_addr`.

An example for each kind (`load [take]` and `copy_addr [take]`):

```
// Input 1 (`load [take]`)
  copy_addr [take] %src to [init] %tmp
  %val = load [take] %src
// Old Output 1
  %val = load [copy] %src
  destroy_addr %src
// New Output 2
  %val = load [take] %src
```

```
// Input 2 (`copy_addr [take]`)
  copy_addr [take] %src to [init] %tmp
  copy_addr [take] %src to [init] %dst
// Old Output 2
  copy_addr %src to [init] %dst
  destroy_addr %src
// New Output 2
  copy_addr [take] %src to [init] %dst
```

rdar://107839979
@nate-chandler nate-chandler requested a review from a team as a code owner April 11, 2023 17:48
@nate-chandler
Copy link
Contributor Author

@swift-ci please test

Copy link
Contributor

@tbkka tbkka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't comment on the implementation, but I like that we're eliminating copies!

@nate-chandler nate-chandler merged commit 563750f into swiftlang:release/5.9 Apr 11, 2023
@nate-chandler nate-chandler deleted the cherrypick/release/5.9/rdar104729396 branch April 11, 2023 22:10
@AnthonyLatsis AnthonyLatsis added the 🍒 release cherry pick Flag: Release branch cherry picks label May 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🍒 release cherry pick Flag: Release branch cherry picks swift 5.9
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants