Comments addressed. I extended custom generators to support the call global
case. Please take another look.


Thanks,
Vitaly



http://codereview.chromium.org/3291015/diff/1/2
File src/arm/stub-cache-arm.cc (right):

http://codereview.chromium.org/3291015/diff/1/2#newcode1532
src/arm/stub-cache-arm.cc:1532: Register result = r0;
On 2010/09/06 18:26:34, antonm wrote:
looks like result is only used at 1553, maybe don't declare an alias
for it as
it confuses me slightly---both receiver and result share the same
register

This is now rewritten.

http://codereview.chromium.org/3291015/diff/1/2#newcode1535
src/arm/stub-cache-arm.cc:1535: __ ldr(code, MemOperand(sp, 0 *
kPointerSize));
On 2010/09/06 18:26:34, antonm wrote:
just curious: could it be faster to batch pop registers on fast path
and push
them back when going slow case?

Probably yes. But I'm not happy about the extra complexity and deviation
from the intel code.

http://codereview.chromium.org/3291015/diff/1/2#newcode1540
src/arm/stub-cache-arm.cc:1540: Handle<Map>(string_function->map()),
On 2010/09/06 18:26:34, antonm wrote:
should we do map check or object identity check?  I am slightly
concerned that
you bail out in compile time if objects is not String, but in runtime
check is
somewhat different.

And chances are identity check could be faster (we don't share this
code between
contexts, do we?)

As we discussed, identity checks won't work. Anyway this place has been
rewritten as well.

http://codereview.chromium.org/3291015/diff/1/2#newcode1546
src/arm/stub-cache-arm.cc:1546: STATIC_ASSERT(kStringTag == 0);
On 2010/09/06 18:26:34, antonm wrote:
typo: kStringTag -> kSmiTag?

Oops! Fixed.

http://codereview.chromium.org/3291015/diff/1/2#newcode1551
src/arm/stub-cache-arm.cc:1551: __ and_(code, code,
Operand(Smi::FromInt(0xffff)));
On 2010/09/06 18:26:34, antonm wrote:
just curious: is it a proper way to convert negative numbers?

I think so. This is covered by the new tests.

http://codereview.chromium.org/3291015/diff/1/4
File src/ia32/stub-cache-ia32.cc (right):

http://codereview.chromium.org/3291015/diff/1/4#newcode1735
src/ia32/stub-cache-ia32.cc:1735: __ and_(code, (0xffff << kSmiTagSize)
| kSmiTag);
On 2010/09/06 18:26:34, antonm wrote:
why not FromInt as in ARM case?

No such and_ in assembler-ia32.

http://codereview.chromium.org/3291015/diff/1/8
File test/mjsunit/string-fromcharcode.js (right):

http://codereview.chromium.org/3291015/diff/1/8#newcode48
test/mjsunit/string-fromcharcode.js:48: assertEquals(expected,
receiver.fromCharCode(0x20 + 0.5));
On 2010/09/06 18:26:34, antonm wrote:
maybe negative indices cases should be added.

I had one already (0x20 - 0x10000) but now the coverage is extended.

http://codereview.chromium.org/3291015/show

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

Reply via email to