Skip to content

Conversation

alexcrichton
Copy link
Member

This removes MovRR and no new instructions were needed in the
assembler as everything was already added. Lots of little updates here
and there but everything pretty straightforward. Much of the conversion
goop should get better over time as preexisting instructions should move
to Gpr instead of Reg which would avoid the need for conversions at
all.

@alexcrichton alexcrichton requested a review from a team as a code owner June 12, 2025 14:34
@alexcrichton alexcrichton requested review from fitzgen and removed request for a team June 12, 2025 14:34
Comment on lines 2679 to 2682
let inst = asm::inst::movq_mr::new(
temp.map(Gpr::unwrap_new),
Gpr::unwrap_new(dst_old.to_reg()),
)
.into();
Inst::External { inst }.emit(sink, info, state);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice to have an emit_asm helper or something so that every call site doesn't need to do the whole .into(); Inst::External { inst }.emit(...) dance. Because the new version of everything here is a lot harder to read.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mentioned this in the OP but while some impedance mismatch is what you mention another big source is using Reg in pseudo-insts instead of Gpr. I'll work on a follow-up to try to fix that.

@github-actions github-actions bot added cranelift Issues related to the Cranelift code generator cranelift:area:x64 Issues related to x64 codegen labels Jun 12, 2025
@alexcrichton alexcrichton added this pull request to the merge queue Jun 12, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to a conflict with the base branch Jun 12, 2025
@alexcrichton alexcrichton enabled auto-merge June 12, 2025 18:44
@alexcrichton alexcrichton added this pull request to the merge queue Jun 12, 2025
@alexcrichton alexcrichton removed this pull request from the merge queue due to a manual request Jun 12, 2025
This removes `MovRR` and no new instructions were needed in the
assembler as everything was already added. Lots of little updates here
and there but everything pretty straightforward. Much of the conversion
goop should get better over time as preexisting instructions should move
to `Gpr` instead of `Reg` which would avoid the need for conversions at
all.
@alexcrichton alexcrichton enabled auto-merge June 12, 2025 22:16
@alexcrichton alexcrichton added this pull request to the merge queue Jun 12, 2025
Merged via the queue into bytecodealliance:main with commit d3c1cc4 Jun 12, 2025
53 checks passed
@alexcrichton alexcrichton deleted the x64-rm-movrr branch June 12, 2025 22:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cranelift:area:x64 Issues related to x64 codegen cranelift Issues related to the Cranelift code generator
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants