ARM improvements to constant div, mod and mul....

ARM improvements to constant div, mod and mul.
* Fast runtime calls for div and mod.
* Fix assembly and disassembly of multiply instructions.
* Strength reduce and inline multiplications to shift-add.
* Strength reduce and inline mod by power of 2.
* Strength reduce mod by other small integers to mul.
* Strength reduce div by 2 and 3.

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

[William Hesse, 2009-07-03 11:27:01]

LGTM, with comments.

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

http://codereview.chromium.org/155047/diff/1/8#newcode779
Line 779: PrintF("GenericBinaryOpStub (%s)\n", Token::String(op_));
Why not make this message more informative, by including known_rhs_?

http://codereview.chromium.org/155047/diff/1/8#newcode792
Line 792: int known_rhs) {
Would "constant_rhs" be a better name than "known_rhs"?

http://codereview.chromium.org/155047/diff/1/8#newcode925
Line 925: if (popcnt > 2) return false;
The whole function can be replaced by:
x &= x - 1;
return (x & (x-1)) != 0;

http://codereview.chromium.org/155047/diff/1/8#newcode1045
Line 1045: case Token::SHL: {
Why no check of shift_value with 0 for SHL?  If it is ok, comment it.

http://codereview.chromium.org/155047/diff/1/8#newcode5477
Line 5477: __ mov(destination, Operand(destination, LSL, first_bit));
Are you going to find out if the value overflowed from the flags?  Will they be
set right from this combination of add and move?

http://codereview.chromium.org/155047/diff/1/8#newcode5496
Line 5496: int* result_needs_shifting) {  // Including Smi tag shift
I would call this shift_amount or required_shift.  result_needs_shifting sounds
like a boolean.

[Erik Corry, 2009-07-03 17:43:38]

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

http://codereview.chromium.org/155047/diff/1/8#newcode779
Line 779: PrintF("GenericBinaryOpStub (%s)\n", Token::String(op_));
On 2009/07/03 11:27:01, William Hesse wrote:
> Why not make this message more informative, by including known_rhs_?

Oops, that is what I was trying to do, but I got the sense of the 'if' wrong.

http://codereview.chromium.org/155047/diff/1/8#newcode792
Line 792: int known_rhs) {
On 2009/07/03 11:27:01, William Hesse wrote:
> Would "constant_rhs" be a better name than "known_rhs"?

Probably

http://codereview.chromium.org/155047/diff/1/8#newcode925
Line 925: if (popcnt > 2) return false;
On 2009/07/03 11:27:01, William Hesse wrote:
> The whole function can be replaced by:
> x &= x - 1;
> return (x & (x-1)) != 0;

Cool

http://codereview.chromium.org/155047/diff/1/8#newcode1045
Line 1045: case Token::SHL: {
On 2009/07/03 11:27:01, William Hesse wrote:
> Why no check of shift_value with 0 for SHL?  If it is ok, comment it.

Shift left by zero is the way reg-reg mov is encoded.  Section A5.1.4 in the
reference manual.  But since that would make this instruction a NOP I will not
emit it.

http://codereview.chromium.org/155047/diff/1/8#newcode5477
Line 5477: __ mov(destination, Operand(destination, LSL, first_bit));
On 2009/07/03 11:27:01, William Hesse wrote:
> Are you going to find out if the value overflowed from the flags?  Will they
be
> set right from this combination of add and move?

No, the flags are not set by this routine.  Comment added.  You have to check
that the result will not overflow beforehand.