WebAssembly/spec

Implement the new NaN semantics

sunfishcode opened this issue · 10 comments

The reference interpreter and testsuite should be updated to reflect WebAssembly/design#660. I expect to work on this at some point, but I don't know when I'll find sufficient time. I'd be happy to help anyone else interested in working on this in the meantime.

One possible implementation would be to extend assert_return_nan. Currently, assert_return_nan tests for either negative or positive default NaN. If we made it also accept any nan that appears as an operand, but with the fraction field MSB set to 1, then we could use it both for cases like this where there are no nan operands:

f32.wast:
(assert_return_nan (invoke "add" (f32.const -infinity) (f32.const infinity))

and cases like this where there's a nan operand and we want to validate that we either get that operand with the fraction field MSB set to 1, or one of the default NaN values:

float_misc.wast:
(assert_return (invoke "f32.add" (f32.const nan:0x200000) (f32.const 1.0)) (f32.const nan:0x600000))

In general, there is no relation between arguments and result of an invoke, so looking at the arguments would perhaps be a bit weird. Instead, what if assert_result_nan simply allowed specifying a list of acceptable NaN values?

The list of acceptable NaN values is a function of the argument list, and it's the same function for all arithmetic operators (neg, abs, copysign notwithstanding), so it'd be very redundant to specify them in a separate list on each test. However it's just convenience for testcases, so I'd be ok either way.

That's the case for operators, but not for functions, which are what gets invoked in an assert. I can easily write one where the result has nothing whatsoever to do with the arguments. ;)

What I've proposed here is indeed a specialized mechanism. While specialized mechanisms typically have fewer uses than general-purpose mechanisms, they can typically offer those uses they do have greater advantages.

Specifically, my proposal would keep a large number of existing tests readable and maintainable, and makes it easier to add tests for new future operators, and makes it easier to tweak the NaN semantics (in case that becomes necessary, which is a very real possibility). The general-purpose mechanism proposed here would not have these advantages, even if it would have other advantages.

WebAssembly/design#716 is now merged in the design repo and has updated NaN propagation semantics.

Specifically, with the rules now in place, we need tests for:

  • Test that arithmetic instructions other than abs/neg/copysign correctly set the most-significant bit of the significand of a NaN result to 1 (in other words, convert "signaling NaN" to "quiet NaN"). We have some coverage of this for some operators, but not all.
  • Test that arithmetic instructions with no "non-canonical" NaN inputs return "canonical" NaNs. Note that "canonical" in wasm can have either value for the sign bit, so tests like (assert_return (invoke "sqrt" (f64.const -nan)) (f64.const -nan)) should be relaxed to accept either nan or -nan.

Testing that operators set the most-significant bit of NaN significands (the first bullet, above) is desirable for all operators, but it's particularly important for min/max because of the way these operators are often implemented on platforms that don't have native support.

rfk commented

I find myself needing this update in order to properly test a little wasm-to-js converter I've been toying with [1], so I'm going to try to brush off my ML and and give it a shot. IIUC there are three behaviours that need to be tested for:

  • Whether operators like abs/neg/copysign produce a specific NaN bitpattern
  • Whether other operators produce one of the two canonical NaNs when given only canonical inputs
  • Whether other operators produce valid quiet NaNs when given non-canonical inputs

Would it suffice to have three assert_return variants for these three cases?

  • assert_return, which is already used for testing for a specific NaN
  • assert_return_nan, which could accept any quiet NaN
  • assert_return_canonical_nan, which could accept only nan and -nan

Thoughts? I won't make any promises, but I'll try to make some time to poke at this over the next couple of days.

[1] https://github.com/rfk/wasm-polyfill

@rfk, thanks, sounds good to me!

With #414 merged, this is now fixed.