addressed review feedback
http://codereview.chromium.org/6323002/diff/40001/src/arm/ic-arm.cc File src/arm/ic-arm.cc (right): http://codereview.chromium.org/6323002/diff/40001/src/arm/ic-arm.cc#newcode1395 src/arm/ic-arm.cc:1395: __ ldr(elements, FieldMemOperand(r1, JSObject::kElementsOffset)); On 2011/01/25 14:49:17, Mads Ager wrote:
We need to check that the receiver is a JSObject before we can load
the
elements. When the object is not a smi it has to be a heap object.
However,
there are heap objects that have no elements array (such as value
objects and
undefined and null).
I think you need to use GenerateKeyedReceiverCheck or something like
that. We
could also get an object that requires access checks or has a keyed
interceptor. Done. http://codereview.chromium.org/6323002/diff/40001/src/arm/ic-arm.cc#newcode1403 src/arm/ic-arm.cc:1403: __ ldr(ip, FieldMemOperand(elements, PixelArray::kLengthOffset)); On 2011/01/25 14:49:17, Mads Ager wrote:
Since we have enough register here, I would prefer not to use ip.
Using ip
always makes me a bit nervous because it is occasionally clobered by
the
macroassembler.
Done. http://codereview.chromium.org/6323002/diff/40001/src/arm/ic-arm.cc#newcode1409 src/arm/ic-arm.cc:1409: __ ldr(ip, FieldMemOperand(elements, PixelArray::kExternalPointerOffset)); On 2011/01/25 14:49:17, Mads Ager wrote:
Ditto for using ip.
Done. http://codereview.chromium.org/6323002/diff/40001/src/ia32/ic-ia32.cc File src/ia32/ic-ia32.cc (right): http://codereview.chromium.org/6323002/diff/40001/src/ia32/ic-ia32.cc#newcode775 src/ia32/ic-ia32.cc:775: // Check that the key is an array index, that is Uint32. On 2011/01/25 14:49:17, Mads Ager wrote:
This code looks more complicated than on ARM and IA32. Not sure I
understand why
this code would be faster than using basically the same code as on
x64? Done. http://codereview.chromium.org/6323002/diff/40001/src/ia32/ic-ia32.cc#newcode787 src/ia32/ic-ia32.cc:787: __ mov(ecx, FieldOperand(edx, JSObject::kElementsOffset)); On 2011/01/25 14:49:17, Mads Ager wrote:
Need a JSObject check.
Done. http://codereview.chromium.org/6323002/diff/40001/src/ic.cc File src/ic.cc (right): http://codereview.chromium.org/6323002/diff/40001/src/ic.cc#newcode1202 src/ic.cc:1202: stub = pixel_array_stub(); This occurred to me too, but I actually think we still need it... Otherwise how do we handle pixel arrays in the case of a call site that goes megamorphic? On 2011/01/25 14:49:17, Mads Ager wrote:
Let's remove the pixel array code in the generic stub now that we have
a special
IC stub?
http://codereview.chromium.org/6323002/diff/40001/src/x64/ic-x64.cc File src/x64/ic-x64.cc (right): http://codereview.chromium.org/6323002/diff/40001/src/x64/ic-x64.cc#newcode780 src/x64/ic-x64.cc:780: __ movq(rcx, FieldOperand(rdx, JSObject::kElementsOffset)); On 2011/01/25 14:49:17, Mads Ager wrote:
JSObject check needed.
Done. http://codereview.chromium.org/6323002/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
