[turbofan] Implemented the RoundUint64ToFloat32 TurboFan operator for x64, arm64,

Implemented the RoundUint64ToFloat32 TurboFan operator for x64, arm64,
and ppc64.

R=titzer@chromium.org

Committed: https://crrev.com/2f7d6b46d07c91c86fc48a13c5495c60dbbd9ee6
Cr-Commit-Position: refs/heads/master@{#32031}

[ahaas, 2015-11-12 11:50:25]

ahaas@chromium.org changed reviewers:
+ v8-ppc-ports@googlegroups.com

[titzer, 2015-11-12 19:06:01]

Looks mostly good other than small comments

https://codereview.chromium.org/1438013003/diff/40001/src/ppc/macro-assembler...
File src/ppc/macro-assembler-ppc.h (right):

https://codereview.chromium.org/1438013003/diff/40001/src/ppc/macro-assembler...
src/ppc/macro-assembler-ppc.h:393: void ConvertUnsignedInt64ToDouble(Register
src, DoubleRegister double_dst);
Can we follow the same naming conventions here as in the compiler? I.e. Uint64
instead of UnsignedInt64.

https://codereview.chromium.org/1438013003/diff/40001/src/ppc/simulator-ppc.cc
File src/ppc/simulator-ppc.cc (right):

https://codereview.chromium.org/1438013003/diff/40001/src/ppc/simulator-ppc.c...
src/ppc/simulator-ppc.cc:2715: double t_val = get_double_from_d_register(frb);
I'm not entirely certain that get_double_from_d_register doesn't accidentally
alter NaN bits somehow. Maybe we need a get_uint64_from_d_register?

[ahaas, 2015-11-12 20:49:18]

https://codereview.chromium.org/1438013003/diff/40001/src/ppc/macro-assembler...
File src/ppc/macro-assembler-ppc.h (right):

https://codereview.chromium.org/1438013003/diff/40001/src/ppc/macro-assembler...
src/ppc/macro-assembler-ppc.h:393: void ConvertUnsignedInt64ToDouble(Register
src, DoubleRegister double_dst);
On 2015/11/12 at 19:06:00, titzer wrote:
> Can we follow the same naming conventions here as in the compiler? I.e. Uint64
instead of UnsignedInt64.

I used the same naming conventions as in other places in the PPC specific code.
I thought it is better to keep it consistent within the file than within the CL.

https://codereview.chromium.org/1438013003/diff/40001/src/ppc/simulator-ppc.cc
File src/ppc/simulator-ppc.cc (right):

https://codereview.chromium.org/1438013003/diff/40001/src/ppc/simulator-ppc.c...
src/ppc/simulator-ppc.cc:2715: double t_val = get_double_from_d_register(frb);
On 2015/11/12 at 19:06:00, titzer wrote:
> I'm not entirely certain that get_double_from_d_register doesn't accidentally
alter NaN bits somehow. Maybe we need a get_uint64_from_d_register?

In my understanding the value in this register is the converted uint64_t value,
so this value cannot contain NaN.

https://codereview.chromium.org/1438013003/diff/40001/src/ppc/simulator-ppc.c...
src/ppc/simulator-ppc.cc:2717: double frt_val = static_cast<float>(*frb_val_p);
I think it should be float frt_val, and not double.

[MTBrandyberry, 2015-11-12 23:00:10]

mbrandy@us.ibm.com changed reviewers:
+ mbrandy@us.ibm.com

[MTBrandyberry, 2015-11-12 23:00:10]

https://codereview.chromium.org/1438013003/diff/40001/src/ppc/simulator-ppc.cc
File src/ppc/simulator-ppc.cc (right):

https://codereview.chromium.org/1438013003/diff/40001/src/ppc/simulator-ppc.c...
src/ppc/simulator-ppc.cc:2715: double t_val = get_double_from_d_register(frb);
On 2015/11/12 20:49:18, ahaas wrote:
> On 2015/11/12 at 19:06:00, titzer wrote:
> > I'm not entirely certain that get_double_from_d_register doesn't
accidentally
> alter NaN bits somehow. Maybe we need a get_uint64_from_d_register?
> 
> In my understanding the value in this register is the converted uint64_t
value,
> so this value cannot contain NaN.

get_d_register, which returns the raw int64 bits, is probably the right answer
here.  I can fix this instance and the others that need it in a follow-on CL.

https://codereview.chromium.org/1438013003/diff/40001/src/ppc/simulator-ppc.c...
src/ppc/simulator-ppc.cc:2717: double frt_val = static_cast<float>(*frb_val_p);
On 2015/11/12 20:49:18, ahaas wrote:
> I think it should be float frt_val, and not double.

PPC's only floating point register representation is double precision.  Thus the
cast is sufficient to convert to 32-bit float before assigning back to the
register.

[MTBrandyberry, 2015-11-12 23:13:12]

On 2015/11/12 23:00:10, mtbrandyberry wrote:
> https://codereview.chromium.org/1438013003/diff/40001/src/ppc/simulator-ppc.cc
> File src/ppc/simulator-ppc.cc (right):
> 
>
https://codereview.chromium.org/1438013003/diff/40001/src/ppc/simulator-ppc.c...
> src/ppc/simulator-ppc.cc:2715: double t_val = get_double_from_d_register(frb);
> On 2015/11/12 20:49:18, ahaas wrote:
> > On 2015/11/12 at 19:06:00, titzer wrote:
> > > I'm not entirely certain that get_double_from_d_register doesn't
> accidentally
> > alter NaN bits somehow. Maybe we need a get_uint64_from_d_register?
> > 
> > In my understanding the value in this register is the converted uint64_t
> value,
> > so this value cannot contain NaN.
> 
> get_d_register, which returns the raw int64 bits, is probably the right answer
> here.  I can fix this instance and the others that need it in a follow-on CL.
> 
>
https://codereview.chromium.org/1438013003/diff/40001/src/ppc/simulator-ppc.c...
> src/ppc/simulator-ppc.cc:2717: double frt_val =
static_cast<float>(*frb_val_p);
> On 2015/11/12 20:49:18, ahaas wrote:
> > I think it should be float frt_val, and not double.
> 
> PPC's only floating point register representation is double precision.  Thus
the
> cast is sufficient to convert to 32-bit float before assigning back to the
> register.

Looks mostly good for PPC -- support for the new fcfidus instruction in
disass-ppc.cc should also be added.

Thanks for making the effort to include the PPC changes!

[ahaas, 2015-11-13 09:09:31]

On 2015/11/12 at 23:13:12, mbrandy wrote:
> On 2015/11/12 23:00:10, mtbrandyberry wrote:
> >
https://codereview.chromium.org/1438013003/diff/40001/src/ppc/simulator-ppc.cc
> > File src/ppc/simulator-ppc.cc (right):
> > 
> >
https://codereview.chromium.org/1438013003/diff/40001/src/ppc/simulator-ppc.c...
> > src/ppc/simulator-ppc.cc:2715: double t_val =
get_double_from_d_register(frb);
> > On 2015/11/12 20:49:18, ahaas wrote:
> > > On 2015/11/12 at 19:06:00, titzer wrote:
> > > > I'm not entirely certain that get_double_from_d_register doesn't
> > accidentally
> > > alter NaN bits somehow. Maybe we need a get_uint64_from_d_register?
> > > 
> > > In my understanding the value in this register is the converted uint64_t
> > value,
> > > so this value cannot contain NaN.
> > 
> > get_d_register, which returns the raw int64 bits, is probably the right
answer
> > here.  I can fix this instance and the others that need it in a follow-on
CL.
> > 
> >
https://codereview.chromium.org/1438013003/diff/40001/src/ppc/simulator-ppc.c...
> > src/ppc/simulator-ppc.cc:2717: double frt_val =
static_cast<float>(*frb_val_p);
> > On 2015/11/12 20:49:18, ahaas wrote:
> > > I think it should be float frt_val, and not double.
> > 
> > PPC's only floating point register representation is double precision.  Thus
the
> > cast is sufficient to convert to 32-bit float before assigning back to the
> > register.
> 
> Looks mostly good for PPC -- support for the new fcfidus instruction in
disass-ppc.cc should also be added.
> 
> Thanks for making the effort to include the PPC changes!

Done

[MTBrandyberry, 2015-11-13 14:53:14]

On 2015/11/13 09:09:48, ahaas wrote:

LGTM for PPC

[ahaas, 2015-11-13 14:54:50]

The CQ bit was checked by ahaas@chromium.org

[commit-bot: I haz the power, 2015-11-13 14:54:56]

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

[commit-bot: I haz the power, 2015-11-13 14:57:11]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-11-13 14:57:12]

Try jobs failed on following builders:
  v8_presubmit on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/7714)

[MTBrandyberry, 2015-11-13 18:40:38]

https://codereview.chromium.org/1438013003/diff/40001/src/ppc/simulator-ppc.cc
File src/ppc/simulator-ppc.cc (right):

https://codereview.chromium.org/1438013003/diff/40001/src/ppc/simulator-ppc.c...
src/ppc/simulator-ppc.cc:2715: double t_val = get_double_from_d_register(frb);
On 2015/11/12 23:00:10, mtbrandyberry wrote:
> On 2015/11/12 20:49:18, ahaas wrote:
> > On 2015/11/12 at 19:06:00, titzer wrote:
> > > I'm not entirely certain that get_double_from_d_register doesn't
> accidentally
> > alter NaN bits somehow. Maybe we need a get_uint64_from_d_register?
> > 
> > In my understanding the value in this register is the converted uint64_t
> value,
> > so this value cannot contain NaN.
> 
> get_d_register, which returns the raw int64 bits, is probably the right answer
> here.  I can fix this instance and the others that need it in a follow-on CL.

See https://codereview.chromium.org/1445603002/ if you want to address this
here.

[ahaas, 2015-11-16 08:43:14]

On 2015/11/13 at 18:40:38, mbrandy wrote:
> https://codereview.chromium.org/1438013003/diff/40001/src/ppc/simulator-ppc.cc
> File src/ppc/simulator-ppc.cc (right):
> 
>
https://codereview.chromium.org/1438013003/diff/40001/src/ppc/simulator-ppc.c...
> src/ppc/simulator-ppc.cc:2715: double t_val = get_double_from_d_register(frb);
> On 2015/11/12 23:00:10, mtbrandyberry wrote:
> > On 2015/11/12 20:49:18, ahaas wrote:
> > > On 2015/11/12 at 19:06:00, titzer wrote:
> > > > I'm not entirely certain that get_double_from_d_register doesn't
> > accidentally
> > > alter NaN bits somehow. Maybe we need a get_uint64_from_d_register?
> > > 
> > > In my understanding the value in this register is the converted uint64_t
> > value,
> > > so this value cannot contain NaN.
> > 
> > get_d_register, which returns the raw int64 bits, is probably the right
answer
> > here.  I can fix this instance and the others that need it in a follow-on
CL.
> 
> See https://codereview.chromium.org/1445603002/ if you want to address this
here.

Done.

[MTBrandyberry, 2015-11-16 14:37:38]

lgtm

[titzer, 2015-11-17 08:47:51]

On 2015/11/16 14:37:38, mtbrandyberry wrote:
> lgtm

lgtm

[ahaas, 2015-11-17 11:42:24]

The CQ bit was checked by ahaas@chromium.org

[ahaas, 2015-11-17 11:42:24]

The patchset sent to the CQ was uploaded after l-g-t-m from mbrandy@us.ibm.com,
titzer@chromium.org
Link to the patchset: https://codereview.chromium.org/1438013003/#ps100001
(title: "Reformated the test")

[commit-bot: I haz the power, 2015-11-17 11:42:30]

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

[commit-bot: I haz the power, 2015-11-17 11:51:32]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2015-11-17 11:52:17]

Patchset 6 (id:??) landed as
https://crrev.com/2f7d6b46d07c91c86fc48a13c5495c60dbbd9ee6
Cr-Commit-Position: refs/heads/master@{#32031}