Wasm SIMD needs to distinguish NaN flavors
Categories
(Core :: JavaScript: WebAssembly, defect, P3)
Tracking
()
Tracking | Status | |
---|---|---|
firefox81 | --- | fixed |
People
(Reporter: lth, Assigned: lth)
References
(Blocks 1 open bug)
Details
Attachments
(4 files, 1 obsolete file)
When the wasm SIMD specs got translated into our harness, the various flavors of NaN were translated simply as "NaN" and the checking that's done when we run tests all check for any kind of NaN. But some of the test cases are specific about which flavor of NaN they want - specifically, some want canonical NaN (100...000) and some want arithmetic NaN (1xxx...xxx). I guess our checking amounts to always testing for arithmetic NaN.
Assignee | ||
Comment 1•4 years ago
|
||
See https://webassembly.github.io/spec/core/syntax/values.html#floating-point for a definition.
See https://webassembly.github.io/spec/core/text/values.html?highlight=canonical#floating-point for a definition of source syntax for various NaN values.
See https://webassembly.github.io/spec/core/exec/numerics.html?highlight=canonical#nan-propagation for rules about NaN propagation in scalar code, which SIMD code should obey (SIMD is invariant wrt scalarization).
Assignee | ||
Comment 2•4 years ago
•
|
||
Some discussion on IM with Benjamin: this is tricky territory. He notes:
f32x4::min does minps twice, and then orps; if one input is a canonical NaN, and the second is not zero and not a NaN, then the output is not canonical
which would seem to violate the NaN propagation rules.
Comment 3•4 years ago
|
||
Yep, confirmed with the following test case:
let { exports } = new WebAssembly.Instance(
new WebAssembly.Module(wasmTextToBinary(`
(module
(func (export "f") (result i64)
;; create a vec of canonical NaNs
f64.const nan
f64x2.splat
;; create a vec with a non-zero, non NaN value
f64.const 42
f64x2.splat
;; take the minimum
f64x2.min
;; extract lane 0, it should be a canonical NaN.
f64x2.extract_lane 0
i64.reinterpret_f64
)
(func (export "canonical_nan") (result i64)
f64.const nan
i64.reinterpret_f64
)
)
`))
);
assertEq(exports.f(), exports.canonical_nan());
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 4•4 years ago
|
||
POC: program that prints JS+wat that tests this. Scheme rules!
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 5•4 years ago
|
||
These test cases were generated by a script from some of the
preliminary test cases in the SIMD spec repository, taking into
account the specific NaN types asked for.
These tests are temporary: once we have proper generated test cases
from the spec repository, these will no longer be needed.
Depends on D86315
Assignee | ||
Comment 6•4 years ago
|
||
Wasm treats signalling and quiet NaN the same - as quiet NaN. Where
convenient, test also signalling NaN. This is complicated by JS not
being able to represent signalling NaN directly.
Depends on D86316
Assignee | ||
Comment 7•4 years ago
|
||
This adds correct NaN handling to the SIMD f32x4/f64x2.min/max code.
This is a bit of a horror show actually. There is a reasonable fast
path if neither operand contains a NaN, but the slow path to handle
NaN is long and there's a lot of code. (This is an Intel-only
problem, on other architectures there's a direct mapping.)
It is possible the slow-path code could be somewhat improved (both
speed and size) by using at least three BLEND instructions, but I
consider that a possible optimization that needs investigation and
empirical backing. Meanwhile, we can land this plausible code.
Depends on D86317
Assignee | ||
Comment 8•4 years ago
|
||
Finished generator script.
Pushed by lhansen@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/64e82366834a Handle NaN in SIMD min, max: Generated test cases. r=jseward https://hg.mozilla.org/integration/autoland/rev/bfd5e17cb4d0 Handle signalling NaN generally: Test cases. r=jseward https://hg.mozilla.org/integration/autoland/rev/c19dcb11b940 Handle NaN in SIMD min, max: Code. r=jseward
Comment 10•4 years ago
|
||
Backed out for SM bustages.
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=312801749&repo=autoland
https://treeherder.mozilla.org/logviewer.html#?job_id=312801766&repo=autoland
Backout link: https://hg.mozilla.org/integration/autoland/rev/2ce43b1fe3db5a3121cb01e5ea3045da7b85b1c9
Assignee | ||
Comment 11•4 years ago
|
||
Yeah ok, serves me right for not running all the tests before landing.
Comment 12•4 years ago
|
||
Pushed by lhansen@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/f1cdbe186a15 Handle NaN in SIMD min, max: Generated test cases. r=jseward https://hg.mozilla.org/integration/autoland/rev/2fa2317aa431 Handle signalling NaN generally: Test cases. r=jseward https://hg.mozilla.org/integration/autoland/rev/0edbc05d65ff Handle NaN in SIMD min, max: Code. r=jseward
Comment 13•4 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/f1cdbe186a15
https://hg.mozilla.org/mozilla-central/rev/2fa2317aa431
https://hg.mozilla.org/mozilla-central/rev/0edbc05d65ff
Description
•