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.
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.
Capitalize comments.
http://codereview.chromium.org/3247008/diff/1/3#newcode1149
src/arm/codegen-arm.cc:1149: // Check that the *signed* result fits in a
smi.
You don't need this for AND or for the SAR if the shift is more than 0.
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.
We can get here by two routes. Either a Smi >>> 0 or a non-Smi >>> 0 or
1. The comment looks wrong and the code may be too?
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
Smi tagging? I think you mean Smi overflow.
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);
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.
http://codereview.chromium.org/3247008/show
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
