-Inlined double variant of compare iff one of the sides is a constant smi and...

-Inlined double variant of compare iff one of the sides is a constant smi and it is not a for loop condition.


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

[iposva, 2009-12-17 21:03:49]

LGTM, but it would be good to get Kevin's input about all the jump target
interaction.

-Ivan

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

http://codereview.chromium.org/507040/diff/1/2#newcode1869
src/ia32/codegen-ia32.cc:1869: bool is_for_loop_compare =
node->AsCompareOperation()
Shouldn't you do a " != NULL" here?

http://codereview.chromium.org/507040/diff/1/2#newcode1874
src/ia32/codegen-ia32.cc:1874: // The right side value is either a smi or heap
number.
Comment does not match the check. Right side is a smi.

http://codereview.chromium.org/507040/diff/1/2#newcode1888
src/ia32/codegen-ia32.cc:1888: __ cvtsi2sd(xmm0, Operand(temp.reg()));
Why do you keep converting the value to double from int on every check? You
could just create the double immediate and load it directly.

http://codereview.chromium.org/507040/diff/1/2#newcode6849
src/ia32/codegen-ia32.cc:6849: // The calling convention with registers is left
in edx and right in eax.
Unmotivated space?

http://codereview.chromium.org/507040/diff/1/6
File src/ia32/disasm-ia32.cc (right):

http://codereview.chromium.org/507040/diff/1/6#newcode1052
src/ia32/disasm-ia32.cc:1052: } if(*data == 0x57) {
missing else?

[bakster, 2009-12-18 06:38:47]

//Lars

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

http://codereview.chromium.org/507040/diff/1/2#newcode1869
src/ia32/codegen-ia32.cc:1869: bool is_for_loop_compare =
node->AsCompareOperation()
On 2009/12/17 21:03:50, iposva wrote:
> Shouldn't you do a " != NULL" here?

Done.

http://codereview.chromium.org/507040/diff/1/2#newcode1874
src/ia32/codegen-ia32.cc:1874: // The right side value is either a smi or heap
number.
On 2009/12/17 21:03:50, iposva wrote:
> Comment does not match the check. Right side is a smi.

Done.

http://codereview.chromium.org/507040/diff/1/2#newcode1888
src/ia32/codegen-ia32.cc:1888: __ cvtsi2sd(xmm0, Operand(temp.reg()));
I'm not sure it makes a difference. I'll try the optimization after the commit.

On 2009/12/17 21:03:50, iposva wrote:
> Why do you keep converting the value to double from int on every check? You
> could just create the double immediate and load it directly.

http://codereview.chromium.org/507040/diff/1/2#newcode6849
src/ia32/codegen-ia32.cc:6849: // The calling convention with registers is left
in edx and right in eax.
On 2009/12/17 21:03:50, iposva wrote:
> Unmotivated space?

Done.

http://codereview.chromium.org/507040/diff/1/6
File src/ia32/disasm-ia32.cc (right):

http://codereview.chromium.org/507040/diff/1/6#newcode1052
src/ia32/disasm-ia32.cc:1052: } if(*data == 0x57) {
On 2009/12/17 21:03:50, iposva wrote:
> missing else?

Done.