-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[4.0] [sil-combine] When deleting a dead partial_apply, insert a destroy for all non-trivial captured values. #10806
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
[4.0] [sil-combine] When deleting a dead partial_apply, insert a destroy for all non-trivial captured values. #10806
Conversation
…r all non-trivial captured values. partial_apply is a confusing instruction since it: 1. Is printed with a function signature. 2. Takes in some arguments of the same type as the underlying types of the given function signatures. 3. Always takes in those arguments at +1 regardless of the convention printed on the partial apply. Eventually we will split the partial apply representation so that the box is represnted explicitly separately from the function signature, eliminating this confusion. The problem that we ran into here is that we were not treating @in values from an alloc_stack or @in_guaranteed at all correctly. The reason why the tests did not catch this is that a seperate sil_combine optimization that eliminates trivially dead live ranges was eliminting the alloc_stack of the @in value in our test. In contrast, the @in_guaranteed case was actually never tested at all. I added tests for all of these conventions and in addition added a special mode to SILCombine that stops the alloc_stack eliminating during testing. rdar://32887993
@swift-ci test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Build failed |
The Linux test failed due to an unrelated issue. Rerunning the test. |
@swift-ci test linux platform |
Thanks Andy! |
Explanation: The problem that we ran into here is that when deleting dead closures, we were not treating @in and @in_guaranteed closure arguments from an alloc_stack. In both cases, we would have leaked values. |
Mmm... made a mistake in my CCC. Sigh. I came up with the SourceKit thing at the last minute as I was typing. It was a mistake. Fixing the CCC. |
Explanation: The problem that we ran into here is that when deleting dead closures, we were not treating @in and @in_guaranteed closure arguments from an alloc_stack. In both cases, we would have leaked values. |
Approved by Bob via email. Merging! |
[sil-combine] When deleting a dead partial_apply, insert a destroy for all non-trivial captured values.
partial_apply is a confusing instruction since it:
Eventually we will split the partial apply representation so that the box is
represnted explicitly separately from the function signature, eliminating this
confusion.
The problem that we ran into here is that we were not treating @in values from
an alloc_stack or @in_guaranteed at all correctly. The reason why the tests did
not catch this is that a seperate sil_combine optimization that eliminates
trivially dead live ranges was eliminting the alloc_stack of the @in value in
our test. In contrast, the @in_guaranteed case was actually never tested at all.
I added tests for all of these conventions and in addition added a special mode
to SILCombine that stops the alloc_stack eliminating during testing.
rdar://32887993