Stream: git-wasmtime

Topic: wasmtime / PR #10722 Refactor call ABI implementation

Wasmtime GitHub notifications bot (May 03 2025 at 14:26):

uweigand opened PR #10722 from uweigand:abi-refactor to bytecodealliance:main:

This refactors implementation of call ABI handling across architectures with the goal of bringing s390x in line with other platforms.

The main idea is to

• handle main call instruction selection and generation in ISLE (like s390x but unlike other platforms today)
• handle argument setup mostly outside of ISLE (like other platforms but unlike s390x today)
• handle return value processing as part of the call instructio (like all platforms today)

All platforms now emit the main call instruction directly from ISLE, which e.g. handles selection of the correct ISA instruction depending on the call destination. This ISLE code calls out to helper routines to handle argument and return value processing. These helpers are mostly common code and provided by the Callee and/or Lower layers, with some platform-specific additions via ISLE Context routines.

The old CallSite abstraction is no longer needed; most of the differences between call and return_call handling disappear. (There is still a common-code CallInfo vs. a platform-specifc ReturnCallInfo. At this point, it should be relatively straight- forward to make CallInfo platform-specific as well if desired, but this is not done here.)

Some ISLE infrastructure for iterators / loops, which was only ever used by the s390x argument processing code, has been removed.

s390x now closely matches all other platforms, with only a few special cases (slightly different tail-call ABI requires some differences in stack offset computations; we still need to handle vector lane swaps for cross-ABI calls), which should simplify future maintenance.

FYI @cfallin

<!--
Please make sure you include the following information:

• If this work has been discussed elsewhere, please include a link to that
  conversation. If it was discussed in an issue, just mention "issue #...".

• Explain why this change is needed. If the details are in an issue already,
  this can be brief.

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

Wasmtime GitHub notifications bot (May 03 2025 at 14:26):

uweigand requested abrown for a review on PR #10722.

Wasmtime GitHub notifications bot (May 03 2025 at 14:26):

uweigand requested wasmtime-compiler-reviewers for a review on PR #10722.

Wasmtime GitHub notifications bot (May 03 2025 at 15:44):

github-actions[bot] commented on PR #10722:

Subscribe to Label Action

cc @cfallin, @fitzgen

<details>
This issue or pull request has been labeled: "cranelift", "cranelift:area:aarch64", "cranelift:area:machinst", "cranelift:area:x64", "isle"

Thus the following users have been cc'd because of the following labels:

• cfallin: isle
• fitzgen: isle

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

Learn more.
</details>

Wasmtime GitHub notifications bot (May 05 2025 at 23:09):

abrown submitted PR review:

This makes sense to me; looking at the x64 backend especially, this refactoring seems fine. I'll defer to @cfallin, though, on any finer points since it may be that you all have discussed this change previously (?).

Wasmtime GitHub notifications bot (May 05 2025 at 23:32):

cfallin submitted PR review:

This looks great -- thanks a bunch for doing this refactor and better unifying all the ABI implementations! I like the more functional approach (external helpers to build info, then emit the one call inst) a lot.

Wasmtime GitHub notifications bot (May 05 2025 at 23:52):

cfallin commented on PR #10722:

Hmm, issue with x86-64 mingw runs segfaulting in CI on merge queue checks: here

Wasmtime GitHub notifications bot (May 06 2025 at 15:07):

uweigand updated PR #10722.

Wasmtime GitHub notifications bot (May 06 2025 at 15:10):

uweigand commented on PR #10722:

Hmm, issue with x86-64 mingw runs segfaulting in CI on merge queue checks: here

I forgot to use accumulate_outgoing_args in the x64 emit_vm_call routine. Libcalls don't usually have stack arguments so this didn't matter, but __cranelift_x86_pshufb does when using the WindowsFastcall convention. Should be fixed now.

Wasmtime GitHub notifications bot (May 06 2025 at 15:12):

cfallin commented on PR #10722:

Thanks, good catch!

A few jobs failing with " Error: Internal server error occurred while resolving "abrown/install-vtune-action@v1". Internal server error occurred while resolving "actions/cache@v4". Internal server error occurred while resolving "actions/checkout@v4"" -- cc @abrown on that one. Probably a spurious network error so restarting.

Wasmtime GitHub notifications bot (May 06 2025 at 15:14):

cfallin has enabled auto merge for PR #10722.

Wasmtime GitHub notifications bot (May 06 2025 at 15:41):

uweigand commented on PR #10722:

Looks like there's another internal error.

Wasmtime GitHub notifications bot (May 06 2025 at 15:44):

cfallin commented on PR #10722:

@abrown it looks like the abrown/install-vtune-action action is failing consistently here -- would you mind putting up a PR to remove this action from CI for now, since it's not tier 1 and it's blocking merges? Then we can rebase here and hopefully merge.

Wasmtime GitHub notifications bot (May 06 2025 at 15:49):

alexcrichton commented on PR #10722:

I'm not certain it's something wrong with the action, the failure is also for actions/cache@v4. I restarted again and it's making progress this time. I think github actions is just having a bad day so far and I don't think there's anything specific about abrown/install-vtune-action

Wasmtime GitHub notifications bot (May 06 2025 at 16:15):

cfallin merged PR #10722.

Last updated: Dec 06 2025 at 07:03 UTC