fitzgen commented on issue #6500:
Looks reasonable. Could we add a comment somewhere noting that for now, the
Tail
convention is treated the same asSystemV
, but is expected to change with subsequent patches? With that noted somewhere, LGTM.It is already documented to have unstable ABI, so I think we should be fine on this front?
cfallin commented on issue #6500:
Looks reasonable. Could we add a comment somewhere noting that for now, the
Tail
convention is treated the same asSystemV
, but is expected to change with subsequent patches? With that noted somewhere, LGTM.It is already documented to have unstable ABI, so I think we should be fine on this front?
Sure, that's the public-facing (lack of) guarantee, but it'd be nice to have a note on the current implementation somewhere (and update it when it changes), IMHO.
fitzgen commented on issue #6500:
So I added another test for enough arguments that some go on the stack, and found a bug in this PR (I had the appropriate changes in another wip branch, but didn't have them here yet) where the call site was still trying to pop stack arguments, even though the callee already did that.
But then there is s390x. I don't really know s390x's calling convention, but even the simpler test that was already in this PR is failing on s390x with wrong values being returned. The new test is causing segfaults. I need to dig in more, but there is a lot for me to learn here first.
fitzgen commented on issue #6500:
I'm having a hard time figuring out the details of s390x here, so I am going to disable support for the
tail
calling convention on s390x (with a loud assertion) to land this PR. We can get s390x working in follow ups.
fitzgen commented on issue #6500:
There seem to be some
wasi-http
-related crashes on windows MSVC in CI: https://github.com/bytecodealliance/wasmtime/actions/runs/5193242010/jobs/9363525711?pr=6500Has anyone seen these before? I highly doubt that this PR is the root cause, as it is changing a calling convention that isn't used by Wasmtime currently.
fitzgen edited a comment on issue #6500:
Edit: nevermind, I completely misread some things.
Last updated: Nov 22 2024 at 16:03 UTC