Add floor, ceil, round (truncate) instructions for ia32, x64 (if SSE4.1) and

Add floor, ceil, round (truncate) instructions for ia32, x64 (if SSE4.1) and
add floor, ceil, round (truncate and away from zero) for arm64.

R=bmeurer@chromium.org, dcarney@chromium.org, mstarzinger@chromium.org, rodolph.perfetta@arm.com
TEST=test/mjsunit/asm/math-floor.js,test/mjsunit/asm/math-ceil.js,test/unittest/compiler/js-builtin-reducer-unittest.cc

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

[sigurds, 2014-10-23 14:13:39]

Patchset #2 (id:20001) has been deleted

[sigurds, 2014-10-23 14:13:43]

Patchset #2 (id:20001) has been deleted

[sigurds, 2014-10-23 14:20:04]

Patchset #2 (id:40001) has been deleted

[sigurds, 2014-10-23 14:32:34]

sigurds@chromium.org changed reviewers:
+ dcarney@chromium.org, mstarzinger@chromium.org, rodolph.perfetta@arm.com

[sigurds, 2014-10-23 14:32:34]

bmeurer, mstarzinger: please take a look.

dcarney: Please look at code-generator-ia32 and x64.

rodolph.perfetta: Please take a look at code-generator-arm64.

I'll kick off trybots as soon as the dependency cl is landed.

[Rodolph Perfetta (ARM), 2014-10-24 14:33:03]

code-generator-arm64 is fine.

[sigurds, 2014-10-24 14:45:52]

On 2014/10/24 14:33:03, Rodolph Perfetta (ARM) wrote:
> code-generator-arm64 is fine.

Thanks, have a good weekend!

[Benedikt Meurer, 2014-10-26 12:48:05]

https://codereview.chromium.org/677433002/diff/110025/src/compiler/arm/instru...
File src/compiler/arm/instruction-selector-arm.cc (right):

https://codereview.chromium.org/677433002/diff/110025/src/compiler/arm/instru...
src/compiler/arm/instruction-selector-arm.cc:830: UnsupportedOperator(node);
Add TODO for ARMv8.

https://codereview.chromium.org/677433002/diff/110025/src/compiler/arm/instru...
src/compiler/arm/instruction-selector-arm.cc:835: UnsupportedOperator(node);
Add TODO for ARMv8.

https://codereview.chromium.org/677433002/diff/110025/src/compiler/arm/instru...
src/compiler/arm/instruction-selector-arm.cc:840: UnsupportedOperator(node);
Add TODO for ARMv8.

https://codereview.chromium.org/677433002/diff/110025/src/compiler/arm/instru...
src/compiler/arm/instruction-selector-arm.cc:845: UnsupportedOperator(node);
Add TODO for ARMv8.

https://codereview.chromium.org/677433002/diff/110025/src/compiler/arm64/inst...
File src/compiler/arm64/instruction-selector-arm64.cc (right):

https://codereview.chromium.org/677433002/diff/110025/src/compiler/arm64/inst...
src/compiler/arm64/instruction-selector-arm64.cc:895: Arm64OperandGenerator
g(this);
How about adding a common VisitRRFloat64 helper?

https://codereview.chromium.org/677433002/diff/110025/src/compiler/instructio...
File src/compiler/instruction-selector.cc (right):

https://codereview.chromium.org/677433002/diff/110025/src/compiler/instructio...
src/compiler/instruction-selector.cc:771: 
Nit: missing newline.

[Benedikt Meurer, 2014-10-26 13:25:06]

Also, how about removing either floor or ceil operator, and express i.e. floor
via -ceil(-x)? We can still generate the best code by matching that sequence
during instruction selection.

[dcarney, 2014-10-27 08:15:08]

https://codereview.chromium.org/677433002/diff/110025/src/compiler/ia32/instr...
File src/compiler/ia32/instruction-selector-ia32.cc (right):

https://codereview.chromium.org/677433002/diff/110025/src/compiler/ia32/instr...
src/compiler/ia32/instruction-selector-ia32.cc:599: void
InstructionSelector::VisitFloat64Floor(Node* node) {
should dcheck that sse4.1 is supported here instead of catching it way
downstream.

[sigurds, 2014-10-28 12:47:23]

Added ARMv8 32bit support (turbofan).

Depends on
https://codereview.chromium.org/682643002/
and
https://codereview.chromium.org/678313002/

https://codereview.chromium.org/677433002/diff/110025/src/compiler/arm/instru...
File src/compiler/arm/instruction-selector-arm.cc (right):

https://codereview.chromium.org/677433002/diff/110025/src/compiler/arm/instru...
src/compiler/arm/instruction-selector-arm.cc:830: UnsupportedOperator(node);
On 2014/10/26 12:48:05, Benedikt Meurer wrote:
> Add TODO for ARMv8.

Done.

https://codereview.chromium.org/677433002/diff/110025/src/compiler/arm/instru...
src/compiler/arm/instruction-selector-arm.cc:835: UnsupportedOperator(node);
On 2014/10/26 12:48:05, Benedikt Meurer wrote:
> Add TODO for ARMv8.

Done.

https://codereview.chromium.org/677433002/diff/110025/src/compiler/arm/instru...
src/compiler/arm/instruction-selector-arm.cc:840: UnsupportedOperator(node);
On 2014/10/26 12:48:05, Benedikt Meurer wrote:
> Add TODO for ARMv8.

Done.

https://codereview.chromium.org/677433002/diff/110025/src/compiler/arm/instru...
src/compiler/arm/instruction-selector-arm.cc:845: UnsupportedOperator(node);
On 2014/10/26 12:48:05, Benedikt Meurer wrote:
> Add TODO for ARMv8.

Done.

https://codereview.chromium.org/677433002/diff/110025/src/compiler/arm64/inst...
File src/compiler/arm64/instruction-selector-arm64.cc (right):

https://codereview.chromium.org/677433002/diff/110025/src/compiler/arm64/inst...
src/compiler/arm64/instruction-selector-arm64.cc:895: Arm64OperandGenerator
g(this);
On 2014/10/26 12:48:05, Benedikt Meurer wrote:
> How about adding a common VisitRRFloat64 helper?

Done.

https://codereview.chromium.org/677433002/diff/110025/src/compiler/ia32/instr...
File src/compiler/ia32/instruction-selector-ia32.cc (right):

https://codereview.chromium.org/677433002/diff/110025/src/compiler/ia32/instr...
src/compiler/ia32/instruction-selector-ia32.cc:599: void
InstructionSelector::VisitFloat64Floor(Node* node) {
On 2014/10/27 08:15:08, dcarney wrote:
> should dcheck that sse4.1 is supported here instead of catching it way
> downstream.

Done.

https://codereview.chromium.org/677433002/diff/110025/src/compiler/instructio...
File src/compiler/instruction-selector.cc (right):

https://codereview.chromium.org/677433002/diff/110025/src/compiler/instructio...
src/compiler/instruction-selector.cc:771: 
On 2014/10/26 12:48:05, Benedikt Meurer wrote:
> Nit: missing newline.

Done.

[Benedikt Meurer, 2014-10-30 11:59:14]

https://codereview.chromium.org/677433002/diff/250001/src/compiler/arm/instru...
File src/compiler/arm/instruction-selector-arm.cc (right):

https://codereview.chromium.org/677433002/diff/250001/src/compiler/arm/instru...
src/compiler/arm/instruction-selector-arm.cc:842: if
(!CpuFeatures::IsSupported(ARMv8)) {
I think this should be DCHECK(CpuFeatures::IsSupported(ARMv8)) instead.

https://codereview.chromium.org/677433002/diff/250001/src/compiler/arm/instru...
src/compiler/arm/instruction-selector-arm.cc:850: if
(!CpuFeatures::IsSupported(ARMv8)) {
I think this should be DCHECK(CpuFeatures::IsSupported(ARMv8)) instead.

https://codereview.chromium.org/677433002/diff/250001/src/compiler/arm/instru...
src/compiler/arm/instruction-selector-arm.cc:858: if
(!CpuFeatures::IsSupported(ARMv8)) {
I think this should be DCHECK(CpuFeatures::IsSupported(ARMv8)) instead.

https://codereview.chromium.org/677433002/diff/250001/src/compiler/arm/instru...
src/compiler/arm/instruction-selector-arm.cc:866: if
(!CpuFeatures::IsSupported(ARMv8)) {
I think this should be DCHECK(CpuFeatures::IsSupported(ARMv8)) instead.

https://codereview.chromium.org/677433002/diff/250001/src/compiler/ia32/instr...
File src/compiler/ia32/instruction-selector-ia32.cc (right):

https://codereview.chromium.org/677433002/diff/250001/src/compiler/ia32/instr...
src/compiler/ia32/instruction-selector-ia32.cc:608: if
(!CpuFeatures::IsSupported(SSE4_1)) {
I think this should be DCHECK(CpuFeatures::IsSupported(ARMv8)) instead.

https://codereview.chromium.org/677433002/diff/250001/src/compiler/ia32/instr...
src/compiler/ia32/instruction-selector-ia32.cc:616: if
(!CpuFeatures::IsSupported(SSE4_1)) {
I think this should be DCHECK(CpuFeatures::IsSupported(ARMv8)) instead.

https://codereview.chromium.org/677433002/diff/250001/src/compiler/ia32/instr...
src/compiler/ia32/instruction-selector-ia32.cc:624: if
(!CpuFeatures::IsSupported(SSE4_1)) {
I think this should be DCHECK(CpuFeatures::IsSupported(ARMv8)) instead.

https://codereview.chromium.org/677433002/diff/250001/src/compiler/instructio...
File src/compiler/instruction-selector.h (right):

https://codereview.chromium.org/677433002/diff/250001/src/compiler/instructio...
src/compiler/instruction-selector.h:16: #define UNSUPPORTED_OPERATOR(node)      
                               \
Please remove this macro and use UNREACHABLE() in Visit<OP>() methods or DCHECK
for the required features.

https://codereview.chromium.org/677433002/diff/250001/src/compiler/x64/instru...
File src/compiler/x64/instruction-selector-x64.cc (right):

https://codereview.chromium.org/677433002/diff/250001/src/compiler/x64/instru...
src/compiler/x64/instruction-selector-x64.cc:735: if
(!CpuFeatures::IsSupported(SSE4_1)) {
I think this should be DCHECK(CpuFeatures::IsSupported(ARMv8)) instead.

https://codereview.chromium.org/677433002/diff/250001/src/compiler/x64/instru...
src/compiler/x64/instruction-selector-x64.cc:743: if
(!CpuFeatures::IsSupported(SSE4_1)) {
I think this should be DCHECK(CpuFeatures::IsSupported(ARMv8)) instead.

https://codereview.chromium.org/677433002/diff/250001/src/compiler/x64/instru...
src/compiler/x64/instruction-selector-x64.cc:751: if
(!CpuFeatures::IsSupported(SSE4_1)) {
I think this should be DCHECK(CpuFeatures::IsSupported(ARMv8)) instead.

https://codereview.chromium.org/677433002/diff/250001/src/compiler/x64/instru...
src/compiler/x64/instruction-selector-x64.cc:759: UNSUPPORTED_OPERATOR(node);
Just put in UNREACHABLE(); here.

https://codereview.chromium.org/677433002/diff/250001/src/compiler/x64/instru...
src/compiler/x64/instruction-selector-x64.cc:759: UNSUPPORTED_OPERATOR(node);
Just put in UNREACHABLE(); here.

[Benedikt Meurer, 2014-10-30 11:59:48]

And of course fix MAc compilation.

[sigurds, 2014-10-30 12:18:27]

Thanks for the review, addressed comments, PTALA.

https://codereview.chromium.org/677433002/diff/250001/src/compiler/arm/instru...
File src/compiler/arm/instruction-selector-arm.cc (right):

https://codereview.chromium.org/677433002/diff/250001/src/compiler/arm/instru...
src/compiler/arm/instruction-selector-arm.cc:842: if
(!CpuFeatures::IsSupported(ARMv8)) {
On 2014/10/30 11:59:12, Benedikt Meurer wrote:
> I think this should be DCHECK(CpuFeatures::IsSupported(ARMv8)) instead.

Done.

https://codereview.chromium.org/677433002/diff/250001/src/compiler/arm/instru...
src/compiler/arm/instruction-selector-arm.cc:850: if
(!CpuFeatures::IsSupported(ARMv8)) {
On 2014/10/30 11:59:12, Benedikt Meurer wrote:
> I think this should be DCHECK(CpuFeatures::IsSupported(ARMv8)) instead.

Done.

https://codereview.chromium.org/677433002/diff/250001/src/compiler/arm/instru...
src/compiler/arm/instruction-selector-arm.cc:858: if
(!CpuFeatures::IsSupported(ARMv8)) {
On 2014/10/30 11:59:12, Benedikt Meurer wrote:
> I think this should be DCHECK(CpuFeatures::IsSupported(ARMv8)) instead.

Done.

https://codereview.chromium.org/677433002/diff/250001/src/compiler/arm/instru...
src/compiler/arm/instruction-selector-arm.cc:866: if
(!CpuFeatures::IsSupported(ARMv8)) {
On 2014/10/30 11:59:12, Benedikt Meurer wrote:
> I think this should be DCHECK(CpuFeatures::IsSupported(ARMv8)) instead.

Done.

https://codereview.chromium.org/677433002/diff/250001/src/compiler/ia32/instr...
File src/compiler/ia32/instruction-selector-ia32.cc (right):

https://codereview.chromium.org/677433002/diff/250001/src/compiler/ia32/instr...
src/compiler/ia32/instruction-selector-ia32.cc:608: if
(!CpuFeatures::IsSupported(SSE4_1)) {
On 2014/10/30 11:59:12, Benedikt Meurer wrote:
> I think this should be DCHECK(CpuFeatures::IsSupported(ARMv8)) instead.

Done.

https://codereview.chromium.org/677433002/diff/250001/src/compiler/ia32/instr...
src/compiler/ia32/instruction-selector-ia32.cc:616: if
(!CpuFeatures::IsSupported(SSE4_1)) {
On 2014/10/30 11:59:13, Benedikt Meurer wrote:
> I think this should be DCHECK(CpuFeatures::IsSupported(ARMv8)) instead.

Done.

https://codereview.chromium.org/677433002/diff/250001/src/compiler/ia32/instr...
src/compiler/ia32/instruction-selector-ia32.cc:624: if
(!CpuFeatures::IsSupported(SSE4_1)) {
On 2014/10/30 11:59:12, Benedikt Meurer wrote:
> I think this should be DCHECK(CpuFeatures::IsSupported(ARMv8)) instead.

Done.

https://codereview.chromium.org/677433002/diff/250001/src/compiler/instructio...
File src/compiler/instruction-selector.h (right):

https://codereview.chromium.org/677433002/diff/250001/src/compiler/instructio...
src/compiler/instruction-selector.h:16: #define UNSUPPORTED_OPERATOR(node)      
                               \
On 2014/10/30 11:59:13, Benedikt Meurer wrote:
> Please remove this macro and use UNREACHABLE() in Visit<OP>() methods or
DCHECK
> for the required features.

Done.

https://codereview.chromium.org/677433002/diff/250001/src/compiler/x64/instru...
File src/compiler/x64/instruction-selector-x64.cc (right):

https://codereview.chromium.org/677433002/diff/250001/src/compiler/x64/instru...
src/compiler/x64/instruction-selector-x64.cc:735: if
(!CpuFeatures::IsSupported(SSE4_1)) {
On 2014/10/30 11:59:13, Benedikt Meurer wrote:
> I think this should be DCHECK(CpuFeatures::IsSupported(ARMv8)) instead.

Done.

https://codereview.chromium.org/677433002/diff/250001/src/compiler/x64/instru...
src/compiler/x64/instruction-selector-x64.cc:743: if
(!CpuFeatures::IsSupported(SSE4_1)) {
On 2014/10/30 11:59:13, Benedikt Meurer wrote:
> I think this should be DCHECK(CpuFeatures::IsSupported(ARMv8)) instead.

Done.

https://codereview.chromium.org/677433002/diff/250001/src/compiler/x64/instru...
src/compiler/x64/instruction-selector-x64.cc:751: if
(!CpuFeatures::IsSupported(SSE4_1)) {
On 2014/10/30 11:59:13, Benedikt Meurer wrote:
> I think this should be DCHECK(CpuFeatures::IsSupported(ARMv8)) instead.

Done.

https://codereview.chromium.org/677433002/diff/250001/src/compiler/x64/instru...
src/compiler/x64/instruction-selector-x64.cc:759: UNSUPPORTED_OPERATOR(node);
On 2014/10/30 11:59:13, Benedikt Meurer wrote:
> Just put in UNREACHABLE(); here.

Done.

[Benedikt Meurer, 2014-10-30 12:21:30]

LGTM

[sigurds, 2014-10-30 14:15:51]

Committed patchset #13 (id:270001) manually as 25018 (presubmit successful).