bjorn3 opened PR #9287 from bjorn3:abi_cleanup5
to bytecodealliance:main
.
bjorn3 requested wasmtime-compiler-reviewers for a review on PR #9287.
bjorn3 requested abrown for a review on PR #9287.
bjorn3 updated PR #9287.
bjorn3 updated PR #9287.
bjorn3 requested wasmtime-core-reviewers for a review on PR #9287.
bjorn3 requested fitzgen for a review on PR #9287.
bjorn3 updated PR #9287.
github-actions[bot] commented on PR #9287:
Subscribe to Label Action
cc @saulecabrera
<details>
This issue or pull request has been labeled: "cranelift", "cranelift:area:aarch64", "cranelift:area:x64", "winch"Thus the following users have been cc'd because of the following labels:
- saulecabrera: winch
To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.
Learn more.
</details>
fitzgen submitted PR review:
Thanks! LGTM, although I have some minor nitpicks. If you feel strongly that some of them are unnecessary, I can be convinced otherwise.
fitzgen created PR review comment:
debug_assert_eq!(args_or_rets, ArgsOrRets::Args);
for slightly better panic messages
fitzgen created PR review comment:
assert_eq!(ArgsOrRets::Args, args_or_rets);
fitzgen created PR review comment:
Feel similarly here
fitzgen created PR review comment:
To make this code a little future proof to shuffling it around at some point in the future, lets not hard code
0
:next_x_reg += 1; Some(ABIArg::reg( x_reg(next_x_reg - 1).to_real_reg().unwrap(), I64, ir::ArgumentExtension::None, ir::ArgumentPurpose::Normal, ))
It may be convenient to have a helper closure that does the increment of
next_x_reg
and returnsOption<Reg>
or whatever for you. I'll leave it up to you, and how much you are annoyed bynext_x_reg - 1
:-p
bjorn3 submitted PR review.
bjorn3 created PR review comment:
There are only two options in this enum and this matches the other debug assertions, but sure.
bjorn3 submitted PR review.
bjorn3 created PR review comment:
This really needs to be the first argument register. If next_x_reg - 1 is not x_start at this point, that is a bug.
bjorn3 updated PR #9287.
bjorn3 updated PR #9287.
fitzgen submitted PR review.
fitzgen has enabled auto merge for PR #9287.
fitzgen merged PR #9287.
Last updated: Dec 23 2024 at 12:05 UTC