IA32: Avoid going into stubs or runtime code for bitops even if the...

IA32: Avoid going into stubs or runtime code for bitops even if the
inputs are heap numbers or the result is a heap number (only with
SSE2).  Make it possible for a deferred code object to work without
spilling all registers.

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

[Lasse Reichstein, 2010-08-06 08:20:12]

LGTM.

http://codereview.chromium.org/3054047/diff/1/3
File src/codegen.h (right):

http://codereview.chromium.org/3054047/diff/1/3#newcode324
src/codegen.h:324: virtual bool AutoSaveAndRestore() { return true; }
I know it's not widespread in V8, but please add comments saying what the
functions do.

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

http://codereview.chromium.org/3054047/diff/1/6#newcode1255
src/ia32/codegen-ia32.cc:1255: __ JumpIfNotInt32(left_, dst_, left_info_,
entry_label());
The JumpIfNotInt32 name seems incorrect if it accepts fractions (and if it
doesn't, the next cvt doesn't need to be truncating). 
Does it check that the value is <2^31 and >=-2^31?

http://codereview.chromium.org/3054047/diff/1/6#newcode1260
src/ia32/codegen-ia32.cc:1260: CpuFeatures::Scope use_sse2(SSE2);
What if SSE2 is not supported?
(It was tested in the case above, so if it can't happen here, make a comment
saying why).

http://codereview.chromium.org/3054047/diff/1/6#newcode1340
src/ia32/codegen-ia32.cc:1340: __ push(left_);  // 0 or 0x80000000.
Make comment and/or ASSERT saying that left_ must be a smi.

http://codereview.chromium.org/3054047/diff/1/8
File src/ia32/macro-assembler-ia32.cc (right):

http://codereview.chromium.org/3054047/diff/1/8#newcode1540
src/ia32/macro-assembler-ia32.cc:1540: j(greater, on_not_int32);
Is this faster than using SSE2 cvttsd2si and checking against 0x8000000? Even if
it is, using cvttsd2si would also give the 32-bit number as a result
immediately, instead of first doing this test and then converting afterwards, as
it seemed was the typical use of this function.
E.g., I'm suggesting a: 
 NumberToInt32(Register dst, Register src, TypeInfo info, Label* on_not_int32)

http://codereview.chromium.org/3054047/diff/1/9
File src/ia32/macro-assembler-ia32.h (right):

http://codereview.chromium.org/3054047/diff/1/9#newcode229
src/ia32/macro-assembler-ia32.h:229: void SmiUntagAndBranchOnNonSmi(Register
reg, TypeInfo info, Label* non_smi) {
In X64 I've been naming functions like this just SmiUntag (or SmiToInteger32),
letting the failure label parameter explain failure behavior.

http://codereview.chromium.org/3054047/diff/1/9#newcode229
src/ia32/macro-assembler-ia32.h:229: void SmiUntagAndBranchOnNonSmi(Register
reg, TypeInfo info, Label* non_smi) {
Do comment that this function is destructive.

http://codereview.chromium.org/3054047/diff/1/9#newcode249
src/ia32/macro-assembler-ia32.h:249: // on the min negative int32.  Ignores
frational parts.
As stated otherwhere, the name doesn't match the functionality since it ignores
fractions.
Not that I have a better suggestion.

[Erik Corry, 2010-08-09 13:13:49]

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

http://codereview.chromium.org/3054047/diff/1/6#newcode1255
src/ia32/codegen-ia32.cc:1255: __ JumpIfNotInt32(left_, dst_, left_info_,
entry_label());
On 2010/08/06 08:20:12, Lasse Reichstein wrote:
> The JumpIfNotInt32 name seems incorrect if it accepts fractions (and if it
> doesn't, the next cvt doesn't need to be truncating). 
> Does it check that the value is <2^31 and >=-2^31?

Renamed to JumpIfNotInt32Range

http://codereview.chromium.org/3054047/diff/1/6#newcode1260
src/ia32/codegen-ia32.cc:1260: CpuFeatures::Scope use_sse2(SSE2);
On 2010/08/06 08:20:12, Lasse Reichstein wrote:
> What if SSE2 is not supported?
> (It was tested in the case above, so if it can't happen here, make a comment
> saying why).

Comment added.

http://codereview.chromium.org/3054047/diff/1/6#newcode1340
src/ia32/codegen-ia32.cc:1340: __ push(left_);  // 0 or 0x80000000.
On 2010/08/06 08:20:12, Lasse Reichstein wrote:
> Make comment and/or ASSERT saying that left_ must be a smi.

Done.

http://codereview.chromium.org/3054047/diff/1/8
File src/ia32/macro-assembler-ia32.cc (right):

http://codereview.chromium.org/3054047/diff/1/8#newcode1540
src/ia32/macro-assembler-ia32.cc:1540: j(greater, on_not_int32);
On 2010/08/06 08:20:12, Lasse Reichstein wrote:
> Is this faster than using SSE2 cvttsd2si and checking against 0x8000000? Even
if
> it is, using cvttsd2si would also give the 32-bit number as a result
> immediately, instead of first doing this test and then converting afterwards,
as
> it seemed was the typical use of this function.
> E.g., I'm suggesting a: 
>  NumberToInt32(Register dst, Register src, TypeInfo info, Label* on_not_int32)
> 

Done.

http://codereview.chromium.org/3054047/diff/1/9
File src/ia32/macro-assembler-ia32.h (right):

http://codereview.chromium.org/3054047/diff/1/9#newcode229
src/ia32/macro-assembler-ia32.h:229: void SmiUntagAndBranchOnNonSmi(Register
reg, TypeInfo info, Label* non_smi) {
On 2010/08/06 08:20:12, Lasse Reichstein wrote:
> In X64 I've been naming functions like this just SmiUntag (or SmiToInteger32),
> letting the failure label parameter explain failure behavior.

Done.

http://codereview.chromium.org/3054047/diff/1/9#newcode229
src/ia32/macro-assembler-ia32.h:229: void SmiUntagAndBranchOnNonSmi(Register
reg, TypeInfo info, Label* non_smi) {
On 2010/08/06 08:20:12, Lasse Reichstein wrote:
> In X64 I've been naming functions like this just SmiUntag (or SmiToInteger32),
> letting the failure label parameter explain failure behavior.

Done.

http://codereview.chromium.org/3054047/diff/1/9#newcode249
src/ia32/macro-assembler-ia32.h:249: // on the min negative int32.  Ignores
frational parts.
On 2010/08/06 08:20:12, Lasse Reichstein wrote:
> As stated otherwhere, the name doesn't match the functionality since it
ignores
> fractions.
> Not that I have a better suggestion.

Renamed.