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.

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

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to