jameysharp opened PR #8275 from jameysharp:fix-retcall-indirect
to bytecodealliance:main
:
x64 now uses a dedicated codegen strategy for tail-calls which writes the callee's stack arguments directly into their final location, but aarch64 and riscv64 still use our previous strategy of setting up for a normal function call and then moving the new stack frame up to overwrite the old one. In order to emit correct code for a normal function call, we need to fake a normal Call/CallIndirect opcode rather than ReturnCall/ReturnCallIndirect.
The new tests are a combination of x64's compile-tests for return-call-indirect.clif, plus a large-stack-frame test from aarch64/riscv64's return-call.clif modified to do an indirect call.
jameysharp requested fitzgen for a review on PR #8275.
jameysharp requested wasmtime-compiler-reviewers for a review on PR #8275.
github-actions[bot] commented on PR #8275:
Subscribe to Label Action
cc @cfallin, @fitzgen
<details>
This issue or pull request has been labeled: "cranelift", "cranelift:area:aarch64", "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>
fitzgen submitted PR review:
Can we have some runtests exercising this stuff as well?
jameysharp commented on PR #8275:
Can we have some runtests exercising this stuff as well?
We do have
cranelift/filetests/filetests/runtests/return-call-indirect.clif
, we just didn't have the indirect case tested in compile-tests for aarch64 and riscv64. Do you want me to just add a similar stack-arguments test there? It wouldn't have caught this bug because we havepreserve_frame_pointers=true
in that test, since x64 currently requires it, but there's no harm adding it. I could do the same in the x64 compile tests for that matter.
fitzgen commented on PR #8275:
I think it would be good to have, yeah
jameysharp updated PR #8275.
jameysharp has enabled auto merge for PR #8275.
jameysharp merged PR #8275.
Last updated: Dec 23 2024 at 13:07 UTC