Addressed comments, ptal again
https://codereview.chromium.org/14850006/diff/42001/src/hydrogen-instructions.cc
File src/hydrogen-instructions.cc (right):
https://codereview.chromium.org/14850006/diff/42001/src/hydrogen-instructions.cc#newcode2525
src/hydrogen-instructions.cc:2525: if (map->is_deprecated()) continue;
On 2013/05/07 13:04:47, danno wrote:
Shouldn't be hitting this case? Maybe this should be an assert.
Done.
https://codereview.chromium.org/14850006/diff/42001/src/hydrogen-instructions.cc#newcode2538
src/hydrogen-instructions.cc:2538:
lookup.GetPropertyDetails().representation().IsDouble()) {
On 2013/05/07 13:04:47, danno wrote:
Do lookup.representation directly.
Done.
https://codereview.chromium.org/14850006/diff/42001/src/ia32/stub-cache-ia32.cc
File src/ia32/stub-cache-ia32.cc (right):
https://codereview.chromium.org/14850006/diff/42001/src/ia32/stub-cache-ia32.cc#newcode910
src/ia32/stub-cache-ia32.cc:910: __ mov(FieldOperand(receiver_reg,
offset), name_reg);
On 2013/05/07 13:04:47, danno wrote:
Give this name_reg guy another name?
Done.
https://codereview.chromium.org/14850006/diff/42001/src/ia32/stub-cache-ia32.cc#newcode1078
src/ia32/stub-cache-ia32.cc:1078: __ RecordWriteField(scratch1,
On 2013/05/07 13:04:47, danno wrote:
Remove the write barrier in the double case
Done. This was already done by returning in case of double. Cleaned up
unnecessary changes.
https://codereview.chromium.org/14850006/diff/42001/src/json-parser.h
File src/json-parser.h (right):
https://codereview.chromium.org/14850006/diff/42001/src/json-parser.h#newcode432
src/json-parser.h:432: JSObject::TransitionToMap(json_object, map);
On 2013/05/07 13:04:47, danno wrote:
AllocateStorageForMap... better name?
Done.
https://codereview.chromium.org/14850006/diff/42001/src/objects-inl.h
File src/objects-inl.h (right):
https://codereview.chromium.org/14850006/diff/42001/src/objects-inl.h#newcode290
src/objects-inl.h:290: MaybeObject* Object::StorageFor(Heap* heap,
Representation representation) {
On 2013/05/07 13:04:47, danno wrote:
AllocateNewStorageFor
Done.
https://codereview.chromium.org/14850006/diff/42001/src/objects.cc
File src/objects.cc (right):
https://codereview.chromium.org/14850006/diff/42001/src/objects.cc#newcode824
src/objects.cc:824: return heap->AllocateHeapNumber(value->Number());
On 2013/05/07 13:04:47, danno wrote:
If this happens too frequently (i.e. having to allocate the HeapNumber
after
calling FastPropertyAt, then shouldn't it actually be inside
FastPropertyAt? And
have a FastPropertyStoreAtRaw access the unboxed value (it would be
used less
frequently).
Done.
https://codereview.chromium.org/14850006/diff/42001/src/objects.cc#newcode1719
src/objects.cc:1719: Representation representation) {
On 2013/05/07 13:04:47, danno wrote:
Perhaps a comment that this is a transition case. How about using the
AllocateNewStorageFor method?
Done.
https://codereview.chromium.org/14850006/diff/42001/src/objects.cc#newcode1721
src/objects.cc:1721: if (FLAG_track_double_fields &&
representation.IsDouble()) {
It is the transition case; added comment.
On 2013/05/07 13:04:47, danno wrote:
Don't you just have to write into the existing box? This doesn't look
like a
transition case.
https://codereview.chromium.org/14850006/diff/42001/src/objects.cc#newcode1800
src/objects.cc:1800: if (map()->unused_property_fields() == 0) {
On 2013/05/07 13:04:47, danno wrote:
Add a TODO to merge the second half of this method with
AddFastPropertyUsingMap
(if possible). At least think about it.
Done.
https://codereview.chromium.org/14850006/diff/42001/src/objects.cc#newcode2291
src/objects.cc:2291: heap->AllocateHeapNumber(Smi::cast(value)->value(),
TENURED);
On 2013/05/07 13:04:47, danno wrote:
AllocateNewStorageFor
Done.
https://codereview.chromium.org/14850006/diff/42001/src/objects.cc#newcode3921
src/objects.cc:3921: result = lookup.holder()->AddFastPropertyUsingMap(
On 2013/05/07 13:04:47, danno wrote:
Can you please revert to self?
Done.
https://codereview.chromium.org/14850006/diff/42001/src/objects.cc#newcode5308
src/objects.cc:5308: MaybeObject* maybe_number =
heap->AllocateHeapNumber(value->Number());
On 2013/05/07 13:04:47, danno wrote:
NewStorageForValue
Done.
https://codereview.chromium.org/14850006/diff/42001/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/14850006/diff/42001/src/parser.cc#newcode4022
src/parser.cc:4022: }
On 2013/05/07 13:04:47, danno wrote:
*COUGH* Any other better way to do this? *COUGH*
Boolean would be better.
Done.
https://codereview.chromium.org/14850006/diff/42001/src/runtime.cc
File src/runtime.cc (right):
https://codereview.chromium.org/14850006/diff/42001/src/runtime.cc#newcode237
src/runtime.cc:237: Factory* factory = isolate->factory();
On 2013/05/07 13:04:47, danno wrote:
Don't need this change.
Done.
https://codereview.chromium.org/14850006/diff/42001/src/stub-cache.cc
File src/stub-cache.cc (right):
https://codereview.chromium.org/14850006/diff/42001/src/stub-cache.cc#newcode228
src/stub-cache.cc:228: field.translate(receiver),
On 2013/05/07 13:04:47, danno wrote:
Extraneous change?
Done.
https://codereview.chromium.org/14850006/diff/42001/src/stub-cache.cc#newcode1510
src/stub-cache.cc:1510: object, receiver(), holder, name, &miss);
On 2013/05/07 13:04:47, danno wrote:
Extraneous whitespace change
Done.
https://codereview.chromium.org/14850006/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
