abrown opened PR #1371 from rounding-average
to master
:
- [x] This has not been discussed in an issue.
- [x] A short description of what this does, why it is needed: implements Wasm SIMD's lane-wise integer rounding average (see https://github.com/WebAssembly/simd/blob/master/proposals/simd/SIMD.md#lane-wise-integer-rounding-average).
- [x] This PR contains test cases, if meaningful.
- [x] A reviewer from the core maintainer team has been assigned for this PR.
abrown requested bnjbvr for a review on PR #1371.
bnjbvr submitted PR Review.
bnjbvr submitted PR Review.
bnjbvr created PR Review Comment:
Can the parser understand the hexadecimal syntax? If so, we should use it here, and then we wouldn't need the comment anymore.
bnjbvr created PR Review Comment:
nit: byte => word
bnjbvr created PR Review Comment:
Should we use the more current abbreviation "avg", or just be explicit and call it "average" (it's not very long)? This is the first time I see this midway abbreviation for average, but I'm not the English native here.
bjorn3 submitted PR Review.
bjorn3 created PR Review Comment:
I believe a word is 16 bits, not 8.
bjorn3 deleted PR Review Comment.
abrown submitted PR Review.
abrown created PR Review Comment:
The complication here is that the semantics are actually "rounding average" instead of just "average"; I went along with the term ["avgr" used by the spec](fhttps://github.com/WebAssembly/simd/blob/master/proposals/simd/SIMD.md#lane-wise-integer-minimum). Do you want to expand it to "rounding_average" or "average_round"?
abrown edited PR Review Comment.
abrown created PR Review Comment:
No, not yet for
i16
,i32
, andi64
; here's an issue to fix that: https://github.com/bytecodealliance/cranelift/issues/1401.
abrown submitted PR Review.
abrown updated PR #1371 from rounding-average
to master
:
- [x] This has not been discussed in an issue.
- [x] A short description of what this does, why it is needed: implements Wasm SIMD's lane-wise integer rounding average (see https://github.com/WebAssembly/simd/blob/master/proposals/simd/SIMD.md#lane-wise-integer-rounding-average).
- [x] This PR contains test cases, if meaningful.
- [x] A reviewer from the core maintainer team has been assigned for this PR.
bnjbvr submitted PR Review.
bnjbvr created PR Review Comment:
Good point! Maybe then
avg_round
is a nice tradeoff? "avg" is standard for average, and we make the rounding part very explicit.
abrown updated PR #1371 from rounding-average
to master
:
- [x] This has not been discussed in an issue.
- [x] A short description of what this does, why it is needed: implements Wasm SIMD's lane-wise integer rounding average (see https://github.com/WebAssembly/simd/blob/master/proposals/simd/SIMD.md#lane-wise-integer-rounding-average).
- [x] This PR contains test cases, if meaningful.
- [x] A reviewer from the core maintainer team has been assigned for this PR.
abrown updated PR #1371 from rounding-average
to master
:
- [x] This has not been discussed in an issue.
- [x] A short description of what this does, why it is needed: implements Wasm SIMD's lane-wise integer rounding average (see https://github.com/WebAssembly/simd/blob/master/proposals/simd/SIMD.md#lane-wise-integer-rounding-average).
- [x] This PR contains test cases, if meaningful.
- [x] A reviewer from the core maintainer team has been assigned for this PR.
abrown updated PR #1371 from rounding-average
to master
:
- [x] This has not been discussed in an issue.
- [x] A short description of what this does, why it is needed: implements Wasm SIMD's lane-wise integer rounding average (see https://github.com/WebAssembly/simd/blob/master/proposals/simd/SIMD.md#lane-wise-integer-rounding-average).
- [x] This PR contains test cases, if meaningful.
- [x] A reviewer from the core maintainer team has been assigned for this PR.
abrown merged PR #1371.
Last updated: Nov 22 2024 at 16:03 UTC