Handle bitwise operations with literal Smi for 32bits integers without...

Handle bitwise operations with literal Smi for 32bits integers without
calling the GenericBinaryOpStub. Refactored and updated the routine to
convert a signed int to a double. A following patch for the generic
case (none of the operand is a literal) is also planned.

BUG=none
TEST=none

[Erik Corry, 2010-08-31 20:28:57]

Perhaps the comments can be clarified around the SHR non-Smi result.  It seems a
little hard to understand right now and it's not obvious to me that it's
correct.

Otherwise LGTM.

[Erik Corry, 2010-08-31 20:29:02]

Perhaps the comments can be clarified around the SHR non-Smi result.  It seems a
little hard to understand right now and it's not obvious to me that it's
correct.

Otherwise LGTM.

[Rodolph Perfetta, 2010-09-01 14:34:10]

comments applied

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

http://codereview.chromium.org/3247008/diff/1/3#newcode975
src/arm/codegen-arm.cc:975: SaveRegisters();  // currently does nothing.
On 2010/08/31 20:28:47, Erik Corry wrote:
> Capitalize comments.

Done.

http://codereview.chromium.org/3247008/diff/1/3#newcode1149
src/arm/codegen-arm.cc:1149: // Check that the *signed* result fits in a smi.
On 2010/08/31 20:28:47, Erik Corry wrote:
> You don't need this for AND or for the SAR if the shift is more than 0.

Done.

http://codereview.chromium.org/3247008/diff/1/3#newcode1185
src/arm/codegen-arm.cc:1185: // >>> 0 is the only case where the Smi is tagged.
Comments updated. We only get here if the input were Smis, but the result of the
operation cannot fit in a Smi. >>> 0 is a special case where the result is
already tagged but wrong because the Smi is negative. We untag it before
converting it.

http://codereview.chromium.org/3247008/diff/1/3#newcode1487
src/arm/codegen-arm.cc:1487: // Smi tagging these two cases can only happen with
shifts
Punctuation error, "tagging" is connected to the sentence above. Comments
updated.

http://codereview.chromium.org/3247008/diff/1/4
File src/arm/macro-assembler-arm.cc (right):

http://codereview.chromium.org/3247008/diff/1/4#newcode1355
src/arm/macro-assembler-arm.cc:1355: cmp(dest, Operand(LONG_MIN), ne);
On 2010/08/31 20:28:47, Erik Corry wrote:
> I wonder if it would be better to add a fudge factor and then do an unsigned
> comparison with 2?  Conditional cmp seems like a tough instruction for the
CPU.

Done.

[Erik Corry, 2010-09-02 08:26:48]

LGTM.

http://codereview.chromium.org/3247008/diff/9001/10002
File src/arm/codegen-arm.cc (right):

http://codereview.chromium.org/3247008/diff/9001/10002#newcode975
src/arm/codegen-arm.cc:975: SaveRegisters();  // Currently does nothing.
I'm going to remove this, because it does nothing and it never will do anything.
 The register allocation logic is handled by the spillall below.

http://codereview.chromium.org/3247008/diff/9001/10002#newcode1050
src/arm/codegen-arm.cc:1050: RestoreRegisters();  // Currently does nothing.
The registers were restored by the MergeTo above.

http://codereview.chromium.org/3247008/diff/9001/10002#newcode1165
src/arm/codegen-arm.cc:1165: // If the allocation fails, fall back to the
GenericBinaryOpStub.
I've added a comment here that we should really spill the registers before using
r4, r5, r6.   At the moment it doesn't matter since we don't use them.

[Erik Corry, 2010-09-02 08:31:22]

Committed as r5401.  Thanks.