Closed Bug 1647288 Opened 4 years ago Closed 4 years ago

Wasm SIMD needs to distinguish NaN flavors

Categories

(Core :: JavaScript: WebAssembly, defect, P3)

x86_64
All
defect

Tracking

()

RESOLVED FIXED
81 Branch
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.

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).

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.

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());
Type: enhancement → defect
Assignee: nobody → lhansen
Status: NEW → ASSIGNED
Blocks: 1656214
Attached file runtest.sch (obsolete) —

POC: program that prints JS+wat that tests this. Scheme rules!

Attachment #9168188 - Attachment mime type: application/octet-stream → text/plain

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

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

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

Attached file generate-nan-tests.sch

Finished generator script.

Attachment #9168188 - Attachment is obsolete: true
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

Yeah ok, serves me right for not running all the tests before landing.

Flags: needinfo?(lhansen)
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
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 81 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: