Skip to content

Followup for #61505 #61676

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

Merged
merged 4 commits into from
Oct 23, 2022
Merged

Followup for #61505 #61676

merged 4 commits into from
Oct 23, 2022

Conversation

meg-gupta
Copy link
Contributor

#61505 had leftover places where begin_borrows were being created to pass a @guaranteed function argument as a phi operand. This PR gets rid of them. Because with @guaranteed forwarding phi support, no @guaranteed value needs additional borrow scope for being used as a phi operand.

@meg-gupta
Copy link
Contributor Author

@swift-ci test

@meg-gupta
Copy link
Contributor Author

@swift-ci test

@meg-gupta meg-gupta merged commit 79565a4 into swiftlang:main Oct 23, 2022
@atrick
Copy link
Contributor

atrick commented Oct 23, 2022

@meg-gupta did you mean to revert two commits in this PR?

On the verifier revert: Can we just verify that the operand of a begin_borrow is always a borrow introducer? If we do that, then we don't need the SILVerifier check for phis, which I didn't completely understand anyway.

What's going on with the ConditionVerifier revert?

@meg-gupta
Copy link
Contributor Author

@atrick ConditionForwarding was turned off for @guaranteed function arguments, I reverted it, so they can be optimized too.

CopyPropagation needs to be fixed to clean up unnecessary begin_borrows after doing some @owned to @guaranteed transformation. I'll enable the verifier assert after that is fixed.

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