Stream: git-wasmtime

Topic: wasmtime / PR #5977 aarch64: Add specialized `shuffle` lo...


view this post on Zulip Wasmtime GitHub notifications bot (Mar 10 2023 at 03:28):

alexcrichton opened PR #5977 from aarch64-shuffles to main:

This is the equivalent of https://github.com/bytecodealliance/wasmtime/pull/5930 but for AArch64. I went through various instructions I saw for AArch64 and added corresponding shuffle lowerings where appropriate. These lowerings cover all the lowerings I found in the meshoptimizer repository plus a few more based on various instructions I found while perusing ARM's documentation. Like with x86_64 I've tried to make sure there's a runtest and a precise-output test for each lowering, even if some of them probably overlap with the x86_64 runtests.

I'll note that many of these lowerings probably won't end up getting used by "portable" wasm binaries since some of the shifts here are pretty specific to AArch64 and don't have efficient 1/2 instruction lowerings on x86_64. That being said these are useful to any sort of hypothetical Cranelift-as-an-AArch64-backend-compiler such as rustc_cranelift_codegen since this broadens the spectrum of instructions supported by Cranelift's AArch64 backend.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 10 2023 at 15:35):

alexcrichton updated PR #5977 from aarch64-shuffles to main.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 10 2023 at 19:15):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 10 2023 at 19:15):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 10 2023 at 19:15):

cfallin created PR review comment:

I wonder if it would make these patterns clearer to have an extractor something like (shuffle_immediate 30 28 26 ...) (with external Rust impl that is Fn(&mut self, imm: Immediate) -> Option<(u8, u8, u8, u8, ...)>)?

view this post on Zulip Wasmtime GitHub notifications bot (Mar 10 2023 at 19:15):

cfallin created PR review comment:

Can we add doc comments here to describe what pattern in the Immediate each of these etors matches on? (Likewise below)

view this post on Zulip Wasmtime GitHub notifications bot (Mar 10 2023 at 20:11):

alexcrichton submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 10 2023 at 20:11):

alexcrichton created PR review comment:

I originally did this in https://github.com/bytecodealliance/wasmtime/pull/5905 but @jameysharp preferred the hex masks instead. I don't mind myself, but I do think it's worth being consistent across the backends so I'd want to update all the x64 things if these aarch64 rules change as wlel.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 10 2023 at 20:12):

jameysharp submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 10 2023 at 20:12):

jameysharp created PR review comment:

Funny, I suggested exactly the opposite in a previous PR :laughing:

view this post on Zulip Wasmtime GitHub notifications bot (Mar 10 2023 at 20:12):

alexcrichton updated PR #5977 from aarch64-shuffles to main.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 10 2023 at 20:24):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 10 2023 at 20:24):

cfallin created PR review comment:

Interesting!

I see the points in #5905 now about exposing more opportunity to islec by making the full mask visible as one value; that's a reasonable argument I think. My rationale was that I was having some friction converting hex values in my head to understand the permutation (but maybe the right answer to that is just to think in hex directly). I don't feel too strongly about it, so this is fine as-is.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 10 2023 at 22:19):

alexcrichton merged PR #5977.


Last updated: Nov 22 2024 at 17:03 UTC