Stream: git-wasmtime

Topic: wasmtime / PR #5402 Reduce sig data size by changing size...

Wasmtime GitHub notifications bot (Dec 08 2022 at 23:58):

tnachen opened PR #5402 from tnachen/sig_u32 to main:

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

• [ ] This has been discussed in issue #..., or if not, please tell us why
  here.

• [ ] A short description of what this does, why it is needed; if the
  description becomes long, the matter should probably be discussed in an issue
  first.

• [ ] This PR contains test cases, if meaningful.

• [ ] A reviewer from the core maintainer team has been assigned for this PR.
  If you don't know who could review this, please indicate so. The list of
  suggested reviewers on the right can help you.

Please ensure all communication adheres to the code of conduct.
-->

Wasmtime GitHub notifications bot (Dec 09 2022 at 00:22):

fitzgen submitted PR review.

Wasmtime GitHub notifications bot (Dec 09 2022 at 00:22):

fitzgen submitted PR review.

Wasmtime GitHub notifications bot (Dec 09 2022 at 00:22):

fitzgen created PR review comment:

If you want, it would be cool to push the u32s back even further, so that M::compute_arg_locs returns a u32 for the stack ret space and all the implementations for each backend use u32s and all that. Could be a follow up PR.

Wasmtime GitHub notifications bot (Dec 09 2022 at 00:22):

fitzgen has enabled auto merge for PR #5402.

Wasmtime GitHub notifications bot (Dec 09 2022 at 00:42):

jameysharp submitted PR review.

Wasmtime GitHub notifications bot (Dec 09 2022 at 00:42):

jameysharp created PR review comment:

Hah, @tnachen wanted to do that and I recommended going for the smaller patch first.

Wasmtime GitHub notifications bot (Dec 09 2022 at 00:48):

jameysharp submitted PR review.

Wasmtime GitHub notifications bot (Dec 09 2022 at 00:48):

jameysharp created PR review comment:

Well, more accurately, @tnachen wanted to change the return types of sized_stack_arg_space() and sized_stack_ret_space(), which propagates out to the backends in a different way. With how widely the backends use i64 I was afraid we'd have to sort out a lot of questions about which values actually should be signed. For example, are the various offsets in StackAMode allowed to be negative?

Maybe changing compute_arg_locs triggers fewer difficult questions, though.

Wasmtime GitHub notifications bot (Dec 11 2022 at 00:38):

fitzgen has disabled auto merge for PR #5402.

Wasmtime GitHub notifications bot (Dec 11 2022 at 00:38):

tnachen updated PR #5402 from tnachen/sig_u32 to main.

Wasmtime GitHub notifications bot (Dec 11 2022 at 12:35):

tnachen updated PR #5402 from tnachen/sig_u32 to main.

Wasmtime GitHub notifications bot (Dec 11 2022 at 23:32):

jameysharp submitted PR review.

Wasmtime GitHub notifications bot (Dec 11 2022 at 23:32):

jameysharp merged PR #5402.

Last updated: Dec 23 2024 at 12:05 UTC