Builtin function EQULAS partially rewritten in C++. Control branches that cal...

Builtin function EQULAS partially rewritten in C++. Control branches that call toString/valueOf are still handled in JS.

[Søren Thygesen Gjesse, 2010-03-02 14:32:17]

Is this change in preparation for direct runtime calls or does it provide
performance improvement in itself?

http://codereview.chromium.org/661369/diff/1/4
File src/runtime.cc (right):

http://codereview.chromium.org/661369/diff/1/4#newcode8335
src/runtime.cc:8335: static inline Object* ObjectEquals(Object* x, Object* y) {
ObjectEquals -> ObjectStrictEquals?

http://codereview.chromium.org/661369/diff/1/4#newcode8354
src/runtime.cc:8354: // in a builting function EQUAL (copmaritions implying
calling custom JS
copmaritions -> comparisons

http://codereview.chromium.org/661369/diff/1/4#newcode8373
src/runtime.cc:8373: ASSERT(SPECIAL_CASE_CODE > 1); \
1 > NOT_EQUAL.

http://codereview.chromium.org/661369/diff/1/4#newcode8379
src/runtime.cc:8379: // (the last one could be also a string, so it must be
checked before).
it must be checked before -> null check must be before string check.

http://codereview.chromium.org/661369/diff/1/4#newcode8396
src/runtime.cc:8396: return Smi::FromInt(4);  // Speclal case: y =
%ToPrimitive(y, NO_HINT).
Please use constants for these special return values. You can add the same
constants in macros.py to have them available in the JavaScript natives.

http://codereview.chromium.org/661369/diff/1/4#newcode8400
src/runtime.cc:8400: TO_NUMBER(y, 3);  // y is not null.
Why not just use a function here, e.g.

bool supported = false;
dy = ObjectToNumber(y, supported);
if (!supported) return ...;

The macro which relies on existing variable names and generates a return makes
this more complicated than necessary.

http://codereview.chromium.org/661369/diff/1/5
File src/runtime.h (right):

http://codereview.chromium.org/661369/diff/1/5#newcode129
src/runtime.h:129: F(EqualsBase, 2, 1) \
How about calling this FastObjectEquals. Fast indicates that this is fast case
handling and that everything isn't handled.

[SeRya, 2010-03-02 15:54:22]

> Is this change in preparation for direct runtime calls or does it provide
performance improvement in itself?

It is for performance. But it affect calling runtime functions in the following
way. Majority of functions suitable to call without Exit Frame related to
comparison. So it's an optimization on the higher level and will probably make
redundant optimization on the lower level. Especially if the FastEquals will
work without CEntryStub.

http://codereview.chromium.org/661369/diff/1/4
File src/runtime.cc (right):

http://codereview.chromium.org/661369/diff/1/4#newcode8335
src/runtime.cc:8335: static inline Object* ObjectEquals(Object* x, Object* y) {
On 2010/03/02 14:32:19, Søren Gjesse wrote:
> ObjectEquals -> ObjectStrictEquals?

Done.

http://codereview.chromium.org/661369/diff/1/4#newcode8354
src/runtime.cc:8354: // in a builting function EQUAL (copmaritions implying
calling custom JS
On 2010/03/02 14:32:19, Søren Gjesse wrote:
> copmaritions -> comparisons

Done.

http://codereview.chromium.org/661369/diff/1/4#newcode8379
src/runtime.cc:8379: // (the last one could be also a string, so it must be
checked before).
On 2010/03/02 14:32:19, Søren Gjesse wrote:
> it must be checked before -> null check must be before string check.

Done.

http://codereview.chromium.org/661369/diff/1/4#newcode8396
src/runtime.cc:8396: return Smi::FromInt(4);  // Speclal case: y =
%ToPrimitive(y, NO_HINT).
On 2010/03/02 14:32:19, Søren Gjesse wrote:
> Please use constants for these special return values. You can add the same
> constants in macros.py to have them available in the JavaScript natives.

Done.

http://codereview.chromium.org/661369/diff/1/4#newcode8400
src/runtime.cc:8400: TO_NUMBER(y, 3);  // y is not null.
On 2010/03/02 14:32:19, Søren Gjesse wrote:
> Why not just use a function here, e.g.
> 
> bool supported = false;
> dy = ObjectToNumber(y, supported);
> if (!supported) return ...;
> 
> The macro which relies on existing variable names and generates a return makes
> this more complicated than necessary.

Done.

http://codereview.chromium.org/661369/diff/1/5
File src/runtime.h (right):

http://codereview.chromium.org/661369/diff/1/5#newcode129
src/runtime.h:129: F(EqualsBase, 2, 1) \
On 2010/03/02 14:32:19, Søren Gjesse wrote:
> How about calling this FastObjectEquals. Fast indicates that this is fast case
> handling and that everything isn't handled.

Good point about 'Fast', but why Objects? We use 'ObjectEquals' with different
meaning: comparing pointers (%_ObjectEquals,
CodeGenerator::GenerateObjectEquals).