One real comment.

https://chromiumcodereview.appspot.com/10816005/diff/9055/src/arm/macro-assembler-arm.cc
File src/arm/macro-assembler-arm.cc (right):

https://chromiumcodereview.appspot.com/10816005/diff/9055/src/arm/macro-assembler-arm.cc#newcode3704
src/arm/macro-assembler-arm.cc:3704: Register
empty_descriptor_array_value = r7;
Drop the "empty_descriptor_array_value".

https://chromiumcodereview.appspot.com/10816005/diff/9055/src/arm/macro-assembler-arm.cc#newcode3726
src/arm/macro-assembler-arm.cc:3726:
LoadRoot(empty_descriptor_array_value,
Better use r7 directly here, so that it's clear that CheckMap above will
destroy it.

https://chromiumcodereview.appspot.com/10816005/diff/9055/src/factory.cc
File src/factory.cc (right):

https://chromiumcodereview.appspot.com/10816005/diff/9055/src/factory.cc#newcode123
src/factory.cc:123: void Factory::SetDescriptors(Handle<Map> map,
This method should definitely be moved into the "Map" class and be made
static.

https://chromiumcodereview.appspot.com/10816005/diff/9055/src/mark-compact.cc
File src/mark-compact.cc (left):

https://chromiumcodereview.appspot.com/10816005/diff/9055/src/mark-compact.cc#oldcode1884
src/mark-compact.cc:1884: void
Marker<T>::MarkDescriptorArray(DescriptorArray* descriptors) {
Oh yeah! I like that!

https://chromiumcodereview.appspot.com/10816005/diff/9055/src/objects.cc
File src/objects.cc (right):

https://chromiumcodereview.appspot.com/10816005/diff/9055/src/objects.cc#newcode5253
src/objects.cc:5253: if (!(*HeapObject::RawField(transition_array,
The HeapObject::map() method should still work here. Also can we turn
this into one single condition ...

if (!transition_array->map()->IsSmi() &&
    !transition_array->IsTransitionArray()) {
  return NULL;
}

https://chromiumcodereview.appspot.com/10816005/

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to