I'm not sure I like the (eax=receiver,ecx=key) calling convention.
I definitely don't like changing the terminology "key" ==> "name" in the
keyed
ICs. I think it just invites confusion.
If you spot cleanups in assembly code while making these changes (newer
idioms
like CmpInstanceType or SmiTag), go ahead and put them in.
http://codereview.chromium.org/601080/diff/11/1014
File src/ia32/builtins-ia32.cc (right):
http://codereview.chromium.org/601080/diff/11/1014#newcode647
src/ia32/builtins-ia32.cc:647: __ mov(ecx, Operand(ebp, kIndexOffset));
So far, if we pass one register argument it is eax, and if we pass two
they are (edx,eax). Based on that I would expect the index in eax and
the arguments object in edx.
We should adopt something that's not arbitrary.
http://codereview.chromium.org/601080/diff/11/1014#newcode660
src/ia32/builtins-ia32.cc:660: // Remove IC arguments from the stack and
push the nth argument.
Please change this comment.
http://codereview.chromium.org/601080/diff/11/1014#newcode663
src/ia32/builtins-ia32.cc:663: // Update the index on the stack and in
register eax.
Please change this comment (if you use ecx).
http://codereview.chromium.org/601080/diff/11/1010
File src/ia32/codegen-ia32.cc (right):
http://codereview.chromium.org/601080/diff/11/1010#newcode6794
src/ia32/codegen-ia32.cc:6794: Result name = cgen_->frame()->Pop();
Doesn't
cgen_->frame()->PushElementAt(1);
cgen_->frame()->PushElementAt(1);
work? If not we should make it so.
http://codereview.chromium.org/601080/diff/11/1015
File src/ia32/ic-ia32.cc (right):
http://codereview.chromium.org/601080/diff/11/1015#newcode223
src/ia32/ic-ia32.cc:223: // -- ecx : name
Not really a "name", more like a "key".
http://codereview.chromium.org/601080/diff/11/1015#newcode245
src/ia32/ic-ia32.cc:245: __ movzx_b(edx, FieldOperand(edx,
Map::kInstanceTypeOffset));
These two instructions are
__ CmpInstanceType(edx, JS_OBJECT_TYPE);
http://codereview.chromium.org/601080/diff/11/1015#newcode252
src/ia32/ic-ia32.cc:252: __ sar(ebx, kSmiTagSize);
__ SmiUntag(ebx);
http://codereview.chromium.org/601080/diff/11/1015#newcode257
src/ia32/ic-ia32.cc:257: __ cmp(FieldOperand(edx,
HeapObject::kMapOffset),
__ CheckMap(edx, Factory::fixed_array_map(), &check_pixel_array, true)
http://codereview.chromium.org/601080/diff/11/1015#newcode265
src/ia32/ic-ia32.cc:265: Operand(edx, ebx, times_4,
FixedArray::kHeaderSize - kHeapObjectTag));
FieldOperand(edx, ebx, times_4, FixedArray::kHeaderSize)
http://codereview.chromium.org/601080/diff/11/1015#newcode280
src/ia32/ic-ia32.cc:280: __ cmp(FieldOperand(edx,
HeapObject::kMapOffset),
__ CheckMap(edx, Factory::pixel_array_map(), &slow, true);
http://codereview.chromium.org/601080/diff/11/1015#newcode318
src/ia32/ic-ia32.cc:318: __ cmp(FieldOperand(ebx,
HeapObject::kMapOffset),
CheckMap if you wish.
http://codereview.chromium.org/601080/diff/11/1015#newcode396
src/ia32/ic-ia32.cc:396: // -- ecx : name
key?
http://codereview.chromium.org/601080/diff/11/1015#newcode474
src/ia32/ic-ia32.cc:474: __ sar(edx, kSmiTagSize); // Untag the index.
__ SmiUntag(edx)
http://codereview.chromium.org/601080/diff/11/1015#newcode530
src/ia32/ic-ia32.cc:530: __ shl(eax, kSmiTagSize);
__ SmiTag(eax);
http://codereview.chromium.org/601080/diff/11/1015#newcode568
src/ia32/ic-ia32.cc:568: __ shl(eax, kSmiTagSize);
SmiTag.
http://codereview.chromium.org/601080/diff/11/1013
File src/ia32/virtual-frame-ia32.cc (right):
http://codereview.chromium.org/601080/diff/11/1013#newcode927
src/ia32/virtual-frame-ia32.cc:927: // Key and receiver are on top of
the frame. The IC expects them on
Comment is now wrong.
http://codereview.chromium.org/601080/diff/11/1013#newcode931
src/ia32/virtual-frame-ia32.cc:931: // Put arguments in registers, and
on stack.
Comment is wrong (no args are on the stack).
http://codereview.chromium.org/601080/diff/11/1013#newcode934
src/ia32/virtual-frame-ia32.cc:934: PrepareForCall(0, 0); // Two stack
args, neither callee-dropped.
Comment is wrong.
http://codereview.chromium.org/601080
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
