Increase sanity of integer division handling on ARM

Increase sanity of integer division handling on ARM

- In the INT32 BinaryOpStub, fix type feedback collection for DIV,
  bringing it in line with other platforms.
- In Lithium codegen, emit proper inlined code, don't call the stub.
- Drive-by fix: assert appropriate CpuFeaturesScope for SDIV.

R=ulan@chromium.org

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

[Jakob Kummerow, 2013-06-10 10:23:41]

Please take a look.

[ulan, 2013-06-10 11:18:32]

LGTM

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

https://codereview.chromium.org/16082008/diff/1/src/arm/lithium-codegen-arm.c...
src/arm/lithium-codegen-arm.cc:1492: // Use vright as scratch register.
I think we can use TryDoubleToInt32Exact() here.

[Jakob Kummerow, 2013-06-10 12:20:27]

Thanks for the review, Ulan.

Rodolph, any comments?

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

https://codereview.chromium.org/16082008/diff/1/src/arm/lithium-codegen-arm.c...
src/arm/lithium-codegen-arm.cc:1492: // Use vright as scratch register.
On 2013/06/10 11:18:33, ulan wrote:
> I think we can use TryDoubleToInt32Exact() here.

Good point, right now we could do that.
My next patch, however, will split the sequence and put half of it in a
conditional block, so I'll leave it as it is for now.

[Rodolph Perfetta, 2013-06-10 18:04:07]

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

https://codereview.chromium.org/16082008/diff/1/src/arm/code-stubs-arm.cc#new...
src/arm/code-stubs-arm.cc:2219: __ add(scratch2, scratch1, Operand(0x40000000),
SetCC);
a possible improvement:

  __ SmiTag(scratch1, SetCC);  // adds s1, s1, s1
  __ b(vs, &return_heap_number);
  ASSERT(kSmiTag == 0);
  __ b(ne, &not_zero);
  ...
  __ bind(&not_zero);
  __ mov(r0, Operand(scratch1));
  ...

https://codereview.chromium.org/16082008/diff/1/src/arm/code-stubs-arm.cc#new...
src/arm/code-stubs-arm.cc:2229: __ b(ne, result_type_ <= BinaryOpIC::INT32 ?
&transition
We now know result_type <= INT32

[Jakob Kummerow, 2013-06-11 09:33:13]

Thanks, Rodolph.

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

https://codereview.chromium.org/16082008/diff/1/src/arm/code-stubs-arm.cc#new...
src/arm/code-stubs-arm.cc:2219: __ add(scratch2, scratch1, Operand(0x40000000),
SetCC);
On 2013/06/10 18:04:07, Rodolph Perfetta wrote:
> a possible improvement:
> 
>   __ SmiTag(scratch1, SetCC);  // adds s1, s1, s1
>   __ b(vs, &return_heap_number);
>   ASSERT(kSmiTag == 0);
>   __ b(ne, &not_zero);
>   ...
>   __ bind(&not_zero);
>   __ mov(r0, Operand(scratch1));
>   ...

Good idea. Done.

https://codereview.chromium.org/16082008/diff/1/src/arm/code-stubs-arm.cc#new...
src/arm/code-stubs-arm.cc:2229: __ b(ne, result_type_ <= BinaryOpIC::INT32 ?
&transition
On 2013/06/10 18:04:07, Rodolph Perfetta wrote:
> We now know result_type <= INT32

Done.

[Jakob Kummerow, 2013-06-11 10:47:54]

Committed patchset #2 manually as r15057 (presubmit successful).