First round of comments.

To improve the generated code for getting the EnumLength value, we should
consider moving that to the LSB of bit_field3 and then just masking away the
other bits. Thereby we get a perfectly SMI tagged value and safe a lot of
instructions and complexity. It seems we only use the tagged representation of
that value in generated code.


https://chromiumcodereview.appspot.com/10824079/diff/21/src/handles.cc
File src/handles.cc (right):

https://chromiumcodereview.appspot.com/10824079/diff/21/src/handles.cc#newcode726
src/handles.cc:726: // Count encountering the empty descriptor array as
a cache hit.
Hmm, "encountering" is an adjective, I would just drop the comment
completely.

https://chromiumcodereview.appspot.com/10824079/diff/21/src/handles.cc#newcode729
src/handles.cc:729: return
Handle<FixedArray>(isolate->factory()->empty_fixed_array());
No need for a copy constructor of the handle here!

https://chromiumcodereview.appspot.com/10824079/diff/21/src/handles.cc#newcode742
src/handles.cc:742: storage =
isolate->factory()->NewFixedArray(num_enum);
Can we just merge the handle declarations and the allocations into one
line?

https://chromiumcodereview.appspot.com/10824079/diff/21/src/heap.cc
File src/heap.cc (right):

https://chromiumcodereview.appspot.com/10824079/diff/21/src/heap.cc#newcode2078
src/heap.cc:2078: Map::EnumLengthBits::update(
This is a little convoluted. Better use ...

map->set_bit_field3(Map::LastAddedBits::encode(Map::kNoneAdded) |

Map::EnumLengthBits::encode(Map::kInvalidEnumCache));

https://chromiumcodereview.appspot.com/10824079/diff/21/src/hydrogen-instructions.h
File src/hydrogen-instructions.h (right):

https://chromiumcodereview.appspot.com/10824079/diff/21/src/hydrogen-instructions.h#newcode109
src/hydrogen-instructions.h:109: V(MapEnumLength)
     \
Alpha-sort!

https://chromiumcodereview.appspot.com/10824079/diff/21/src/ia32/full-codegen-ia32.cc
File src/ia32/full-codegen-ia32.cc (right):

https://chromiumcodereview.appspot.com/10824079/diff/21/src/ia32/full-codegen-ia32.cc#newcode1093
src/ia32/full-codegen-ia32.cc:1093: Label no_descriptors;
Move this up above the __ bind() but below the comment. Also drop all
the empty newlines around the label. The label isn't the central thing
here, the assembler instructions are.

https://chromiumcodereview.appspot.com/10824079/diff/21/src/ia32/lithium-ia32.h
File src/ia32/lithium-ia32.h (right):

https://chromiumcodereview.appspot.com/10824079/diff/21/src/ia32/lithium-ia32.h#newcode93
src/ia32/lithium-ia32.h:93: V(MapEnumLength)
 \
Alpha-sort!

https://chromiumcodereview.appspot.com/10824079/diff/21/src/objects.h
File src/objects.h (right):

https://chromiumcodereview.appspot.com/10824079/diff/21/src/objects.h#newcode4888
src/objects.h:4888: int LastAdded() {
Can we move LastAdded() up just above SetLastAdded()?

https://chromiumcodereview.appspot.com/10824079/

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

Reply via email to