Replace HeapNumber as doublebox with an explicit MutableHeapNumber.

Replace HeapNumber as doublebox with an explicit MutableHeapNumber.

BUG=

[Toon Verwaest, 2014-03-26 10:21:39]

PTAL

[Igor Sheludko, 2014-03-27 11:30:15]

First round of comments.

BTW, what if GVN decide to merge two equal mutable heap numbers?

https://codereview.chromium.org/211333002/diff/20001/src/deoptimizer.cc
File src/deoptimizer.cc (right):

https://codereview.chromium.org/211333002/diff/20001/src/deoptimizer.cc#newco...
src/deoptimizer.cc:1911:
HeapNumber::cast(*value)->set_map(isolate_->heap()->heap_number_map());
I think it is worth adding a comment here about why mutable heap number turns
immutable.

https://codereview.chromium.org/211333002/diff/20001/src/heap.cc
File src/heap.cc (right):

https://codereview.chromium.org/211333002/diff/20001/src/heap.cc#newcode2964
src/heap.cc:2964: MaybeObject* Heap::AllocateMutableHeapNumber(double value,
This code is the same as AllocateHeapNumber() except map value. Probably it
would be better to extend AllocateHeapNumber() with a enum {IMMUTABLE,MUTABLE}
parameter. 
Same note applies to MacroAssembler and Factory.

https://codereview.chromium.org/211333002/diff/20001/src/hydrogen.cc
File src/hydrogen.cc (right):

https://codereview.chromium.org/211333002/diff/20001/src/hydrogen.cc#newcode5374
src/hydrogen.cc:5374: HEAP_NUMBER_TYPE);
MUTABLE_HEAP_NUMBER_TYPE?

https://codereview.chromium.org/211333002/diff/20001/src/hydrogen.cc#newcode9912
src/hydrogen.cc:9912: Add<HAllocate>(heap_number_constant, HType::HeapNumber(),
Don't we need a HType::MutableHeapNumber() thing?

https://codereview.chromium.org/211333002/diff/20001/src/hydrogen.cc#newcode9913
src/hydrogen.cc:9913: pretenure_flag, HEAP_NUMBER_TYPE);
MUTABLE_HEAP_NUMBER_TYPE?

https://codereview.chromium.org/211333002/diff/20001/src/hydrogen.cc#newcode9918
src/hydrogen.cc:9918: Add<HConstant>(Handle<HeapNumber>::cast(value)->value());
value->Number()

https://codereview.chromium.org/211333002/diff/20001/src/ia32/macro-assembler...
File src/ia32/macro-assembler-ia32.h (right):

https://codereview.chromium.org/211333002/diff/20001/src/ia32/macro-assembler...
src/ia32/macro-assembler-ia32.h:652: void AllocateMutableHeapNumber(Register
result,
What about other platforms we support?

https://codereview.chromium.org/211333002/diff/20001/src/ic.cc
File src/ic.cc (right):

https://codereview.chromium.org/211333002/diff/20001/src/ic.cc#newcode677
src/ic.cc:677: return isolate->factory()->heap_number_map();
I'm afraid we could loose the knowledge about mutability of the values.

https://codereview.chromium.org/211333002/diff/20001/src/ic.cc#newcode690
src/ic.cc:690: if (map->instance_type() == HEAP_NUMBER_TYPE) {
What about supporting MUTABLE_HEAP_NUMBER_TYPE case here?

https://codereview.chromium.org/211333002/diff/20001/src/objects-inl.h
File src/objects-inl.h (right):

https://codereview.chromium.org/211333002/diff/20001/src/objects-inl.h#newcod...
src/objects-inl.h:289: } else if (IsMutableHeapNumber()) {
If you extend IsNumber() to return true for mutable heap numbers then you will
not need this special case as Number() will handle this case automagically.

https://codereview.chromium.org/211333002/diff/20001/src/objects-inl.h#newcod...
src/objects-inl.h:305: return
heap->AllocateHeapNumber(HeapNumber::cast(this)->value());
Same here: use Number().

https://codereview.chromium.org/211333002/diff/20001/src/objects-inl.h#newcod...
src/objects-inl.h:615: return IsSmi() || IsHeapNumber();
|| IsMutableHeapNumber() ?

https://codereview.chromium.org/211333002/diff/20001/src/objects-inl.h#newcod...
src/objects-inl.h:1964: return raw_value->WrapForRead(GetHeap(),
representation);
Shouldn't we also repack mutable heap number to immutable in LoadIC?

https://codereview.chromium.org/211333002/diff/20001/src/objects-printer.cc
File src/objects-printer.cc (right):

https://codereview.chromium.org/211333002/diff/20001/src/objects-printer.cc#n...
src/objects-printer.cc:95: case MUTABLE_HEAP_NUMBER_TYPE:
I think you can join this case with HEAP_NUMBER_TYPE case.

https://codereview.chromium.org/211333002/diff/20001/src/objects.h
File src/objects.h (right):

https://codereview.chromium.org/211333002/diff/20001/src/objects.h#newcode1498
src/objects.h:1498: } else if (FLAG_track_double_fields && IsHeapNumber()) {
|| IsMutableHeapNumber() ?

[Toon Verwaest, 2014-03-31 09:36:01]

Addressed comments, PTAL again.

[Igor Sheludko, 2014-03-31 11:19:04]

lgtm with nits:

https://codereview.chromium.org/211333002/diff/80001/src/json-parser.h
File src/json-parser.h (right):

https://codereview.chromium.org/211333002/diff/80001/src/json-parser.h#newcod...
src/json-parser.h:416: // double, use the existing box.
Please update comment to match the code.

https://codereview.chromium.org/211333002/diff/80001/src/objects-debug.cc
File src/objects-debug.cc (right):

https://codereview.chromium.org/211333002/diff/80001/src/objects-debug.cc#new...
src/objects-debug.cc:294: if (r.IsDouble())
ASSERT(value->IsMutableHeapNumber());
ASSERT(r.IsDouble() == value->IsMutableHeapNumber()); instead of if(...) ?

https://codereview.chromium.org/211333002/diff/80001/test/cctest/test-heap-pr...
File test/cctest/test-heap-profiler.cc (right):

https://codereview.chromium.org/211333002/diff/80001/test/cctest/test-heap-pr...
test/cctest/test-heap-profiler.cc:1628: CHECK(js_n_prop ==
heap_profiler->FindObjectById(n_prop->GetId()));
Probably it would be less confusing to check that number properties are not
visible to the heap profiler:
CHECK(n_prop == NULL) or something like that. 
Or just remove the n_prop because currently the test performs the same checks
for both s_prop and n_prop.