[turbofan] Fixed a bug in TryTruncateFloatXXToInt64 with INT64_MIN.

[turbofan] Fixed a bug in TryTruncateFloatXXToInt64 with INT64_MIN.

On x64 and arm64 TryTruncateFloatXXToInt64 incorrectly failed when the
input was INT64_MIN.

R=bradnelson@chromium.org, mstarzinger@chromium.org, v8-arm-ports@googlegroups.com

Committed: https://crrev.com/2358a5be4c847b7758e0206bfe29f78174d90226
Cr-Commit-Position: refs/heads/master@{#32908}

[jbramley, 2015-12-16 12:47:01]

jacob.bramley@arm.com changed reviewers:
+ jacob.bramley@arm.com

[jbramley, 2015-12-16 12:47:01]

https://codereview.chromium.org/1526293002/diff/1/src/compiler/arm64/code-gen...
File src/compiler/arm64/code-generator-arm64.cc (right):

https://codereview.chromium.org/1526293002/diff/1/src/compiler/arm64/code-gen...
src/compiler/arm64/code-generator-arm64.cc:1048: __ Cset(i.OutputRegister(1),
vc);
This can now be a simple "__ Mov(i.OutputRegister(1), 1)". In the !vc case, it
gets overwritten anyway.

The same applies for Float64ToInt64.

https://codereview.chromium.org/1526293002/diff/1/src/compiler/arm64/code-gen...
src/compiler/arm64/code-generator-arm64.cc:1051: __
Fcmp(i.InputFloat32Register(0), -9223372036854775808.0f);
Consider using "static_cast<float>(-INT64_MIN)" here to make the intention
clear.

https://codereview.chromium.org/1526293002/diff/1/src/compiler/arm64/code-gen...
src/compiler/arm64/code-generator-arm64.cc:1065: __
Fcmp(i.InputDoubleRegister(0), -9223372036854775808.0f);
Ditto. (Otherwise, remove the "f" since this is a double.)

[Michael Starzinger, 2015-12-16 13:01:29]

LGTM (rubber-stamped "compiler", don't know the details behind the change).

https://codereview.chromium.org/1526293002/diff/1/src/compiler/arm64/code-gen...
File src/compiler/arm64/code-generator-arm64.cc (right):

https://codereview.chromium.org/1526293002/diff/1/src/compiler/arm64/code-gen...
src/compiler/arm64/code-generator-arm64.cc:1051: __
Fcmp(i.InputFloat32Register(0), -9223372036854775808.0f);
On 2015/12/16 12:47:01, jbramley wrote:
> Consider using "static_cast<float>(-INT64_MIN)" here to make the intention
> clear.

+1, magic number is being magic, please use a named value.

https://codereview.chromium.org/1526293002/diff/1/src/compiler/arm64/code-gen...
src/compiler/arm64/code-generator-arm64.cc:1065: __
Fcmp(i.InputDoubleRegister(0), -9223372036854775808.0f);
On 2015/12/16 12:47:01, jbramley wrote:
> Ditto. (Otherwise, remove the "f" since this is a double.)

+1

https://codereview.chromium.org/1526293002/diff/1/src/compiler/x64/code-gener...
File src/compiler/x64/code-generator-x64.cc (right):

https://codereview.chromium.org/1526293002/diff/1/src/compiler/x64/code-gener...
src/compiler/x64/code-generator-x64.cc:1059: __ Move(kScratchDoubleReg,
-9223372036854775808.0f);
Likewise for magic number.

https://codereview.chromium.org/1526293002/diff/1/src/compiler/x64/code-gener...
src/compiler/x64/code-generator-x64.cc:1088: __ Move(kScratchDoubleReg,
-9223372036854775808.0);
Likewise for magic number.

https://codereview.chromium.org/1526293002/diff/1/test/cctest/compiler/value-...
File test/cctest/compiler/value-helper.h (right):

https://codereview.chromium.org/1526293002/diff/1/test/cctest/compiler/value-...
test/cctest/compiler/value-helper.h:76: -9223372036854775808.0f,
nit: Placement looks random, can we move it up or down? Also use a name if this
is derived from INT64_MIN/MAX here as well.

[ahaas, 2015-12-16 14:35:24]

https://codereview.chromium.org/1526293002/diff/1/src/compiler/arm64/code-gen...
File src/compiler/arm64/code-generator-arm64.cc (right):

https://codereview.chromium.org/1526293002/diff/1/src/compiler/arm64/code-gen...
src/compiler/arm64/code-generator-arm64.cc:1048: __ Cset(i.OutputRegister(1),
vc);
On 2015/12/16 at 12:47:01, jbramley wrote:
> This can now be a simple "__ Mov(i.OutputRegister(1), 1)". In the !vc case, it
gets overwritten anyway.
> 
> The same applies for Float64ToInt64.

Done.

https://codereview.chromium.org/1526293002/diff/1/src/compiler/arm64/code-gen...
src/compiler/arm64/code-generator-arm64.cc:1051: __
Fcmp(i.InputFloat32Register(0), -9223372036854775808.0f);
On 2015/12/16 at 13:01:29, Michael Starzinger wrote:
> On 2015/12/16 12:47:01, jbramley wrote:
> > Consider using "static_cast<float>(-INT64_MIN)" here to make the intention
> > clear.
> 
> +1, magic number is being magic, please use a named value.

Done.

https://codereview.chromium.org/1526293002/diff/1/src/compiler/arm64/code-gen...
src/compiler/arm64/code-generator-arm64.cc:1065: __
Fcmp(i.InputDoubleRegister(0), -9223372036854775808.0f);
On 2015/12/16 at 13:01:29, Michael Starzinger wrote:
> On 2015/12/16 12:47:01, jbramley wrote:
> > Ditto. (Otherwise, remove the "f" since this is a double.)
> 
> +1

Done.

https://codereview.chromium.org/1526293002/diff/1/src/compiler/x64/code-gener...
File src/compiler/x64/code-generator-x64.cc (right):

https://codereview.chromium.org/1526293002/diff/1/src/compiler/x64/code-gener...
src/compiler/x64/code-generator-x64.cc:1059: __ Move(kScratchDoubleReg,
-9223372036854775808.0f);
On 2015/12/16 at 13:01:29, Michael Starzinger wrote:
> Likewise for magic number.

Done.

https://codereview.chromium.org/1526293002/diff/1/src/compiler/x64/code-gener...
src/compiler/x64/code-generator-x64.cc:1088: __ Move(kScratchDoubleReg,
-9223372036854775808.0);
On 2015/12/16 at 13:01:29, Michael Starzinger wrote:
> Likewise for magic number.

Done.

[jbramley, 2015-12-16 14:41:01]

LGTM (arm64).

[ahaas, 2015-12-16 15:15:47]

The CQ bit was checked by ahaas@chromium.org

[ahaas, 2015-12-16 15:15:47]

The patchset sent to the CQ was uploaded after l-g-t-m from
mstarzinger@chromium.org, jacob.bramley@arm.com
Link to the patchset: https://codereview.chromium.org/1526293002/#ps60001
(title: "Use constants with comments instead of type casts")

[commit-bot: I haz the power, 2015-12-16 15:15:55]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1526293002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1526293002/60001

[commit-bot: I haz the power, 2015-12-16 15:34:31]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-12-16 15:34:41]

Description was changed from

==========
[turbofan] Fixed a bug in TryTruncateFloatXXToInt64 with INT64_MIN.

On x64 and arm64 TryTruncateFloatXXToInt64 incorrectly failed when the
input was INT64_MIN.

R=bradnelson@chromium.org, mstarzinger@chromium.org,
v8-arm-ports@googlegroups.com
==========

to

==========
[turbofan] Fixed a bug in TryTruncateFloatXXToInt64 with INT64_MIN.

On x64 and arm64 TryTruncateFloatXXToInt64 incorrectly failed when the
input was INT64_MIN.

R=bradnelson@chromium.org, mstarzinger@chromium.org,
v8-arm-ports@googlegroups.com

Committed: https://crrev.com/2358a5be4c847b7758e0206bfe29f78174d90226
Cr-Commit-Position: refs/heads/master@{#32908}
==========

[commit-bot: I haz the power, 2015-12-16 15:34:42]

Patchset 4 (id:??) landed as
https://crrev.com/2358a5be4c847b7758e0206bfe29f78174d90226
Cr-Commit-Position: refs/heads/master@{#32908}