All recommended changes made.  Interface changed to:
key: eax
receiver: edx

Some uses of SmiTag, SmiUntag, MapCheck added as suggested.



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));
On 2010/02/17 12:50:52, Kevin Millikin wrote:
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.

OK: receiver in edx, index in eax.

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();
On 2010/02/17 12:50:52, Kevin Millikin wrote:
Doesn't

cgen_->frame()->PushElementAt(1);
cgen_->frame()->PushElementAt(1);

work?  If not we should make it so.

Done.

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
On 2010/02/17 12:50:52, Kevin Millikin wrote:
Not really a "name", more like a "key".

Done.

http://codereview.chromium.org/601080/diff/11/1015#newcode245
src/ia32/ic-ia32.cc:245: __ movzx_b(edx, FieldOperand(edx,
Map::kInstanceTypeOffset));
On 2010/02/17 12:50:52, Kevin Millikin wrote:
These two instructions are

__ CmpInstanceType(edx, JS_OBJECT_TYPE);

Done.

http://codereview.chromium.org/601080/diff/11/1015#newcode252
src/ia32/ic-ia32.cc:252: __ sar(ebx, kSmiTagSize);
On 2010/02/17 12:50:52, Kevin Millikin wrote:
__ SmiUntag(ebx);

Done.

http://codereview.chromium.org/601080/diff/11/1015#newcode257
src/ia32/ic-ia32.cc:257: __ cmp(FieldOperand(edx,
HeapObject::kMapOffset),
On 2010/02/17 12:50:52, Kevin Millikin wrote:
__ CheckMap(edx, Factory::fixed_array_map(), &check_pixel_array, true)

Done.

http://codereview.chromium.org/601080/diff/11/1015#newcode265
src/ia32/ic-ia32.cc:265: Operand(edx, ebx, times_4,
FixedArray::kHeaderSize - kHeapObjectTag));
On 2010/02/17 12:50:52, Kevin Millikin wrote:
FieldOperand(edx, ebx, times_4, FixedArray::kHeaderSize)

Done.

http://codereview.chromium.org/601080

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

Reply via email to