x64: Migrate div instructions to the new assembler #10820
Merged
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
github-actions
bot
added
cranelift
Subscribe to Label Action
This issue or pull request has been labeled: "cranelift", "cranelift:area:x64", "cranelift:meta", "winch"
Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
abrown
reviewed
May 21, 2025
Comment on lines
88
to
93
| pub fn trap() -> Operand { | ||
| Operand { | ||
| implicit: true, | ||
| ..Operand::from(Location::trap) | ||
| } | ||
| } |
There was a problem hiding this comment.
What if instead of "trap as an Operand" we add a field like Inst::trap? I'm concerned that trap codes aren't really operands after all and that we already have a TrapCode hidden away in the Amode with all of its own machinery. Seems like it would be better to:
- define all instructions that can trap with something like
inst(...).can_trap() - decide whether we truly need to pass around trap codes in ISLE or if we can define them earlier, e.g.,
.can_trap(242)
There was a problem hiding this comment.
Tried it out and I like it better too 👍
This mostly required adding a new kind of operand representing a `TrapCode` and plumbing around a few bits and bobs to ensure that this compile-time-only-abstraction does not need to be accounted for in all the encoding bits.
Reduces the size of `KnownOffset` so `Option<KnownOffset>` doesn't have such a high alignment or size, greatly shrinking the size of the instruction and prevents the changes to `Div` from increasing the size of the overall `Inst` enum. In fact it now shrinks!
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.
This mostly required adding a new kind of operand representing a
TrapCodeand plumbing around a few bits and bobs to ensure that this compile-time-only-abstraction does not need to be accounted for in all the encoding bits.