tc39 / ecmascript_simd

SIMD numeric type for EcmaScript
Other
543 stars 64 forks source link

Semantics of minNum and maxNum don't match IEEE 754-2008 for signaling NaNs #341

Open stoklund opened 8 years ago

stoklund commented 8 years ago

The IEEE 754-2008 operations minNum and maxNum suppress quiet NaNs, but they still propagate signaling NaNs, canonicalising them to qNaNs:

minNum(1.0, qNaN) -> 1.0
minNum(1.0, sNaN) -> qNaN

The ARMv8 vminnm and vmaxnm instructions have the same semantics (my emphasis):

It handles NaNs in consistence with the IEEE754-2008 specification. It returns the numerical operand when one operand is numerical and the other is a quiet NaN, but otherwise the result is identical to floating-point VMAX.

The minNum and maxNum operations currently in our specification don't distinguish between sNaNs and qNaNs. Given our approach to NaNs, it seems difficult to add the distinction.

I would suggest that we either:

  1. Rename minNum and maxNum to avoid confusion with the IEEE operations of the same name, or
  2. Remove the two functions from the specification.
ljharb commented 8 years ago

The API needs to be consistent with JS developer expectations as well as SIMD developer expectations - and as such, differentiable NaNs aren't anything I'd think anybody would expect. What's the hazard you see from leaving it as-is?

jfbastien commented 8 years ago

The intent of minNum and maxNum is to follow IEEE754-2008 semantics precisely.

ljharb commented 8 years ago

Right, but JS doesn't have qNaN or sNaN, just NaN.

jfbastien commented 8 years ago

Right, JS adds non-determinism on top because the NaN values aren't guaranteed, but one of the goals of the SIMD spec is (IIUC?) to be shareable with WebAssembly where the NaN values are observable. WebAssembly has its own NaN observability specification which provides more guarantees. From that POV, I think minNum and maxNum should therefore follow IEEE754-2008 closely, and then the SIMD spec has a generic "here's what happens to NaNs" on top of everything.

sunfishcode commented 8 years ago

This issue exists independently of the shared spec idea or WebAssembly. It comes down to this:

When we added minNum and maxNum to SIMD.js, we thought we were specifying semantics compatible with the relevant subset of IEEE 754-2008, which is why we used IEEE 754-2008's names. Now it turns out we were mistaken; they aren't actually compatible in the way we thought.

Making these functions aware of signaling NaN would be one way to address this issue, however that would have significant implications for the rest of the SIMD.js spec and probably for JS as well, as it would require us to add semantics to many other places, so that developers could reason about where signaling NaNs might appear.

The suggestion above is that we either just rename these functions, to avoid confusion, since minNum and maxNum are names that IEEE 754-2008 coined, or that we (perhaps temporarily) remove them from SIMD.js. At present, removing them seems like the safest option to me; we can always reintroduce them in the future.

ljharb commented 8 years ago

Without those methods, how can I compare two SIMD values and determine the smaller or larger? The process document specifies that the only changes expected in Stage 3 are "Limited: only those deemed critical based on implementation experience" - this does not fall into that bucket.

I don't care what they're called, but the comparative APIs to Math.min and Math.max imo are crucial to include.

sunfishcode commented 8 years ago

The comparative APIs to Math.min and Math.max are already in the SIMD spec, under the names min and max, and they have the same semantics as their Math counterparts in JS (and similar functions in Java and other languages).

minNum and maxNum have different semantics, and were added specifically to provide IEEE 754 compatible functionality (and similar functions in C and other languages). It is these functions which are under discussion here.

ljharb commented 8 years ago

aha, ok thanks for clarifying (it's a big API 😉).

Given that, I'm withdrawing my opinion from the discussion 😄

nmostafa commented 8 years ago

I am in favor of removing them from SIMD.js, since their semantics doesn't serve any purpose now (neither JS or IEEE compliant). We can add them to WASM after semantics fix.

stoklund commented 8 years ago

I wrote a blog post about the consequences of having non-associative min / max operations.