ARM: Use division instructions in lithium and stubs

ARM: Use division instructions in lithium and stubs

BUG=none
TEST=Added to test/mjsunit/math-floor-of-div.js, math-floor-of-div-nosudiv.js

[m.m.capewell, 2012-11-20 16:04:01]

Use division instructions on CPUs that support them. Although it doesn't show
performance increase in mainstream benchmarks, a microbenchmark shows a good
improvement.

-- code --
var limit = 5000;

function test_div() {
  for (var i = 1; i <= limit; i++) {
    for (var j = 1; j <= limit; j++) {
      Math.floor(i / j);
    }
  }
}

test_div();

-- results --
$ time v8/d8-nopatch --nodead-code-elimination math-floor-of-div.js
real    0m3.913s
user    0m3.920s
sys     0m0.020s

$ time v8/d8-patch --nodead-code-elimination math-floor-of-div.js
real    0m0.195s
user    0m0.190s
sys     0m0.000s

[danno, 2012-11-22 11:37:59]

lgtm if you address the comments.

https://codereview.chromium.org/11316105/diff/1/src/arm/lithium-arm.cc
File src/arm/lithium-arm.cc (right):

https://codereview.chromium.org/11316105/diff/1/src/arm/lithium-arm.cc#newcod...
src/arm/lithium-arm.cc:1236: return
constant_val->CopyToRepresentation(Representation::Integer32(),
Is it possible to not duplicate the very similar constant code to the SUDIV
case?  e.g. turning the "if (LChunkBuilder::HasMagicNumberForDivisor(int32_val))
{" into "if (LChunkBuilder::HasMagicNumberForDivisor(int32_val) ||
CpuFeatures::IsSupported(SUDIV)) {"?

https://codereview.chromium.org/11316105/diff/1/src/arm/lithium-codegen-arm.cc
File src/arm/lithium-codegen-arm.cc (right):

https://codereview.chromium.org/11316105/diff/1/src/arm/lithium-codegen-arm.c...
src/arm/lithium-codegen-arm.cc:1402: // We operated a truncating division.
Correct the result if necessary.
nit: s/operated/performed

[m.m.capewell, 2012-11-23 11:03:16]

Addressed review comments.

[danno, 2012-11-27 22:17:20]

Sorry to re-open this up, but I have some questions/comments after being able to
think about this for a while.

Also, can you also please add unit tests that ensure coverage of the odd edge
cases like x/0, kMinInt / -1 and -0 result with and without crankshaft? You
should add two copies of the test, a "normal" one and one that passes the 
enable_sudiv flag in the flags header to make sure that the SDIV path gets
executed?

https://codereview.chromium.org/11316105/diff/5001/src/arm/lithium-codegen-ar...
File src/arm/lithium-codegen-arm.cc (right):

https://codereview.chromium.org/11316105/diff/5001/src/arm/lithium-codegen-ar...
src/arm/lithium-codegen-arm.cc:1341: if (instr->right()->IsConstantOperand()) {
Can you please make sure the comment about "division by constants" remains and
is adjusted appropriately?

https://codereview.chromium.org/11316105/diff/5001/src/arm/lithium-codegen-ar...
src/arm/lithium-codegen-arm.cc:1360: __ sdiv(result, left, scratch);
from here to the sub(result, result, ....) the code is the same as in the
SDIV-specific else below. If you add a return above after the 
EmitSignedIntegerDivisionByConstant, you can share this common code

https://codereview.chromium.org/11316105/diff/5001/src/arm/lithium-codegen-ar...
src/arm/lithium-codegen-arm.cc:1364: // We performed a truncating division.
Correct the result if necessary.
Why is it safe to omit the x/0, kMinInt/-1 and -0 checks below (or simplified
forms of them)  which can still occur with a constant divisor on this path?

[m.m.capewell, 2012-12-06 14:50:45]

Addressed review comments and added tests.

[baptiste.afsa, 2012-12-06 15:28:31]

On 2012/12/06 14:50:45, m.m.capewell wrote:
> Addressed review comments and added tests.

This one misses one test file. We will re-upload the patch.

[m.m.capewell, 2012-12-06 17:57:37]

Added previously missing additional test file.

[danno, 2012-12-20 16:25:38]

Although slight, I do see the improvement in Kraken crypto. Landing with fixed
nits.

https://codereview.chromium.org/11316105/diff/15001/src/arm/code-stubs-arm.cc
File src/arm/code-stubs-arm.cc (right):

https://codereview.chromium.org/11316105/diff/15001/src/arm/code-stubs-arm.cc...
src/arm/code-stubs-arm.cc:2444: case Token::DIV: {
nit: indentation

https://codereview.chromium.org/11316105/diff/15001/src/arm/lithium-arm.cc
File src/arm/lithium-arm.cc (right):

https://codereview.chromium.org/11316105/diff/15001/src/arm/lithium-arm.cc#ne...
src/arm/lithium-arm.cc:1272: LOperand* divisor = CpuFeatures::IsSupported(SUDIV)
? UseRegister(right)
nit: indentation. For 80+col conditional operator statements, formatting should
be of form:
<condition>
  ? <value_true>
  : <value_false>;