Some further testing revealed that this change still have bugs. So it will
not
be committed just now.
http://codereview.chromium.org/1735007/diff/15001/16001
File src/arm/codegen-arm.cc (right):
http://codereview.chromium.org/1735007/diff/15001/16001#newcode5282
src/arm/codegen-arm.cc:5282: // Setup the name register and call load
IC.
On 2010/04/23 12:21:24, Mads Ager wrote:
All arguments to keyed load ic are on the stack.
Done.
http://codereview.chromium.org/1735007/diff/15001/16001#newcode5286
src/arm/codegen-arm.cc:5286: // kayed load has been inlined.
On 2010/04/23 14:56:47, Erik Corry wrote:
kayed -> keyed.
Done.
http://codereview.chromium.org/1735007/diff/15001/16001#newcode5362
src/arm/codegen-arm.cc:5362: // Parts of this code is patched, so the
exact instructions generated needs
On 2010/04/23 14:56:47, Erik Corry wrote:
Parts ... is -> Parts ... are
instructions ... needs -> instructions ... need
Done.
http://codereview.chromium.org/1735007/diff/15001/16001#newcode5363
src/arm/codegen-arm.cc:5363: // to be fixed. Therefore the instruction
pool is blocked when generating
On 2010/04/23 12:21:24, Mads Ager wrote:
instruction -> constant
Done.
http://codereview.chromium.org/1735007/diff/15001/16001#newcode5388
src/arm/codegen-arm.cc:5388: // Check that the key is a heap object.
On 2010/04/23 12:21:24, Mads Ager wrote:
Check that the key is a smi. And as Anton comments you should check
r1. Looks
like we need another test case if we are not hitting this with our
current
tests.
Done.
http://codereview.chromium.org/1735007/diff/15001/16001#newcode5389
src/arm/codegen-arm.cc:5389: __ tst(r0, Operand(kSmiTagMask));
On 2010/04/23 10:37:41, antonm wrote:
r0 is an object, no? Should it be r1 and check that key is smi?
And sorry for being pushy but there is BranchOnSmi
Done.
http://codereview.chromium.org/1735007/diff/15001/16001#newcode5389
src/arm/codegen-arm.cc:5389: __ tst(r0, Operand(kSmiTagMask));
On 2010/04/23 14:56:47, Erik Corry wrote:
On 2010/04/23 10:37:41, antonm wrote:
> r0 is an object, no? Should it be r1 and check that key is smi?
>
> And sorry for being pushy but there is BranchOnSmi
I don't think it works for deferred. Perhaps we should have
BranchOnSmi on
deferred.
There is no special branch methods for deferred code.
http://codereview.chromium.org/1735007/diff/15001/16001#newcode5393
src/arm/codegen-arm.cc:5393: // is not a dictionary.
On 2010/04/23 14:56:47, Erik Corry wrote:
Can it become a dictionary without changing the map?
I don't think changing the representation of the elements is reflected
in the map. Thats only for properties.
http://codereview.chromium.org/1735007/diff/15001/16001#newcode5396
src/arm/codegen-arm.cc:5396: __ mov(r4,
Operand(Factory::fixed_array_map()));
On 2010/04/23 12:21:24, Mads Ager wrote:
Use root array?
Done.
http://codereview.chromium.org/1735007/diff/15001/16001#newcode5406
src/arm/codegen-arm.cc:5406: __ mov(r3,
Operand(Factory::the_hole_value()));
On 2010/04/23 12:21:24, Mads Ager wrote:
Use root array?
Done.
http://codereview.chromium.org/1735007/diff/15001/16001#newcode5410
src/arm/codegen-arm.cc:5410: __ cmp(r1, r3);
On 2010/04/23 14:56:47, Erik Corry wrote:
Surely you should be comparing r3 against r0 here?
Absolutely.
http://codereview.chromium.org/1735007/show
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
