[v8-dev] Re: Decide that different detectable objects are not equal without calling runtim... (issue2805046)

[whesse]

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.

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

--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev