Skip to content

asm: implement write-only operands #10759

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

Merged
merged 2 commits into from
May 9, 2025
Merged

Conversation

abrown
Copy link
Contributor

@abrown abrown commented May 9, 2025

This change adds support to the new assembler for write-only operands. This implementation appeared first in #10754 but is split out here to unblock implementation of instructions that require it: multiplication, conversions, moves, etc. This starts roughly the same as what was implemented for write-only XMMs in #10754 but includes support for write-only GPRs as well and generates the temporary registers which are needed.

This change adds support to the new assembler for write-only operands.
This implementation appeared first in [bytecodealliance#10754] but is split out here to
unblock implementation of instructions that require it: multiplication,
conversions, moves, etc. This starts roughly the same as what was
implemented for write-only XMMs in [bytecodealliance#10754] but includes support for
write-only GPRs as well and generates the temporary registers which are
needed.

[bytecodealliance#10754]: bytecodealliance#10754

Co-authored-by: Johnnie Birch <[email protected]>
@abrown abrown requested a review from a team as a code owner May 9, 2025 20:54
@abrown abrown requested review from fitzgen and removed request for a team May 9, 2025 20:54
Comment on lines 129 to 139
Write => match one.location.kind() {
// One write-only register output? Output the
// instruction and that register.
OperandKind::Reg(r) => {
let ty = r.reg_class().unwrap().to_string();
let var = ty.to_lowercase();
fmtln!(f, "let {var} = {r}.as_ref().clone();");
fmtln!(f, "AssemblerOutputs::Ret{ty} {{ inst, {var} }}");
}
_ => unimplemented!(),
},
Copy link
Member

Choose a reason for hiding this comment

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

Should this get collapsed into ReadWrite as well below? This has the same handling for Reg and all the other cases look applicable for one-operand-written instructions too (e.g. if something stores in memory or outputs to a fixed register)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately, there's slightly different behavior for retrieving the underlying Cranelift type. The ReadWrite variant needs to access the write field; this one should actually be doing a as_ref().to_reg() (let me fix that). I think in the fullness of time we will want to merge some things like I did over in the assembler generator but let's leave this as-is until we have a few more write-only instructions implemented.

@abrown abrown added this pull request to the merge queue May 9, 2025
Merged via the queue into bytecodealliance:main with commit d9468c6 May 9, 2025
53 checks passed
@abrown abrown deleted the asm-write branch May 9, 2025 22:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants