SIL: let SingleValueInstruction only inherit from a single SILNode (2nd try) #35615
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Replace the `isa(SILNode *)` with `isa(SILInstruction *)` and `isa(SILValue)`. This is much clearer and it also works if the SILValue is a MultiValueInstructionResult of an apply instruction. Also, use `isa` instead of `classof` in canOptimize()
Split `markValueLive(SILNode *)` into `markValueLive(SILValue)` and `markInstructionLive(SILInstruction*)`
… when checking for invariant arguments. Use `getDefiningInstruction` instead of casting a a SILValue to SingleValueInstruction.
…om SILNode* to SILValue A small cleanup, NFC.
This resolves a FIXME. Also, use `getDefiningInstruction()` instead of `getRepresentativeSILNodeInObject()` NFC
…tUsersOfInstruction. A small cleanup, NFC.
…tion *)` A small cleanup, NFC
A small cleanup, NFC.
This removes the ambiguity when casting from a SingleValueInstruction to SILNode, which makes the code simpler. E.g. the "isRepresentativeSILNode" logic is not needed anymore. Also, it reduces the size of the most used instruction class - SingleValueInstruction - by one pointer. Conceptually, SILInstruction is still a SILNode. But implementation-wise SILNode is not a base class of SILInstruction anymore. Only the two sub-classes of SILInstruction - SingleValueInstruction and NonSingleValueInstruction - inherit from SILNode. SingleValueInstruction's SILNode is embedded into a ValueBase and its relative offset in the class is the same as in NonSingleValueInstruction (see SILNodeOffsetChecker). This makes it possible to cast from a SILInstruction to a SILNode without knowing which SILInstruction sub-class it is. Casting to SILNode cannot be done implicitly, but only with an LLVM `cast` or with SILInstruction::asSILNode(). But this is a rare case anyway.
|
@swift-ci test |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
After fixing a bug, this reinstates #35554 (after reverted by #35600).
Letting SingleValueInstruction only inherit from a single SILNode removes the ambiguity when casting from a SingleValueInstruction to SILNode, which makes the code simpler. E.g. the "isRepresentativeSILNode" logic is not needed anymore.
Also, it reduces the size of the most used instruction class - SingleValueInstruction - by one word.
Conceptually, SILInstruction is still a SILNode. But implementation-wise SILNode is not a base class of SILInstruction anymore.
Only the two sub-classes of SILInstruction - SingleValueInstruction and NonSingleValueInstruction - inherit from SILNode. SingleValueInstruction's SILNode is embedded into a ValueBase and its relative offset in the class is the same as in NonSingleValueInstruction (see SILNodeOffsetChecker).
This makes it possible to cast from a SILInstruction to a SILNode without knowing which SILInstruction sub-class it is.
Casting to SILNode cannot be done implicitly, but only with an LLVM cast or with SILInstruction::asSILNode(). But this is a rare case anyway.
Only the last commit of this PR does the actual SILNode change. All previous commits contain small cleanups, which are required for the change, but are good for their own.