Reviewers: Jakob,
Message:
I had to add changes to ignore element transitions in map check where
"appropriate" (where the check is guarding an array length access), this
part of
the CL is new and should be reviewed as such.
I also addressed all code review comments (trivial changes).
https://chromiumcodereview.appspot.com/11299004/diff/1/src/hydrogen.cc
File src/hydrogen.cc (right):
https://chromiumcodereview.appspot.com/11299004/diff/1/src/hydrogen.cc#newcode6669
src/hydrogen.cc:6669: fast_mode = false;
On 2012/11/16 12:57:10, Jakob wrote:
Why do you always set this to false?
Because in this case I will emit an instance check instead of a map
check, so I really cannot know if the arrays that will be handled will
be in fast more or not.
https://chromiumcodereview.appspot.com/11299004/diff/1/src/ic.cc
File src/ic.cc (right):
https://chromiumcodereview.appspot.com/11299004/diff/1/src/ic.cc#newcode987
src/ic.cc:987: // we have IC support only for getting the array length
property.
On 2012/11/16 12:57:10, Jakob wrote:
nit: capital W
Done.
https://chromiumcodereview.appspot.com/11299004/diff/1/src/ic.cc#newcode988
src/ic.cc:988: if (holder->IsJSArray() &&
On 2012/11/16 12:57:10, Jakob wrote:
nit: You can avoid one level of indentation here by moving this up
into a new
"else if" block, and keeping the original "else { ASSERT(...); return;
}" after
that.
Done.
https://chromiumcodereview.appspot.com/11299004/diff/1/src/objects.h
File src/objects.h (right):
https://chromiumcodereview.appspot.com/11299004/diff/1/src/objects.h#newcode8356
src/objects.h:8356: class PropertyIndex;
On 2012/11/16 12:57:10, Jakob wrote:
nit: two empty lines between top-level definitions (i.e. two before
"class
PropertyIndex" and two after).
Done.
https://chromiumcodereview.appspot.com/11299004/diff/1/test/mjsunit/array-length.js
File test/mjsunit/array-length.js (right):
https://chromiumcodereview.appspot.com/11299004/diff/1/test/mjsunit/array-length.js#newcode132
test/mjsunit/array-length.js:132: var getLength = function(a) { return
a.length; }
On 2012/11/16 12:57:10, Jakob wrote:
What's the point of doing this? Are you trying to get a fresh copy of
the
function with uninitialized ICs?
Yes, now I am creating new functions every time.
https://chromiumcodereview.appspot.com/11299004/diff/1/test/mjsunit/array-length.js#newcode147
test/mjsunit/array-length.js:147: length = 4;
On 2012/11/16 12:57:10, Jakob wrote:
unused variable?
No, I am setting the 'length property' of the global object.
Now I added a comment to state it :-)
Description:
Use the property load IC for accessing the array length.
BUG=
Please review this at https://chromiumcodereview.appspot.com/11299004/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files:
M src/arm/lithium-codegen-arm.cc
M src/hydrogen-instructions.h
M src/hydrogen.cc
M src/ia32/lithium-codegen-ia32.cc
M src/ic.cc
M src/mips/lithium-codegen-mips.cc
M src/objects-inl.h
M src/objects.h
M src/x64/lithium-codegen-x64.cc
M test/mjsunit/array-length.js
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
