Comments.

http://codereview.chromium.org/8258015/diff/7025/src/arm/full-codegen-arm.cc
File src/arm/full-codegen-arm.cc (right):

http://codereview.chromium.org/8258015/diff/7025/src/arm/full-codegen-arm.cc#newcode1534
src/arm/full-codegen-arm.cc:1534: __ CheckFastElements(r2, r3,
&double_elements);
This code checks the element kind for every stored computed value.  A
small improvement to this code is to check up front, save the kind
(tagged in the stack?) as a state, and update it when a transition is
taken.

http://codereview.chromium.org/8258015/diff/7025/src/arm/full-codegen-arm.cc#newcode1538
src/arm/full-codegen-arm.cc:1538: __ CheckFastSmiOnlyElements(r2, r3,
&fast_elements);
It's a bit confusing (to me) that the label is for *not* fast smi only
elements.  That's because it seems more natural, given the function
name, to interpret it as something like "branch on fast smi only
elements".

http://codereview.chromium.org/8258015/diff/7025/src/arm/full-codegen-arm.cc#newcode1550
src/arm/full-codegen-arm.cc:1550: __ CallRuntime(Runtime::kSetProperty,
5);
Save this for a future CL, but I think that it would be better to have a
custom function for exactly setting an element as part of initializing
an array literal.

The general SetProperty has a lot of stuff that we know we don't need
(checking attributes, strict vs. non-strict differences, proxies, etc.).

http://codereview.chromium.org/8258015/diff/7025/src/arm/full-codegen-arm.cc#newcode1556
src/arm/full-codegen-arm.cc:1556: __
StoreNumberToDoubleElements(result_register(),
My biggest concern is that this is a lot of generated code (for the
double store particularly, but also the write barrier and all the rest)
per-computed value.

It might be better to pass the value, array, index, and current element
kind to a stub (and get back a new element kind or update it in place),
where the stub is a little interpreter for array stores implemented in
assembly.

What do you think?

http://codereview.chromium.org/8258015/

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

Reply via email to