Normative: Allow NaN values to be optionally canoncalized #758

littledan · 2016-12-28T16:06:02Z

This patch legalizes V8's occasional canonicalization of NaNs
by changing SetValueInBuffer to either a particular value
for the implementation-distinguishable NaN value or an
implementation-defined canonical value.

This semantic change reached consensus at the November 2016
TC39 meeting.

Closes #635

This patch still needs the associated test262 update. The tests that are affected include these, but maybe more:

built-ins/Object/internals/DefineOwnProperty/nan-equivalence
built-ins/TypedArray/prototype/fill/fill-values-conversion-operations-consistent-nan
built-ins/TypedArrays/internals/DefineOwnProperty/conversion-operation-consistent-nan
built-ins/TypedArrays/internals/Set/conversion-operation-consistent-nan

littledan · 2016-12-28T16:06:30Z

cc @erights @allenwb

bterlson · 2016-12-29T18:19:47Z

@allenwb does this seem fine to you? (Just checking as I think you were involved in the discussions closely at the last meeting).

littledan · 2018-03-05T10:40:00Z

What do we need from here to decide whether to land this patch?

@jfbastien has clarified that, even if this patch lands, we won't really have a full description of what happens with NaN values in practice. This is because processors sometimes change the NaN bit pattern later, e.g., in MOV instructions, in a way that JS implementations which don't canonicalize are probably not guarding against.

Even as I'd personally prefer that we leave the NaN bit pattern completely unspecified, I think this patch still brings us a little bit closer to reality and would be helpful to land.

rwaldron · 2018-03-07T20:07:04Z

spec.html

@@ -33539,7 +33539,7 @@ <h1>SetValueInBuffer ( _arrayBuffer_, _byteIndex_, _type_, _value_ [ , _isLittle
          1. If _type_ is `"Float32"`, then
            1. Set _rawBytes_ to a List containing the 4 bytes that are the result of converting _value_ to IEEE 754-2008 binary32 format using &ldquo;Round to nearest, ties to even&rdquo; rounding mode. If _isLittleEndian_ is *false*, the bytes are arranged in big endian order. Otherwise, the bytes are arranged in little endian order. If _value_ is *NaN*, _rawValue_ may be set to any implementation chosen IEEE 754-2008 binary32 format Not-a-Number encoding. An implementation must always choose the same encoding for each implementation distinguishable *NaN* value.
          1. Else if _type_ is `"Float64"`, then
-            1. Set _rawBytes_ to a List containing the 8 bytes that are the IEEE 754-2008 binary64 format encoding of _value_. If _isLittleEndian_ is *false*, the bytes are arranged in big endian order. Otherwise, the bytes are arranged in little endian order. If _value_ is *NaN*, _rawValue_ may be set to any implementation chosen IEEE 754-2008 binary64 format Not-a-Number encoding. An implementation must always choose the same encoding for each implementation distinguishable *NaN* value.
+            1. Set _rawBytes_ to a List containing the 8 bytes that are the IEEE 754-2008 binary64 format encoding of _value_. If _isLittleEndian_ is *false*, the bytes are arranged in big endian order. Otherwise, the bytes are arranged in little endian order. If _value_ is *NaN*, _rawValue_ may be set to any implementation chosen IEEE 754-2008 binary64 format Not-a-Number encoding. An implementation must always choose either the same encoding for each implementation distinguishable *NaN* value, or an implementation-defined canonical value.


I see that this change is only being made to the Float64 branch, should it also be applied to the Float32 branch just above?

allenwb

A agree with Rick. It should also apply to the Float32 case

This patch legalizes V8's occasional canonicalization of NaNs by changing SetValueInBuffer to *either* a particular value for the implementation-distinguishable NaN value *or* an implementation-defined canonical value. This semantic change reached consensus at the November 2016 TC39 meeting. Closes tc39#635

@rwaldron

Becasue I still agree with @rwaldron

allenwb

I agree with Rick.

In the Float32 case a Number value which may be a NaN is converted to a Float32 so a 32-bit NaN needs to be created from the 64-bing NaN. The bit encoding of the 32-bit NaN is observable in the buffer so we want the choice of encoding to be consistent within an implementaiton, just like for Float32.

rwaldron · 2018-03-08T18:25:20Z

spec.html

@@ -35819,9 +35819,9 @@ <h1>NumberToRawBytes( _type_, _value_, _isLittleEndian_ )</h1>
        <p>The abstract operation NumberToRawBytes takes three parameters, a String _type_, a Number _value_, and a Boolean _isLittleEndian_. This operation performs the following steps:</p>
        <emu-alg>
          1. If _type_ is `"Float32"`, then
-            1. Let _rawBytes_ be a List containing the 4 bytes that are the result of converting _value_ to IEEE 754-2008 binary32 format using &ldquo;Round to nearest, ties to even&rdquo; rounding mode. If _isLittleEndian_ is *false*, the bytes are arranged in big endian order. Otherwise, the bytes are arranged in little endian order. If _value_ is *NaN*, _rawBytes_ may be set to any implementation chosen IEEE 754-2008 binary32 format Not-a-Number encoding. An implementation must always choose the same encoding for each implementation distinguishable *NaN* value.
+            1. Let _rawBytes_ be a List containing the 4 bytes that are the result of converting _value_ to IEEE 754-2008 binary32 format using &ldquo;Round to nearest, ties to even&rdquo; rounding mode. If _isLittleEndian_ is *false*, the bytes are arranged in big endian order. Otherwise, the bytes are arranged in little endian order. If _value_ is *NaN*, _rawBytes_ may be set to any implementation chosen IEEE 754-2008 binary32 format Not-a-Number encoding. An implementation must always choose the same encoding for each implementation distinguishable *NaN* value, or an implementation-defined canonical value.


littledan · 2018-03-08T21:10:39Z

Thanks for the reviews everyone!

ljharb · 2019-04-26T00:02:33Z

@littledan would you mind rebasing this PR, and summarizing the current status? I believe it has consensus, and i'm not sure if it still needs tests.

@rwaldron can you rereview after the PR is rebased?

littledan · 2019-04-26T14:27:13Z

Well, I would not feel comfortable making the change suggested above, to give more guarantees to Float32, given that this is unlikely to correspond to actual implementations. I would rather move this patch to removing any guarantees about how floats are canonicalized, given the information from @jfbastien, about how this is the implementation reality and is unlikely to change. @waldemarhorwat encouraged this path as well. @erights, would you be OK with this change?

erights · 2019-04-26T15:55:03Z

Yes, this change looks good to me.

littledan · 2019-04-26T17:10:32Z

@erights To be concrete, I'm talking about, not this PR, but instead a PR to say that there's no stability for NaN (since this is the implementation reality).

erights · 2019-04-27T02:03:13Z

@erights To be concrete, I'm talking about, not this PR, but instead a PR to say that there's no stability for NaN (since this is the implementation reality).

Thanks for clarifying. I did not know that. Link to other PR?

erights approved these changes Dec 28, 2016

View reviewed changes

bterlson added needs consensus This needs committee consensus before it can be eligible to be merged. needs test262 tests The proposal should specify how to test an implementation. Ideally via github.com/tc39/test262 labels Dec 29, 2016

bterlson removed the needs consensus This needs committee consensus before it can be eligible to be merged. label Dec 29, 2016

bterlson requested a review from allenwb January 5, 2017 19:15

littledan mentioned this pull request Mar 6, 2018

Weaken tests about NaN canonicalization tc39/test262#1476

Closed

rwaldron suggested changes Mar 7, 2018

View reviewed changes

allenwb suggested changes Mar 7, 2018

View reviewed changes

littledan force-pushed the nan-relax branch from 1862a3b to 0718a90 Compare March 8, 2018 16:01

allenwb previously approved these changes Mar 8, 2018

View reviewed changes

allenwb reviewed Mar 8, 2018

View reviewed changes

rwaldron reviewed Mar 8, 2018

View reviewed changes

ljharb added the normative change Affects behavior required to correctly evaluate some ECMAScript source text label Apr 26, 2019

littledan mentioned this pull request Jan 3, 2020

Should ECMAScript implementations be allowed to canonicalize NaN when writing into an Array? #635

Open

AndrewKhizhnyak approved these changes Jan 22, 2020

View reviewed changes

ljharb force-pushed the master branch 3 times, most recently from 3d0c24c to 7a79833 Compare June 29, 2021 02:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Normative: Allow NaN values to be optionally canoncalized #758

Normative: Allow NaN values to be optionally canoncalized #758

littledan commented Dec 28, 2016

littledan commented Dec 28, 2016

bterlson commented Dec 29, 2016

littledan commented Mar 5, 2018 •

edited

Loading

rwaldron Mar 7, 2018

littledan Mar 8, 2018

allenwb left a comment

allenwb left a comment

rwaldron Mar 8, 2018

littledan commented Mar 8, 2018

ljharb commented Apr 26, 2019

littledan commented Apr 26, 2019

erights commented Apr 26, 2019

littledan commented Apr 26, 2019

erights commented Apr 27, 2019

Normative: Allow NaN values to be *optionally* canoncalized #758

Are you sure you want to change the base?

Normative: Allow NaN values to be *optionally* canoncalized #758

Conversation

littledan commented Dec 28, 2016

littledan commented Dec 28, 2016

bterlson commented Dec 29, 2016

littledan commented Mar 5, 2018 • edited Loading

rwaldron Mar 7, 2018

Choose a reason for hiding this comment

littledan Mar 8, 2018

Choose a reason for hiding this comment

allenwb left a comment

Choose a reason for hiding this comment

allenwb left a comment

Choose a reason for hiding this comment

rwaldron Mar 8, 2018

Choose a reason for hiding this comment

littledan commented Mar 8, 2018

ljharb commented Apr 26, 2019

littledan commented Apr 26, 2019

erights commented Apr 26, 2019

littledan commented Apr 26, 2019

erights commented Apr 27, 2019

Normative: Allow NaN values to be optionally canoncalized #758

Normative: Allow NaN values to be optionally canoncalized #758

littledan commented Mar 5, 2018 •

edited

Loading