-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[ownership] Centralize all info about SILInstruction forwarding in the SILInstruction class hierarchy itself. #34915
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
…ead of FieldIndexCacheBase. This is NFCI. THis is in preparation for making FieldIndexCacheBase a templated subclass.
Previously FieldIndexCacheBase only had a parent class of SingleValueInstruction. I need to be able to in certain cases shim in a SingleValueInstruction subclass as a parent class instead. In my case it is to imbue ownership forwarding on StructExtractInst. This commit itself doesn't make that change and instead just always templatizes using SingleValueInstruction.
@swift-ci test |
Build failed |
fca2238
to
7aa4d9f
Compare
Build failed |
…t in OSSA. Operationally it just means that in SILGlobalVariable blocks, all operands have ownership constraint: {OwnershipKind::Any, UseLifetimeConstraint::NonLifetimeEnding} and all values yield an ownership kind of: OwnershipKind::None.
…e SILInstruction class hierarchy itself. This commit is doing a few things: 1. It is centralizing all decisions about whether an operand's owner instruction or a value's parent instruction is forwarding in each SILInstruction itself. This will prevent this information from getting out of sync. 2. This allowed me to hide the low level queries in OwnershipUtils.h that determined if a SILNodeKind was "forwarding". I tried to minimize the amount of churn in this PR and thus didn't remove the is{Owned,Ownership,Guaranteed}Forwarding{Use,Value} checks. Instead I left them alone but added in asserts to make sure that if the old impl ever returns true, the neew impl does as well. In a subsequent commit, I am going to remove the old impl in favor of isa queries. 3. I also in the process discovered that there were some instructions that were being inconsistently marked as forwarding. All of the asserts in the PR caught these and I fixed these inconsistencies.
7aa4d9f
to
09ae2ef
Compare
@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 except the names are attempting to describe all the invariants rather than just being unique and easily recognizable, like OwnershipForwardingInst
and AggregateOwnershipForwardingInst
@swift-ci test source compatibility |
No description provided.