ARM: Tweak ECMAToInt32VFP to address regression on Nexus 4

ARM: Tweak ECMAToInt32VFP to address regression on Nexus 4

After r13912, we saw a small regression in Kraken crypto-aes and
crypto-ccm on Nexus 4. This patch, proposed by Rodolph Perfetta,
addresses that without regressing other platforms.

Instead of looking at the exponent of double_input and trying to figure
out if the conversion will overflow, eagerly do the VCVT and return
early unless it saturated.

BUG=none

Committed: https://code.google.com/p/v8/source/detail?r=13948

[hans, 2013-03-14 10:35:40]

Please take a look.

[Rodolph Perfetta, 2013-03-14 11:57:58]

https://codereview.chromium.org/12544025/diff/1/src/arm/macro-assembler-arm.cc
File src/arm/macro-assembler-arm.cc (right):

https://codereview.chromium.org/12544025/diff/1/src/arm/macro-assembler-arm.c...
src/arm/macro-assembler-arm.cc:2538: sub(scratch, result, Operand(1));
Maybe a comment saying we are testing for saturation (0x7fffffff and 0x80000000)
would be useful.

https://codereview.chromium.org/12544025/diff/1/src/arm/macro-assembler-arm.c...
src/arm/macro-assembler-arm.cc:2544: sub(scratch, scratch,
Operand(HeapNumber::kExponentBias + 1));
I would leave the comment above.

https://codereview.chromium.org/12544025/diff/1/src/arm/macro-assembler-arm.c...
src/arm/macro-assembler-arm.cc:2545: cmp(scratch, Operand(83));
ditto.

[hans, 2013-03-14 12:12:05]

Thanks for the review! New version uploaded.

https://codereview.chromium.org/12544025/diff/1/src/arm/macro-assembler-arm.cc
File src/arm/macro-assembler-arm.cc (right):

https://codereview.chromium.org/12544025/diff/1/src/arm/macro-assembler-arm.c...
src/arm/macro-assembler-arm.cc:2538: sub(scratch, result, Operand(1));
On 2013/03/14 11:57:58, Rodolph Perfetta wrote:
> Maybe a comment saying we are testing for saturation (0x7fffffff and
0x80000000)
> would be useful.

Done.

https://codereview.chromium.org/12544025/diff/1/src/arm/macro-assembler-arm.c...
src/arm/macro-assembler-arm.cc:2544: sub(scratch, scratch,
Operand(HeapNumber::kExponentBias + 1));
On 2013/03/14 11:57:58, Rodolph Perfetta wrote:
> I would leave the comment above.

Done.

https://codereview.chromium.org/12544025/diff/1/src/arm/macro-assembler-arm.c...
src/arm/macro-assembler-arm.cc:2545: cmp(scratch, Operand(83));
On 2013/03/14 11:57:58, Rodolph Perfetta wrote:
> ditto.

Done.

[Rodolph Perfetta, 2013-03-14 13:39:58]

LGTM with nits addressed.

https://codereview.chromium.org/12544025/diff/7001/src/arm/macro-assembler-ar...
File src/arm/macro-assembler-arm.cc (right):

https://codereview.chromium.org/12544025/diff/7001/src/arm/macro-assembler-ar...
src/arm/macro-assembler-arm.cc:2539: // If result is not saturated (0x7fffffff
or (0x80000000), we are done.
nit: ( before 0x80000000

https://codereview.chromium.org/12544025/diff/7001/src/arm/macro-assembler-ar...
src/arm/macro-assembler-arm.cc:2553: // This test also catch NaN and infinities
which also return 0.
This comment is not accurate anymore, vcvt will map NaN to 0 (which is what ECMA
specs require) so NaN will be handled as part of the non saturating early exit
above.

[hans, 2013-03-14 14:28:40]

Thanks! Uploaded new patch with nits addressed.

Rodolph or Ulan, would you like to land for me?

https://codereview.chromium.org/12544025/diff/7001/src/arm/macro-assembler-ar...
File src/arm/macro-assembler-arm.cc (right):

https://codereview.chromium.org/12544025/diff/7001/src/arm/macro-assembler-ar...
src/arm/macro-assembler-arm.cc:2539: // If result is not saturated (0x7fffffff
or (0x80000000), we are done.
On 2013/03/14 13:39:58, Rodolph Perfetta wrote:
> nit: ( before 0x80000000

Done.

https://codereview.chromium.org/12544025/diff/7001/src/arm/macro-assembler-ar...
src/arm/macro-assembler-arm.cc:2553: // This test also catch NaN and infinities
which also return 0.
On 2013/03/14 13:39:58, Rodolph Perfetta wrote:
> This comment is not accurate anymore, vcvt will map NaN to 0 (which is what
ECMA
> specs require) so NaN will be handled as part of the non saturating early exit
> above.

Removed.

[ulan, 2013-03-14 14:35:27]

LGTM, I will land it for you.

[ulan, 2013-03-14 15:28:29]

Committed patchset #4 manually as r13948 (presubmit successful).