-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[CopyPropagation] Canonicalize all values. #41943
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
[CopyPropagation] Canonicalize all values. #41943
Conversation
@swift-ci please benchmark |
|
We're really starting to see the benefits of ownership representation now |
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.
Good. This means the lifetime of owned values will be consistently canonicalized, within the new boundaries of lexical lifetimes.
cd76eb3
to
6b06ce2
Compare
@swift-ci please benchmark |
@swift-ci please test |
@swift-ci please benchmark |
@swift-ci please test macos platform |
@swift-ci please test linux platform |
1 similar comment
@swift-ci please test linux platform |
@swift-ci please clean test macos platform |
@swift-ci please clean test linux platform |
Thanks for fixing the outliner to handle consuming arguments. It's good to have this pass working well with OSSA! |
@swift-ci please clean test linux platform |
@nate-chandler I just realized this never landed. Anything holding it up? |
There were some docc test failures earlier. Let's see if those were spurious. |
@swift-ci please test |
6b06ce2
to
aaf4750
Compare
@swift-ci please test |
aaf4750
to
899141e
Compare
@swift-ci please test |
The new utility looks through operands whose values are themselves SILPhiArguments and visits those arguments' operands.
899141e
to
6fcd1f6
Compare
@swift-ci please test |
6fcd1f6
to
228232f
Compare
@swift-ci please test |
The new utility, given an phi, visits all adjacent phis (i.e. arguments to the same block) which are (potentially iterated) reborrows of a value reaching the given phi.
When canonicalizing lifetime of an owned phi, add adjacent phis which are reborrows of its reaching values to the worklist. Update liveness to consider uses whose operand ownership is specific to guaranteed values (i.e. uses which are derived from these adjacent reborrow phis) as standard, non-lifetime-ending uses. rdar://94346482
Properly handle the different sorts of reborrows when they are visited: - non-branch reborrows (tuples, etc) are non-lifetime-ending uses; such reborrows are relevant defs for liveness - branch reborrows which also consume the owned phi are lifetime-ending uses - branch reborrows which DO NOT consume the owned phi are non-lifetime-ending uses, and the corresponding phis are relevant defs for liveness rdar://97399065
228232f
to
7feccbd
Compare
@swift-ci please test |
Previously, the match failed to find the larger sequence that began with load [copy] and ended with destroy_value because the iterator advanced after finding the load [copy]. Advanced the iterator here. Enables reverting the test outlining test changes introduced in a52b896.
Previously, the lifetimes of only values which were copied were canonicalized during the non-mandatory CopyPropagation pass. The mandatory pass (i.e. MandatoryCopyPropagation) has been disabled, meaning that non-copied values lifetimes were never canonicalized. So enable canonicalization of all lifetimes during regular CopyPropagation. rdar://54335055
Thanks to CopyPropagation canonicalizing all values, not just those for which there is a copy_value instruction, the lifetime of the value that is loaded in the BridgedProperty pattern is shortened and the destroy_value appears right after its use rather than after the full CFG for the bridged property. Updated the BridgedProperty pattern to match that newly hoisted instruction.
Put off calculating the outlined function name until the point at which we will have all the inputs for the name.
Previously, for the BridgedProperty pattern, there were two flavors of outlined function that would be formed, varying on the ownership of the instance whose field is being access: (1) guaranteed_in -- for use by addresses (2) unowned -- for use by values which were not destroyed until some time after the pattern of interest is matched Now that the lifetime of the instance may be shortened to just after the call to the objc method, the latter of these is not appropriate for values. We would have to re-extend the lifetime until after the method returns. Instead, here, we add a new variant (3) owned -- for use by values which were destroyed just after the objc method call Doing so requires new mangling.
7feccbd
to
a903805
Compare
@swift-ci please test |
@swift-ci please test source compatibility |
Previously, the lifetimes of only values which were copied were canonicalized during the non-mandatory CopyPropagation pass. The mandatory pass (i.e. MandatoryCopyPropagation) has been disabled, meaning that non-copied values lifetimes were never canonicalized. So enable canonicalization of all lifetimes during regular CopyPropagation.
rdar://54335055