Stream: git-wasmtime

Topic: wasmtime / PR #3268 Implement `Swizzle` and `Splat` for i...


view this post on Zulip Wasmtime GitHub notifications bot (Aug 31 2021 at 11:19):

dheaton-arm opened PR #3268 from implement-swizzle-splat to main:

Implemented for the Cranelift interpreter:

Copyright (c) 2021, Arm Limited

<!--

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 (Aug 31 2021 at 12:26):

bjorn3 submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 31 2021 at 12:26):

bjorn3 created PR review comment:

Could you return an error in case of going out-of-bounds?

view this post on Zulip Wasmtime GitHub notifications bot (Aug 31 2021 at 12:30):

dheaton-arm submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 31 2021 at 12:30):

dheaton-arm created PR review comment:

In the docs, it's specified that the resulting lane should just be 0 for indices out of bounds - unless that's incorrect?

view this post on Zulip Wasmtime GitHub notifications bot (Aug 31 2021 at 12:41):

bjorn3 submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 31 2021 at 12:41):

bjorn3 created PR review comment:

Didn't know that. https://github.com/bytecodealliance/wasmtime/blob/16854e73c5bcd3323a51214b267f628ac41c9607/cranelift/codegen/src/isa/x64/lower.rs#L6339-L6343

view this post on Zulip Wasmtime GitHub notifications bot (Aug 31 2021 at 16:19):

abrown submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 31 2021 at 16:19):

abrown submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 31 2021 at 16:19):

abrown created PR review comment:

I'm slightly surprised that I allowed swizzles for types other than i8x16 (here). In the spirit of constraining the instruction set, this might be another place to consider pruning: it is a bit strange that one can send in an i8x16, rearrange it with the swizzle mask, and then have some other vector type. Any thoughts on this? @cfallin, @akirilov-arm, @jlb6740?

(In any case, this PR should merge as-is and we can remove these later if that's what we decide--thanks for finding this!).

view this post on Zulip Wasmtime GitHub notifications bot (Aug 31 2021 at 16:19):

abrown created PR review comment:

nit: when visually checking these values, it would be more helpful if they were in hex format so we could distinguish what lanes have what values.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 31 2021 at 16:27):

abrown edited PR review comment.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 31 2021 at 16:31):

afonso360 submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 31 2021 at 16:31):

afonso360 created PR review comment:

We can fix the old backend errors by replacing the target x86_64 with target x86_64 machinst.

This causes it to be ignored on the old style x86 backend.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 31 2021 at 16:34):

abrown submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 31 2021 at 16:34):

abrown created PR review comment:

Perfect, let's do that then.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 31 2021 at 16:38):

afonso360 submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 01 2021 at 14:31):

dheaton-arm updated PR #3268 from implement-swizzle-splat to main.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 03 2021 at 09:17):

dheaton-arm updated PR #3268 from implement-swizzle-splat to main.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 03 2021 at 16:28):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 03 2021 at 16:28):

cfallin created PR review comment:

Agreed!

view this post on Zulip Wasmtime GitHub notifications bot (Sep 03 2021 at 16:28):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 03 2021 at 16:28):

cfallin created PR review comment:

Yes, in principle I agree; I think we should be sort of systematic about this in a pass over all instructions, thinking about what types make sense, so I don't think we need to make an issue just for this, or adjust the test right now, but noted!

view this post on Zulip Wasmtime GitHub notifications bot (Sep 07 2021 at 12:32):

dheaton-arm updated PR #3268 from implement-swizzle-splat to main.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 07 2021 at 16:53):

abrown merged PR #3268.


Last updated: Nov 22 2024 at 16:03 UTC