ZJIT: Avoid splitting add_into/sub_into for x86_64 #14177
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
This was referenced Aug 12, 2025
XrXr
approved these changes
Aug 12, 2025
tekknolagi
reviewed
Aug 12, 2025
| @@ -1889,9 +1889,8 @@ impl Assembler { | |||
| out | |||
| } | |||
|
|
|||
| pub fn add_into(&mut self, left: Opnd, right: Opnd) -> Opnd { | |||
| pub fn add_into(&mut self, left: Opnd, right: Opnd) { | |||
There was a problem hiding this comment.
I don't love that this breaks SSA if we use a vreg operand as an argument. Can we require it only take memory/phyreg? (here and below)
There was a problem hiding this comment.
I changed them to only take Opnd::Reg for now a43e62e since it's the only expected usage of them.
Btw if you want LIR to be SSA, you need to fix cpop_into, lea_into, load_into, mov, and store as well.
There was a problem hiding this comment.
Thank you for the change. Didn't know that. I thought LIR was SSA...
There was a problem hiding this comment.
Almost. You can make it happen with just ^. We just haven't come up with anything that needs the guarantee yet.
❌ Tests Failed✖️no tests failed ✔️62266 tests passed(1 flake) |
st0012
approved these changes
Aug 12, 2025
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 PR addresses #14154 (comment).
On x86_64, we were splitting
asm.add_intoto two instructions. By fixing it, this PR also updatesasm.sub_intoto use an implementation that looks similar toasm.add_into.Before
add_intoin concatstrings looked like this:After
It's now:
The instructions generated by
sub_intoin concatstrings stayed the same because merging sub + mov works fine.