Stream: git-wasmtime

Topic: wasmtime / PR #9287 Make the Tail call conv follow the sy...


view this post on Zulip Wasmtime GitHub notifications bot (Sep 19 2024 at 19:44):

bjorn3 opened PR #9287 from bjorn3:abi_cleanup5 to bytecodealliance:main.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 19 2024 at 19:44):

bjorn3 requested wasmtime-compiler-reviewers for a review on PR #9287.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 19 2024 at 19:44):

bjorn3 requested abrown for a review on PR #9287.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 19 2024 at 20:01):

bjorn3 updated PR #9287.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 19 2024 at 20:11):

bjorn3 updated PR #9287.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 19 2024 at 20:13):

bjorn3 requested wasmtime-core-reviewers for a review on PR #9287.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 19 2024 at 20:13):

bjorn3 requested fitzgen for a review on PR #9287.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 19 2024 at 20:13):

bjorn3 updated PR #9287.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 19 2024 at 21:44):

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:

To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.

Learn more.
</details>

view this post on Zulip Wasmtime GitHub notifications bot (Sep 20 2024 at 15:32):

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.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 20 2024 at 15:32):

fitzgen created PR review comment:

            debug_assert_eq!(args_or_rets, ArgsOrRets::Args);

for slightly better panic messages

view this post on Zulip Wasmtime GitHub notifications bot (Sep 20 2024 at 15:32):

fitzgen created PR review comment:

            assert_eq!(ArgsOrRets::Args, args_or_rets);

view this post on Zulip Wasmtime GitHub notifications bot (Sep 20 2024 at 15:32):

fitzgen created PR review comment:

Feel similarly here

view this post on Zulip Wasmtime GitHub notifications bot (Sep 20 2024 at 15:32):

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 returns Option<Reg> or whatever for you. I'll leave it up to you, and how much you are annoyed by next_x_reg - 1 :-p

view this post on Zulip Wasmtime GitHub notifications bot (Sep 20 2024 at 15:36):

bjorn3 submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 20 2024 at 15:36):

bjorn3 created PR review comment:

There are only two options in this enum and this matches the other debug assertions, but sure.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 20 2024 at 15:39):

bjorn3 submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 20 2024 at 15:39):

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.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 20 2024 at 15:48):

bjorn3 updated PR #9287.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 20 2024 at 15:57):

bjorn3 updated PR #9287.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 20 2024 at 16:02):

fitzgen submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 20 2024 at 16:02):

fitzgen has enabled auto merge for PR #9287.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 20 2024 at 16:26):

fitzgen merged PR #9287.


Last updated: Dec 23 2024 at 12:05 UTC