Stream: git-wasmtime

Topic: wasmtime / PR #2067 Enable the spec::simd::simd_lane test...

Wasmtime GitHub notifications bot (Jul 23 2020 at 14:32):

akirilov-arm opened PR #2067 from simd_lane to main:

This PR enables the spec::simd::simd_lane test for AArch64. Of particular note is that it was necessary to make compromises in the Shuffle implementation because the TBL instruction requires consecutive registers for the table.

Wasmtime GitHub notifications bot (Jul 27 2020 at 21:18):

cfallin submitted PR Review.

Wasmtime GitHub notifications bot (Jul 27 2020 at 21:18):

cfallin created PR Review Comment:

Could we expand these doc comments (on VecTbl and VecTbl2) a bit? I went to the manual to understand what tbl/tbx do; but it would be helpful to summarize. Especially the is_extension bit that alters the way that rd is treated (full overwrite vs. conditional one), and the requirement that rn2 is immediately after rn for VecTbl2.

Wasmtime GitHub notifications bot (Jul 27 2020 at 21:18):

cfallin submitted PR Review.

Wasmtime GitHub notifications bot (Jul 27 2020 at 21:18):

cfallin created PR Review Comment:

I'd prefer for this helper to retain the Option<u128> return value -- it forces the caller to handle the wrong-instruction-type case explicitly with something like const_param_to_u128(...).expect("Invalid const insn for i128 const") if it should be infallible, or do something else if the input may or may not be a constant.

Actually, ideally, we'd handle u128 constants with the same machinery as u64 ones, via the constant field on LowerCtx::get_input()'s returned information. This would let us dedup and handle constant pools in the machine-independent code. I'm happy to defer that to later, though!

Wasmtime GitHub notifications bot (Jul 30 2020 at 20:59):

akirilov-arm submitted PR Review.

Wasmtime GitHub notifications bot (Jul 30 2020 at 20:59):

akirilov-arm created PR Review Comment:

Just to check my understanding of your idea (otherwise I'd go for the simpler solution for the time being) - I don't think that it would help us for Opcode::Shuffle because the mask is not an input from the point of view of LowerCtx::get_input() (Opcode::Vconst is different, of course).

As for the constant pool improvements - yes, I was keeping that in mind and I think that concentrating the constant handling in as few places as possible would help eventually, hence why I made the changes to this helper. In fact I don't think I understand the practical distinction between DataFlowGraph::constants and DataFlowGraph::immediates - at least in the cases of Opcode::Vconst and Opcode::Shuffle the handling is pretty much equivalent.

Wasmtime GitHub notifications bot (Jul 31 2020 at 02:26):

cfallin submitted PR Review.

Wasmtime GitHub notifications bot (Jul 31 2020 at 02:26):

cfallin created PR Review Comment:

Yeah, I am fine with the design suggested in the first paragraph -- just return Option<u128>. I need to do more work to make use of the constant-pool functionality in MachBuffer, so we can revisit this later.

Wasmtime GitHub notifications bot (Aug 04 2020 at 15:13):

akirilov-arm updated PR #2067 from simd_lane to main:

This PR enables the spec::simd::simd_lane test for AArch64. Of particular note is that it was necessary to make compromises in the Shuffle implementation because the TBL instruction requires consecutive registers for the table.

Wasmtime GitHub notifications bot (Aug 06 2020 at 11:43):

akirilov-arm updated PR #2067 from simd_lane to main:

This PR enables the spec::simd::simd_lane test for AArch64. Of particular note is that it was necessary to make compromises in the Shuffle implementation because the TBL instruction requires consecutive registers for the table.

Wasmtime GitHub notifications bot (Aug 06 2020 at 17:33):

cfallin merged PR #2067.

Last updated: Dec 23 2024 at 12:05 UTC