Stream: git-wasmtime

Topic: wasmtime / PR #4585 [s390x, abi_impl] Support struct args...

Wasmtime GitHub notifications bot (Aug 02 2022 at 22:32):

uweigand opened PR #4585 from s390x-struct-arg to main:

This adds support for StructArgument on s390x. The ABI for this
platform requires that the address of the buffer holding the copy
of the struct argument is passed from caller to callee as hidden
pointer, using a register or overflow stack slot.

To implement this, I've added an optional "pointer" filed to
ABIArg::StructArg, and code to handle the pointer both in common
abi_impl code and the s390x back-end.

One notable change necessary to make this work involved the
"copy_to_arg_order" mechanism. Currently, for struct args
we only need to copy the data (and that need to happen before
setting up any other args), while for non-struct args we only
need to set up the appropriate registers or stack slots.
This order is ensured by sorting the arguments appropriately
into a "copy_to_arg_order" list.

However, for struct args with explicit pointers we need to both
copy the data (again, before everything else), and set up a
register or stack slot. Since we now need to touch the argument
twice, we cannot solve the ordering problem by a simple sort.
Instead, the abi_impl common code now provided two callbacks,
emit_copy_regs_to_buffer and emit_copy_regs_to_arg, and expects
the back end to first call copy..to_buffer for all args, and
then call copy.._to_arg for all args. This required updates
to all back ends.

In the s390x back end, in addition to the new ABI code, I'm now
adding code to actually copy the struct data, using the MVC
instruction (for small buffers) or a memcpy libcall (for larger
buffers). This also requires a bit of new infrastructure:

• MVC is the first memory-to-memory instruction we use, which
  needed a bit of memory argument tweaking

• We also need to set up the infrastructure to emit libcalls.

(FYI @cfallin - This implements the first half of issue #4565.)

<!--

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 (Aug 03 2022 at 17:51):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Aug 03 2022 at 17:51):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Aug 03 2022 at 17:51):

cfallin created PR review comment:

This will either need a rebase over @fitzgen's conversion of slots to smallvec or, if I'm misremembering what became a smallvec, this should become a smallvec :-)

Wasmtime GitHub notifications bot (Aug 03 2022 at 18:24):

uweigand updated PR #4585 from s390x-struct-arg to main.

Wasmtime GitHub notifications bot (Aug 03 2022 at 18:39):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Aug 03 2022 at 18:39):

cfallin has enabled auto merge for PR #4585.

Wasmtime GitHub notifications bot (Aug 03 2022 at 18:50):

uweigand created PR review comment:

Yes, had to become a smallvec. Rebased now, thanks!

Wasmtime GitHub notifications bot (Aug 03 2022 at 18:50):

uweigand submitted PR review.

Wasmtime GitHub notifications bot (Aug 03 2022 at 19:00):

cfallin merged PR #4585.

Last updated: Dec 23 2024 at 12:05 UTC