Decide that different detectable objects are not equal without calling runtim...

Decide that different detectable objects are not equal without calling runtime, on all platforms.

Committed: http://code.google.com/p/v8/source/detail?r=4992

[William Hesse, 2010-06-29 13:56:42]

Erik, can you check the ARM version.  It is quite different than the other two
platforms.

http://codereview.chromium.org/2805046/diff/1/3
File src/ia32/codegen-ia32.cc (right):

http://codereview.chromium.org/2805046/diff/1/3#newcode11737
src/ia32/codegen-ia32.cc:11737: // Strict equality can quickly decide whether
objects are equal.
The next 60 lines are just reformatted, not changed.  Just the if() and the
braces are changed.

[Lasse Reichstein, 2010-06-30 08:12:02]

LGTM

http://codereview.chromium.org/2805046/diff/1/3
File src/ia32/codegen-ia32.cc (right):

http://codereview.chromium.org/2805046/diff/1/3#newcode11905
src/ia32/codegen-ia32.cc:11905: __ bind(&call_builtin);
Seems odd to put more code at the call_builtin label.
The label should probably be renamed to something else.

http://codereview.chromium.org/2805046/diff/1/3#newcode11911
src/ia32/codegen-ia32.cc:11911: // At most one is a smi, so we can test for smi
by adding the two.
More explanation of why this works. Perhaps just "If one is a smi, the low bit
is 1, if both are non-smi, it is zero."

http://codereview.chromium.org/2805046/diff/1/3#newcode11916
src/ia32/codegen-ia32.cc:11916: __ j(not_zero, &not_shown_unequal);
Maybe change label to "not_both_detectable_js_objects".

http://codereview.chromium.org/2805046/diff/1/3#newcode11926
src/ia32/codegen-ia32.cc:11926: __ j(not_zero, &not_shown_unequal);
Could you load both bitfields into ecx and ebx and or them and then only do one
test?

http://codereview.chromium.org/2805046/diff/1/4
File src/x64/codegen-x64.cc (right):

http://codereview.chromium.org/2805046/diff/1/4#newcode9132
src/x64/codegen-x64.cc:9132: Condition a_smi = masm->CheckEitherSmi(rax, rdx);
The CheckEitherSmi macro seems less efficient than what you have in the 32-bit
version.
Maybe add a macro for CheckEitherSmiGivenNotBothSmi (with, hopefully, a better
name).

http://codereview.chromium.org/2805046/diff/1/4#newcode9146
src/x64/codegen-x64.cc:9146: __ bind(&not_shown_unequal);
Same comments as for ia32 applies here.

[William Hesse, 2010-06-30 12:04:54]

http://codereview.chromium.org/2805046/diff/1/3
File src/ia32/codegen-ia32.cc (right):

http://codereview.chromium.org/2805046/diff/1/3#newcode11905
src/ia32/codegen-ia32.cc:11905: __ bind(&call_builtin);
On 2010/06/30 08:12:02, Lasse Reichstein wrote:
> Seems odd to put more code at the call_builtin label.
> The label should probably be renamed to something else.

Done.

http://codereview.chromium.org/2805046/diff/1/3#newcode11911
src/ia32/codegen-ia32.cc:11911: // At most one is a smi, so we can test for smi
by adding the two.
On 2010/06/30 08:12:02, Lasse Reichstein wrote:
> More explanation of why this works. Perhaps just "If one is a smi, the low bit
> is 1, if both are non-smi, it is zero."

Done.

http://codereview.chromium.org/2805046/diff/1/3#newcode11916
src/ia32/codegen-ia32.cc:11916: __ j(not_zero, &not_shown_unequal);
On 2010/06/30 08:12:02, Lasse Reichstein wrote:
> Maybe change label to "not_both_detectable_js_objects".

How about "not_both_objects".

http://codereview.chromium.org/2805046/diff/1/3#newcode11926
src/ia32/codegen-ia32.cc:11926: __ j(not_zero, &not_shown_unequal);
On 2010/06/30 08:12:02, Lasse Reichstein wrote:
> Could you load both bitfields into ecx and ebx and or them and then only do
one
> test?

The effective way to do this uses the and_b instruction, which is not yet
implemented.  Maybe in a followup change.
> 

Even if one or both objects is undetectable, we know the result of the
comparison, and return it here.

http://codereview.chromium.org/2805046/diff/1/4
File src/x64/codegen-x64.cc (right):

http://codereview.chromium.org/2805046/diff/1/4#newcode9132
src/x64/codegen-x64.cc:9132: Condition a_smi = masm->CheckEitherSmi(rax, rdx);
On 2010/06/30 08:12:02, Lasse Reichstein wrote:
> The CheckEitherSmi macro seems less efficient than what you have in the 32-bit
> version.
> Maybe add a macro for CheckEitherSmiGivenNotBothSmi (with, hopefully, a better
> name).

Changed to agree with ia32

http://codereview.chromium.org/2805046/diff/1/4#newcode9146
src/x64/codegen-x64.cc:9146: __ bind(&not_shown_unequal);
On 2010/06/30 08:12:02, Lasse Reichstein wrote:
> Same comments as for ia32 applies here.

Changed as in ia32.

[Erik Corry, 2010-06-30 12:34:39]

ARM part LGTM