Implement proposed new identity spec.

Fix for issue 6678: Result of identical(a, b) with a and b doubles not consistent.

Implement proposed new identity spec (flag --new_identity_spec). It will be enabled/committed once the new spec been confirmed.

The new spec says that identity of numbers is computed on their values instead of on the object reference. This provides deterministic behavior without having to disable optimizations in the VM. In some cases 'identical' is now slower than 'equality' as more checks need to be done. The VM compiler can optimize 'identical' in the future.

My benchmarks do not show any slow down.
Committed: https://code.google.com/p/dart/source/detail?r=15332

[Florian Schneider, 2012-11-23 10:33:40]

LGTM with comments from the compiler side.

https://codereview.chromium.org/11414136/diff/8002/runtime/vm/flow_graph_comp...
File runtime/vm/flow_graph_compiler_ia32.cc (right):

https://codereview.chromium.org/11414136/diff/8002/runtime/vm/flow_graph_comp...
runtime/vm/flow_graph_compiler_ia32.cc:1119: } else {
Maybe reduce the nesting level by using

} else if (needs_number_check) {

here.

https://codereview.chromium.org/11414136/diff/8002/runtime/vm/flow_graph_comp...
runtime/vm/flow_graph_compiler_ia32.cc:1123: __
call(&StubCode::IdenticalWithNumberCheckLabel());
Maybe add comment that the stub return result in the flags-register.

https://codereview.chromium.org/11414136/diff/8002/runtime/vm/flow_graph_comp...
File runtime/vm/flow_graph_compiler_x64.cc (right):

https://codereview.chromium.org/11414136/diff/8002/runtime/vm/flow_graph_comp...
runtime/vm/flow_graph_compiler_x64.cc:1123: } else {
} else if ...

https://codereview.chromium.org/11414136/diff/8002/runtime/vm/flow_graph_comp...
runtime/vm/flow_graph_compiler_x64.cc:1127: __
call(&StubCode::IdenticalWithNumberCheckLabel());
Please add comment that result is in flags.

https://codereview.chromium.org/11414136/diff/8002/runtime/vm/intermediate_la...
File runtime/vm/intermediate_language.cc (right):

https://codereview.chromium.org/11414136/diff/8002/runtime/vm/intermediate_la...
runtime/vm/intermediate_language.cc:1800: const intptr_t kNumTemps = 00;
s/00/0/

https://codereview.chromium.org/11414136/diff/8002/runtime/vm/intermediate_la...
runtime/vm/intermediate_language.cc:1810: // Special code for doubles (compare
values instead of references.)
s/doubles/numbers/

https://codereview.chromium.org/11414136/diff/8002/runtime/vm/stub_code_ia32.cc
File runtime/vm/stub_code_ia32.cc (right):

https://codereview.chromium.org/11414136/diff/8002/runtime/vm/stub_code_ia32....
runtime/vm/stub_code_ia32.cc:2185: __ cmpl(temp, FieldAddress(right,
Mint::value_offset() + 0 * kWordSize));
right has yet not been checked for kMintCid here.

https://codereview.chromium.org/11414136/diff/8002/runtime/vm/stub_code_ia32....
runtime/vm/stub_code_ia32.cc:2206: __ Bind(&reference_compare);
Please add a comment that the code relies on the assumption that there is never
a bigint containing an integer in mint- or smi-range and that a mint never
contains an integer in smi-range.

Also I'm not sure if we assert this property in all places where we allocate
Mints and Bigints.

https://codereview.chromium.org/11414136/diff/8002/runtime/vm/stub_code_x64.cc
File runtime/vm/stub_code_x64.cc (right):

https://codereview.chromium.org/11414136/diff/8002/runtime/vm/stub_code_x64.c...
runtime/vm/stub_code_x64.cc:2140: __ cmpq(left, FieldAddress(right,
Mint::value_offset()));
right has not yet been checked for kMintCid here. Please add a test for this
case that covers both platforms.

https://codereview.chromium.org/11414136/diff/8002/runtime/vm/stub_code_x64.c...
runtime/vm/stub_code_x64.cc:2158: __ Bind(&reference_compare);
Please add a comment that the code relies on the assumption that there is never
a bigint containing an integer in mint- or smi-range and that a mint never
contains an integer in smi-range.

https://codereview.chromium.org/11414136/diff/8002/tests/language/number_iden...
File tests/language/number_identity_test.dart (right):

https://codereview.chromium.org/11414136/diff/8002/tests/language/number_iden...
tests/language/number_identity_test.dart:25: // Expect.isFalse(identical(a, b));
 // Fails on dart2js, since bigint.
Maybe group the tests where dart2js currently differs from the VM in a separate
file, so that we can mark just those in the test expectations file.

[srdjan, 2012-11-23 19:45:39]

https://codereview.chromium.org/11414136/diff/8002/runtime/vm/flow_graph_comp...
File runtime/vm/flow_graph_compiler_ia32.cc (right):

https://codereview.chromium.org/11414136/diff/8002/runtime/vm/flow_graph_comp...
runtime/vm/flow_graph_compiler_ia32.cc:1119: } else {
On 2012/11/23 10:33:40, Florian Schneider wrote:
> Maybe reduce the nesting level by using
> 
> } else if (needs_number_check) {
> 
> here.

changed to use returns instead of else.

https://codereview.chromium.org/11414136/diff/8002/runtime/vm/flow_graph_comp...
runtime/vm/flow_graph_compiler_ia32.cc:1123: __
call(&StubCode::IdenticalWithNumberCheckLabel());
On 2012/11/23 10:33:40, Florian Schneider wrote:
> Maybe add comment that the stub return result in the flags-register.

Done.

https://codereview.chromium.org/11414136/diff/8002/runtime/vm/flow_graph_comp...
File runtime/vm/flow_graph_compiler_x64.cc (right):

https://codereview.chromium.org/11414136/diff/8002/runtime/vm/flow_graph_comp...
runtime/vm/flow_graph_compiler_x64.cc:1123: } else {
On 2012/11/23 10:33:40, Florian Schneider wrote:
> } else if ...

Done.

https://codereview.chromium.org/11414136/diff/8002/runtime/vm/flow_graph_comp...
runtime/vm/flow_graph_compiler_x64.cc:1127: __
call(&StubCode::IdenticalWithNumberCheckLabel());
On 2012/11/23 10:33:40, Florian Schneider wrote:
> Please add comment that result is in flags.

Done.

https://codereview.chromium.org/11414136/diff/8002/runtime/vm/intermediate_la...
File runtime/vm/intermediate_language.cc (right):

https://codereview.chromium.org/11414136/diff/8002/runtime/vm/intermediate_la...
runtime/vm/intermediate_language.cc:1800: const intptr_t kNumTemps = 00;
On 2012/11/23 10:33:40, Florian Schneider wrote:
> s/00/0/

Done.

https://codereview.chromium.org/11414136/diff/8002/runtime/vm/intermediate_la...
runtime/vm/intermediate_language.cc:1810: // Special code for doubles (compare
values instead of references.)
On 2012/11/23 10:33:40, Florian Schneider wrote:
> s/doubles/numbers/

Done.

https://codereview.chromium.org/11414136/diff/8002/runtime/vm/stub_code_ia32.cc
File runtime/vm/stub_code_ia32.cc (right):

https://codereview.chromium.org/11414136/diff/8002/runtime/vm/stub_code_ia32....
runtime/vm/stub_code_ia32.cc:2185: __ cmpl(temp, FieldAddress(right,
Mint::value_offset() + 0 * kWordSize));
On 2012/11/23 10:33:40, Florian Schneider wrote:
> right has yet not been checked for kMintCid here.

Done.

https://codereview.chromium.org/11414136/diff/8002/runtime/vm/stub_code_ia32....
runtime/vm/stub_code_ia32.cc:2206: __ Bind(&reference_compare);
On 2012/11/23 10:33:40, Florian Schneider wrote:
> Please add a comment that the code relies on the assumption that there is
never
> a bigint containing an integer in mint- or smi-range and that a mint never
> contains an integer in smi-range.
> 
> Also I'm not sure if we assert this property in all places where we allocate
> Mints and Bigints.

Added comment at the beginning of method:
// Note: A Mint cannot contain a value that would fit in Smi, a Bigint
// cannot contain a value that fits in Mint or Smi.

If that is violated somewhere in the VM we will have crashes. I will verify that
assumption.

https://codereview.chromium.org/11414136/diff/8002/runtime/vm/stub_code_x64.cc
File runtime/vm/stub_code_x64.cc (right):

https://codereview.chromium.org/11414136/diff/8002/runtime/vm/stub_code_x64.c...
runtime/vm/stub_code_x64.cc:2140: __ cmpq(left, FieldAddress(right,
Mint::value_offset()));
On 2012/11/23 10:33:40, Florian Schneider wrote:
> right has not yet been checked for kMintCid here. Please add a test for this
> case that covers both platforms.

Done.

https://codereview.chromium.org/11414136/diff/8002/runtime/vm/stub_code_x64.c...
runtime/vm/stub_code_x64.cc:2158: __ Bind(&reference_compare);
On 2012/11/23 10:33:40, Florian Schneider wrote:
> Please add a comment that the code relies on the assumption that there is
never
> a bigint containing an integer in mint- or smi-range and that a mint never
> contains an integer in smi-range.

Added comment and the head of the function.

https://codereview.chromium.org/11414136/diff/8002/tests/language/number_iden...
File tests/language/number_identity_test.dart (right):

https://codereview.chromium.org/11414136/diff/8002/tests/language/number_iden...
tests/language/number_identity_test.dart:25: // Expect.isFalse(identical(a, b));
 // Fails on dart2js, since bigint.
On 2012/11/23 10:33:40, Florian Schneider wrote:
> Maybe group the tests where dart2js currently differs from the VM in a
separate
> file, so that we can mark just those in the test expectations file.

Added two test files:
- one is for NaN, where we would expect dart2js to pass
- one is for working with bigints which is not supportable with dart2js


Also added appropriate status files changes and added more combined tests, using
mint value that is mint in x64 as well.