test/relaxed_dot: add i16-intermediate overflow boundary cases - #2228
Open
matthargett wants to merge 1 commit into
Open
test/relaxed_dot: add i16-intermediate overflow boundary cases#2228matthargett wants to merge 1 commit into
matthargett wants to merge 1 commit into
Conversation
The existing relaxed dot-product cases all use byte values whose i16 pair sums stay within i16 range, so an implementation that skips the i16 intermediate entirely (summing four byte products directly into i32 in the add variant) produces identical results on every existing input and the deviation is unobservable. Add a = b = -128 cases where each pair sum is 32768, one past the i16 maximum: for i16x8.relaxed_dot_i8x16_i7x16_s every allowed interpretation yields either -32768 (wrapping) or 32767 (saturating), and for i32x4.relaxed_dot_i8x16_i7x16_add_s the allowed lane values are -65536, -1, or 65534 -- while the no-i16-intermediate shape produces 65536, outside the set, so the boundary is now observable. Ported from WebAssembly/relaxed-simd#164 per maintainer guidance that the proposal repository is merged and inactive.
|
I did a quick review of V8 and most of the code should handle this correctly. I do see one potential issue with the wasm interpreter (not used by chrome) and |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The existing relaxed dot-product cases all use byte values whose i16 pair sums stay within i16 range (e.g.
-128 * -127 * 2 = 32512). An implementation that skips the i16 intermediate — summing four byte products directly into i32 in the add variant — therefore produces identical results on every existing input, and the deviation from the spec'srelaxedi16 truncation point is unobservable. (I hit exactly this implementation bug while porting the instructions to an interpreter; the current suite passed it.)This adds
a = b = -128boundary cases where each pair sum is32768, one pastINT16_MAX:i16x8.relaxed_dot_i8x16_i7x16_s: every allowed interpretation yields-32768(wrapping, including the x86PMADDUBSWsigned-saturating path, which saturates at-32768for these inputs) or32767(saturating), asserted witheither.i32x4.relaxed_dot_i8x16_i7x16_add_swithc = 0: the pair-wise i16 results feedextadd_pairwise(which cannot overflow i32), so the allowed lane values are-65536(wrap+wrap),-1(wrap+sat), or65534(sat+sat). The no-i16-intermediate shape produces65536, which is outside the allowed set — making the truncation point observable.Both files validate with
wast2json --enable-all.I hit this when benchmarking SIMD workloads across multiple open source WASM runtimes on iPhone XS / Apple Watch SE2. Two of them had this implementation bug, but the CTS passed on both. (I already submitted fixes to both projects, and they're both merged.)
Originally opened as WebAssembly/relaxed-simd#164; relocated here per maintainer guidance that the proposal repository is merged and inactive.