Stream: git-wasmtime

Topic: wasmtime / PR #5172 Generate SSA code from returns


view this post on Zulip Wasmtime GitHub notifications bot (Nov 01 2022 at 23:01):

elliottt opened PR #5172 from trevor/ret-ssa to main:

Modify return pseudoinstructions to have pairs of registers: virtual and real. This allows us to constrain the virtual registers to the real ones specified by the abi, instead of directly emitting moves to those real regisers.

Additionally, this PR marks r14 on s390x as non-allocatable, as it's used as the link register in the return instruction.
<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

Please ensure all communication adheres to the code of conduct.
-->

view this post on Zulip Wasmtime GitHub notifications bot (Nov 01 2022 at 23:47):

elliottt updated PR #5172 from trevor/ret-ssa to main.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 02 2022 at 17:12):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 02 2022 at 17:12):

cfallin created PR review comment:

This is going to make things a little interesting too: properly speaking, an extend should allocate a new temp for fully SSA output, rather than coerce the preg to a Writable and extend into it.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 02 2022 at 17:12):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 02 2022 at 18:21):

elliottt updated PR #5172 from trevor/ret-ssa to main.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 02 2022 at 20:49):

elliottt updated PR #5172 from trevor/ret-ssa to main.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 03 2022 at 18:13):

elliottt updated PR #5172 from trevor/ret-ssa to main.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 03 2022 at 18:18):

elliottt has marked PR #5172 as ready for review.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 03 2022 at 19:24):

elliottt submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 03 2022 at 19:24):

elliottt created PR review comment:

Neat, we avoided a move :tada:

view this post on Zulip Wasmtime GitHub notifications bot (Nov 03 2022 at 20:54):

elliottt updated PR #5172 from trevor/ret-ssa to main.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 03 2022 at 21:01):

elliottt updated PR #5172 from trevor/ret-ssa to main.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 03 2022 at 21:12):

uweigand submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 03 2022 at 21:12):

uweigand submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 03 2022 at 21:12):

uweigand created PR review comment:

Hi @elliottt this seems a bug: the lgr overwrites the effect of the llgfr. Is there some problem with the extension handling on return values?

view this post on Zulip Wasmtime GitHub notifications bot (Nov 03 2022 at 21:12):

uweigand created PR review comment:

Hi @elliottt this seems a bug: the lgr overwrites the effect of the llgfr. Is there some problem with the extension handling on return values?

view this post on Zulip Wasmtime GitHub notifications bot (Nov 03 2022 at 21:38):

uweigand submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 03 2022 at 21:38):

uweigand submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 03 2022 at 21:38):

uweigand created PR review comment:

Doesn't this now violate the x64 ABI? I thought this requires the struct-ret pointer to be returned in %rax, that was the whole point of the logic of adding a StructReturn element to the return values?

view this post on Zulip Wasmtime GitHub notifications bot (Nov 03 2022 at 23:13):

elliottt updated PR #5172 from trevor/ret-ssa to main.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 03 2022 at 23:53):

elliottt submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 03 2022 at 23:53):

elliottt created PR review comment:

There are currently some problems with struct-return that I'm tracking down. I think all the comments you added are directly related.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 04 2022 at 00:17):

elliottt created PR review comment:

@uweigand: I think that Chris's comment here is the same problem you noticed with extension on returns.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 04 2022 at 00:17):

elliottt submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 04 2022 at 21:28):

elliottt updated PR #5172 from trevor/ret-ssa to main.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 04 2022 at 21:29):

elliottt submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 04 2022 at 21:29):

elliottt created PR review comment:

@uweigand could you take another look? After the change to introduce a temporary for the extension, these tests also lack the initial lgr instruction, turning into just llgfr %r2, %r2.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 04 2022 at 21:48):

uweigand created PR review comment:

That's even better, removes an unnecessary copy and just zero-extends %r2 in place. The s390x test changes look all good to me now. (Mostly just renamed registers; in several cases we get better code by eliminating register copies or spills, and a few cases -notably relating to divide instructions- we get slightly worse code. Overall still good though.)

view this post on Zulip Wasmtime GitHub notifications bot (Nov 04 2022 at 21:48):

uweigand submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 04 2022 at 23:00):

elliottt updated PR #5172 from trevor/ret-ssa to main.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 07 2022 at 04:44):

elliottt edited PR #5172 from trevor/ret-ssa to main:

Modify return pseudoinstructions to have pairs of registers: virtual and real. This allows us to constrain the virtual registers to the real ones specified by the abi, instead of directly emitting moves to those real regisers.
<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

Please ensure all communication adheres to the code of conduct.
-->

view this post on Zulip Wasmtime GitHub notifications bot (Nov 07 2022 at 20:30):

elliottt updated PR #5172 from trevor/ret-ssa to main.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 08 2022 at 17:52):

elliottt updated PR #5172 from trevor/ret-ssa to main.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 08 2022 at 18:07):

elliottt updated PR #5172 from trevor/ret-ssa to main.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 08 2022 at 18:42):

elliottt updated PR #5172 from trevor/ret-ssa to main.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 08 2022 at 18:43):

elliottt created PR review comment:

This should be fixed now.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 08 2022 at 18:43):

elliottt submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 08 2022 at 18:45):

elliottt requested cfallin for a review on PR #5172.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 08 2022 at 18:56):

elliottt updated PR #5172 from trevor/ret-ssa to main.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 08 2022 at 21:39):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 09 2022 at 00:00):

elliottt merged PR #5172.


Last updated: Dec 23 2024 at 12:05 UTC