-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[5.0] Fix SILCombine to devirtualize existentials wrapped in enum. #21239
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
[5.0] Fix SILCombine to devirtualize existentials wrapped in enum. #21239
Conversation
This allows Swift code to implement a fast path via a protocol type check as follows: if let existentialVal = genericVal as? SomeProtocol { // do something fast. } Fixes <rdar://problem/46322928> Failure to devirtualize a protocol method applied to an opened existential blocks implemention of DataProtocol. Note: the approach of devirtualization via backward pattern matching is fundamentally wrong and will never be fully general. It should be a forward type propagation.
@swift-ci test. |
@swift-ci test source compatibility. |
--- CCC --- Explanation: Fix SILCombine to devirtualize Optional existentials. This blocks the addition of DataProtocol which is an ABI change. Scope: Only affects -O builds. Extends an existing optimization to recognize more complex SIL patterns. Issue: rdar://problem/46322928 Failure to devirtualize a protocol method applied to an opened existential blocks implemention of DataProtocol. Risk: Medium. This adds a conservative pattern match to SILCombine. The biggest risk is that performing more optimization exposes some other bug in the optimizer. Testing: New unit tests tests were added in both .swift and .sil. |
@eeckstein please review for CCC. I won't merge it until 5.0 branch is ready for PRs. |
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!
SILValue swift::getAddressOfStackInit(AllocStackInst *ASI, | ||
/// | ||
/// allocStackAddr may either itself be an AllocStackInst or an | ||
/// InitEnumDataAddrInst that projects the value of an AllocStackInst. |
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.
It would make sense to turn this comment into an assert.
@swift-ci test source compatibility. |
This allows Swift code to implement a fast path via a protocol type
check as follows:
if let existentialVal = genericVal as? SomeProtocol {
// do something fast.
}
Fixes rdar://problem/46322928 Failure to devirtualize a protocol
method applied to an opened existential blocks implemention of
DataProtocol.
Note: the approach of devirtualization via backward pattern matching
is fundamentally wrong and will never be fully general. It should be a
forward type propagation.