Stream: git-wasmtime

Topic: wasmtime / PR #8406 aarch64: Fixed destination register f...

Wasmtime GitHub notifications bot (Apr 18 2024 at 23:38):

elliottt requested fitzgen for a review on PR #8406.

Wasmtime GitHub notifications bot (Apr 18 2024 at 23:38):

elliottt opened PR #8406 from elliottt:trevor/aarch64-fixed-return-call-ind-dest to bytecodealliance:main:

After switching to using callee-saved registers with the tail calling convention on aarch64, we missed using a fixed constraint for the destination of indirect return calls. This omission introduces a situation where the destination could be placed in a callee-saved register, and then clobbered as part of the call to emit_return_call_common_sequence.

This PR fixes the problem by always using x1 for return call destinations, and relaxes the same constraint on indirect calls of tail call functions, as callee-saved registers can be used for normal indirect calls now.

<!--
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 (Apr 18 2024 at 23:38):

elliottt requested wasmtime-compiler-reviewers for a review on PR #8406.

Wasmtime GitHub notifications bot (Apr 18 2024 at 23:41):

elliottt requested jameysharp for a review on PR #8406.

Wasmtime GitHub notifications bot (Apr 19 2024 at 00:13):

jameysharp submitted PR review:

I hope @cfallin can double-check this later, but I'm pretty confident this is all correct and you can go ahead and merge it.

We need to constrain ReturnCallInd, because it will overwrite any callee-saved registers before reading the value of callee, so we must not use any of those registers for callee.

But we don't need to constrain CallInd to avoid using callee-saved registers, because it doesn't overwrite any of them before reading the value of callee. In fact the whole point of callee-saved registers is that the actual call will preserve them, so we should _prefer_ to use them for normal calls if we still need the address afterward.

Wasmtime GitHub notifications bot (Apr 19 2024 at 00:56):

elliottt merged PR #8406.

Wasmtime GitHub notifications bot (Apr 22 2024 at 19:33):

cfallin commented on PR #8406:

I hope @cfallin can double-check this later

Yep, looks good to me as well! I double-checked that with the tailcall ABI we start assigning args to registers at x2, so x1 is free to use for this purpose here and I can't think of anything else that would clobber it between setup and use.

Last updated: Oct 23 2024 at 20:03 UTC