Stream: git-wasmtime

Topic: wasmtime / PR #10023 Winch: implement rmw `and`, `xor` an...

view this post on Zulip Wasmtime GitHub notifications bot (Jan 16 2025 at 11:23):

MarinPostma edited PR #10023.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 16 2025 at 16:21):

MarinPostma updated PR #10023.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 16 2025 at 16:31):

MarinPostma updated PR #10023.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 16 2025 at 16:43):

MarinPostma updated PR #10023.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 16 2025 at 17:07):

MarinPostma updated PR #10023.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 16 2025 at 22:26):

saulecabrera submitted PR review:

Winch pieces look good to me, modulo the panic comment below.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 16 2025 at 22:26):

saulecabrera created PR review comment:

I don't see an issue with exposing this. However, given that I'm not deeply familiar with the history of x64 backend in Cranelift, I'll kindly ask @cfallin for confirmation.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 16 2025 at 22:26):

saulecabrera created PR review comment:

Could you return an error here instead of panicking?

view this post on Zulip Wasmtime GitHub notifications bot (Jan 16 2025 at 22:37):

MarinPostma submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 16 2025 at 22:37):

MarinPostma created PR review comment:

does it make sense to return an error here? we explicitely check that we only match the right variants above, this branch should be legitmately unreachable. No CodeGenError seem to correspond, so I was reluctant to add an error variant for a trivially unreachable statement.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 16 2025 at 22:49):

saulecabrera submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 16 2025 at 22:49):

saulecabrera created PR review comment:

Yeah you're right, it's probably fine as is. I'm trying to error on the side of caution when it comes to panics, because we've put a lot of effort to make sure that they are minimal, particularly if they affect partial development of features at the visitor/masm layer. Here I got confused by the nested match which led me to think that we were missing an implementation.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 17 2025 at 21:32):

saulecabrera submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 17 2025 at 21:33):

saulecabrera commented on PR #10023:

@MarinPostma just a heads up that there are a couple of conflicts that need to be resolved before merging.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 18 2025 at 11:41):

MarinPostma updated PR #10023.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 18 2025 at 11:53):

MarinPostma updated PR #10023.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 18 2025 at 11:53):

MarinPostma commented on PR #10023:

@saulecabrera I squashed to make conflict resolution more manageable

view this post on Zulip Wasmtime GitHub notifications bot (Jan 19 2025 at 15:17):

saulecabrera submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 19 2025 at 15:37):

saulecabrera merged PR #10023.

Last updated: Jan 24 2025 at 00:11 UTC