Implement remaining Boolean SIMD operations on ARM.

Implement remaining Boolean SIMD operations on ARM.

- Implements Select instructions using a single ARM vbsl instruction.
- Renames boolean machine operators to match renamed S1xN machine types.
- Implements S1xN vector logical ops, AND, OR, XOR, NOT for ARM.
- Implements S1xN AnyTrue, AllTrue ops for ARM.
- Eliminates unused SIMD op categories in opcodes.h.

LOG=N
BUG=v8:6020
Review-Url: https://codereview.chromium.org/2711863002
Cr-Commit-Position: refs/heads/master@{#43556}
Committed: https://chromium.googlesource.com/v8/v8/+/386e5a1149dcf5d2834773604c6934c0e797ba0c

[bbudge, 2017-02-23 02:40:34]

Description was changed from

==========
mplement remaining Boolean SIMD operations on ARM.

- Combines ARM Select instructions into a single instruction.
- Renames machine operators to match renamed SIMD bool vector machine types.
- Implements Boolean vector logical ops, AND, OR, XOR, NOT for ARM.
- Implements AnyTrue, AllTrue ops for ARM.
- Eliminates unused SIMD op categories in opcodes.h.

LOG=N
BUG=v8:4124
==========

to

==========
Implement remaining Boolean SIMD operations on ARM.

- Implements Select instructions using a single ARM vbsl instruction.
- Renames boolean machine operators to match renamed S1xN machine types.
- Implements S1xN vector logical ops, AND, OR, XOR, NOT for ARM.
- Implements S1xN AnyTrue, AllTrue ops for ARM.
- Eliminates unused SIMD op categories in opcodes.h.

LOG=N
BUG=v8:4124
==========

[bbudge, 2017-02-28 01:42:06]

The CQ bit was checked by bbudge@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-28 01:42:14]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[bbudge, 2017-02-28 02:35:22]

bbudge@chromium.org changed reviewers:
+ bmeurer@chromium.org, martyn.capewell@arm.com

[bbudge, 2017-02-28 02:35:23]

Martyn for ARM specific changes.
Benedikt for Turbofan / Wasm

[commit-bot: I haz the power, 2017-02-28 02:52:52]

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

[commit-bot: I haz the power, 2017-02-28 02:52:53]

Dry run: This issue passed the CQ dry run.

[Benedikt Meurer, 2017-02-28 04:55:25]

LGTM on TurboFan changes.

[martyn.capewell, 2017-02-28 15:45:24]

https://codereview.chromium.org/2711863002/diff/80001/src/arm/assembler-arm.cc
File src/arm/assembler-arm.cc (right):

https://codereview.chromium.org/2711863002/diff/80001/src/arm/assembler-arm.c...
src/arm/assembler-arm.cc:4083: case VABS:
Depending on whether you like fallthrough cases, these could be written as:

case VABSF:
  DCHECK_EQ(Neon32, size);
  op_encoding = B10;
  // Fall through.
case VABS:
  op_encoding |= 0x6 * B7;
  break;

and similarly for VNEG.

https://codereview.chromium.org/2711863002/diff/80001/src/arm/assembler-arm.c...
src/arm/assembler-arm.cc:4105: return 0x1E7U * B23 | d * B22 | 0x3 * B20 | size
* B18 | B16 | vd * B12 | B8 |
The switch above always sets B8, so it can be removed from here.

https://codereview.chromium.org/2711863002/diff/80001/src/arm/assembler-arm.c...
src/arm/assembler-arm.cc:4578: // Qd = vpmin(Qn, Qm) SIMD integer pairwise MIN.
These are D registers for min and max.

https://codereview.chromium.org/2711863002/diff/80001/src/arm/disasm-arm.cc
File src/arm/disasm-arm.cc (right):

https://codereview.chromium.org/2711863002/diff/80001/src/arm/disasm-arm.cc#n...
src/arm/disasm-arm.cc:1957: int size = kBitsPerByte * (1 << instr->Bits(21,
20));
It might be worthwhile calculating this outside the switch, as it's used in many
of the cases, and/or adding a method to the Instruction class. The simulator
needs bits 21-20 too, though it creates an enumeration with them.

https://codereview.chromium.org/2711863002/diff/80001/src/arm/simulator-arm.cc
File src/arm/simulator-arm.cc (right):

https://codereview.chromium.org/2711863002/diff/80001/src/arm/simulator-arm.c...
src/arm/simulator-arm.cc:4367: if (min) {
How about a templated minmax helper?
template <typename T>
T Simulator::minmax(T a, T b, bool is_min) {
  return is_min ? std::min(a, b) : std::max(a, b);
}

(I looked at std::minmax, but that doesn't seem to simplify anything here)

Then each case becomes something like:
for (int i = 0; i < 4; i++) {
  dst[i + 0] = minmax(src1[i * 2], src1[i * 2 + 1], is_min);
  dst[i + 4] = minmax(src2[i * 2], src2[i * 2 + 1], is_min);
}

[bbudge, 2017-02-28 17:39:21]

The CQ bit was checked by bbudge@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-28 17:39:27]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[bbudge, 2017-02-28 17:39:35]

https://codereview.chromium.org/2711863002/diff/80001/src/arm/assembler-arm.cc
File src/arm/assembler-arm.cc (right):

https://codereview.chromium.org/2711863002/diff/80001/src/arm/assembler-arm.c...
src/arm/assembler-arm.cc:4083: case VABS:
On 2017/02/28 15:45:24, martyn.capewell wrote:
> Depending on whether you like fallthrough cases, these could be written as:
> 
> case VABSF:
>   DCHECK_EQ(Neon32, size);
>   op_encoding = B10;
>   // Fall through.
> case VABS:
>   op_encoding |= 0x6 * B7;
>   break;
> 
> and similarly for VNEG.

I think v8 style is to avoid fall through unless it's very compelling.

https://codereview.chromium.org/2711863002/diff/80001/src/arm/assembler-arm.c...
src/arm/assembler-arm.cc:4105: return 0x1E7U * B23 | d * B22 | 0x3 * B20 | size
* B18 | B16 | vd * B12 | B8 |
On 2017/02/28 15:45:24, martyn.capewell wrote:
> The switch above always sets B8, so it can be removed from here.

Wow, good eye. Done.

https://codereview.chromium.org/2711863002/diff/80001/src/arm/assembler-arm.c...
src/arm/assembler-arm.cc:4578: // Qd = vpmin(Qn, Qm) SIMD integer pairwise MIN.
On 2017/02/28 15:45:24, martyn.capewell wrote:
> These are D registers for min and max.

Done here and below.

https://codereview.chromium.org/2711863002/diff/80001/src/arm/disasm-arm.cc
File src/arm/disasm-arm.cc (right):

https://codereview.chromium.org/2711863002/diff/80001/src/arm/disasm-arm.cc#n...
src/arm/disasm-arm.cc:1957: int size = kBitsPerByte * (1 << instr->Bits(21,
20));
On 2017/02/28 15:45:24, martyn.capewell wrote:
> It might be worthwhile calculating this outside the switch, as it's used in
many
> of the cases, and/or adding a method to the Instruction class. The simulator
> needs bits 21-20 too, though it creates an enumeration with them.

I moved these to the top of cases 0x4, 0x6, and 0xA to reduce duplication.

https://codereview.chromium.org/2711863002/diff/80001/src/arm/simulator-arm.cc
File src/arm/simulator-arm.cc (right):

https://codereview.chromium.org/2711863002/diff/80001/src/arm/simulator-arm.c...
src/arm/simulator-arm.cc:4367: if (min) {
On 2017/02/28 15:45:24, martyn.capewell wrote:
> How about a templated minmax helper?
> template <typename T>
> T Simulator::minmax(T a, T b, bool is_min) {
>   return is_min ? std::min(a, b) : std::max(a, b);
> }
> 
> (I looked at std::minmax, but that doesn't seem to simplify anything here)
> 
> Then each case becomes something like:
> for (int i = 0; i < 4; i++) {
>   dst[i + 0] = minmax(src1[i * 2], src1[i * 2 + 1], is_min);
>   dst[i + 4] = minmax(src2[i * 2], src2[i * 2 + 1], is_min);
> }

That does look nicer. Done.

[commit-bot: I haz the power, 2017-02-28 18:06:23]

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

[commit-bot: I haz the power, 2017-02-28 18:06:24]

Dry run: This issue passed the CQ dry run.

[bbudge, 2017-03-01 00:25:33]

Description was changed from

==========
Implement remaining Boolean SIMD operations on ARM.

- Implements Select instructions using a single ARM vbsl instruction.
- Renames boolean machine operators to match renamed S1xN machine types.
- Implements S1xN vector logical ops, AND, OR, XOR, NOT for ARM.
- Implements S1xN AnyTrue, AllTrue ops for ARM.
- Eliminates unused SIMD op categories in opcodes.h.

LOG=N
BUG=v8:4124
==========

to

==========
Implement remaining Boolean SIMD operations on ARM.

- Implements Select instructions using a single ARM vbsl instruction.
- Renames boolean machine operators to match renamed S1xN machine types.
- Implements S1xN vector logical ops, AND, OR, XOR, NOT for ARM.
- Implements S1xN AnyTrue, AllTrue ops for ARM.
- Eliminates unused SIMD op categories in opcodes.h.

LOG=N
BUG=v8:6020
==========

[martyn.capewell, 2017-03-01 14:04:05]

A couple of extra comments after some thought about a better way to write the
codegen. Then I think it's good.

https://codereview.chromium.org/2711863002/diff/100001/src/compiler/arm/code-...
File src/compiler/arm/code-generator-arm.cc (right):

https://codereview.chromium.org/2711863002/diff/100001/src/compiler/arm/code-...
src/compiler/arm/code-generator-arm.cc:1942: __ ExtractLane(i.OutputRegister(),
kScratchQuadReg, NeonS32, 2);
An ExtractLane function that accepts a double register would avoid the need for
kScratchQuadReg, the adjacency with kDoubleRegZero, and the odd index.

https://codereview.chromium.org/2711863002/diff/100001/src/compiler/arm/code-...
src/compiler/arm/code-generator-arm.cc:1976: case kArmSimd1x16AnyTrue: {
I think we could save an instruction here:
  __ vorr(kScratchDoubleReg, src.low(), src.high());
  __ vpmax(NeonU8, kScratchDoubleReg, kScratchDoubleReg, kScratchDoubleReg);
  __ vcgt(NeonU32, kScratchDoubleReg, kScratchDoubleReg, kZeroDoubleReg);
  __ ExtractLane(...);

[bbudge, 2017-03-02 00:31:52]

The CQ bit was checked by bbudge@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-02 00:31:54]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[bbudge, 2017-03-02 00:34:47]

https://codereview.chromium.org/2711863002/diff/100001/src/compiler/arm/code-...
File src/compiler/arm/code-generator-arm.cc (right):

https://codereview.chromium.org/2711863002/diff/100001/src/compiler/arm/code-...
src/compiler/arm/code-generator-arm.cc:1942: __ ExtractLane(i.OutputRegister(),
kScratchQuadReg, NeonS32, 2);
On 2017/03/01 14:04:05, martyn.capewell wrote:
> An ExtractLane function that accepts a double register would avoid the need
for
> kScratchQuadReg, the adjacency with kDoubleRegZero, and the odd index.

That's a nice improvement. Done.

https://codereview.chromium.org/2711863002/diff/100001/src/compiler/arm/code-...
src/compiler/arm/code-generator-arm.cc:1976: case kArmSimd1x16AnyTrue: {
On 2017/03/01 14:04:05, martyn.capewell wrote:
> I think we could save an instruction here:
>   __ vorr(kScratchDoubleReg, src.low(), src.high());
>   __ vpmax(NeonU8, kScratchDoubleReg, kScratchDoubleReg, kScratchDoubleReg);
>   __ vcgt(NeonU32, kScratchDoubleReg, kScratchDoubleReg, kZeroDoubleReg);
>   __ ExtractLane(...);

I'd rather not add the DwVfpRegister versions of Neon instructions if we don't
have to. You inspired me though, and we can get a similar effect with
    vpmax
    vpmax
    vtst  (turns any bits in low 32 bits to 0xffffffff.
    ExtractLane

https://codereview.chromium.org/2711863002/diff/100001/src/compiler/arm/code-...
src/compiler/arm/code-generator-arm.cc:1978: __ vpmax(NeonU16,
kScratchDoubleReg, src.low(), src.high());
whoops, typo: NeonU16

[commit-bot: I haz the power, 2017-03-02 00:59:41]

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

[commit-bot: I haz the power, 2017-03-02 00:59:42]

Dry run: Try jobs failed on following builders:
  v8_linux_arm_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm_rel_ng/builds/...)
  v8_linux_arm_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm_rel_ng_trigger...)

[martyn.capewell, 2017-03-02 14:09:03]

lgtm

[bbudge, 2017-03-02 16:13:29]

The CQ bit was checked by bbudge@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-02 16:13:35]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-02 16:41:42]

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

[commit-bot: I haz the power, 2017-03-02 16:41:43]

Dry run: This issue passed the CQ dry run.

[bbudge, 2017-03-02 16:55:28]

The CQ bit was checked by bbudge@chromium.org

[bbudge, 2017-03-02 16:55:29]

The patchset sent to the CQ was uploaded after l-g-t-m from
bmeurer@chromium.org, martyn.capewell@arm.com
Link to the patchset: https://codereview.chromium.org/2711863002/#ps140001
(title: "Fix macro assembler test.")

[commit-bot: I haz the power, 2017-03-02 16:55:32]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-02 16:58:41]

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

[commit-bot: I haz the power, 2017-03-02 16:58:42]

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

[bbudge, 2017-03-02 17:04:11]

bbudge@chromium.org changed reviewers:
+ bradnelson@chromium.org

[bbudge, 2017-03-02 17:04:12]

+Brad for OWNERS wasm-opcodes.*

[bbudge, 2017-03-02 19:25:22]

bbudge@chromium.org changed reviewers:
+ titzer@chromium.org
- bradnelson@chromium.org

[bbudge, 2017-03-02 19:25:22]

+ Ben, -Brad

[titzer, 2017-03-02 19:29:03]

On 2017/03/02 19:25:22, bbudge wrote:
> + Ben, -Brad

lgtm

[bbudge, 2017-03-02 19:47:45]

The CQ bit was checked by bbudge@chromium.org

[commit-bot: I haz the power, 2017-03-02 19:47:54]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-02 19:50:34]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1488484065591680,
"parent_rev": "91cd0707698a7447f881dee2bd3a4c7bc27613a2", "commit_rev":
"386e5a1149dcf5d2834773604c6934c0e797ba0c"}

[commit-bot: I haz the power, 2017-03-02 19:50:39]

Description was changed from

==========
Implement remaining Boolean SIMD operations on ARM.

- Implements Select instructions using a single ARM vbsl instruction.
- Renames boolean machine operators to match renamed S1xN machine types.
- Implements S1xN vector logical ops, AND, OR, XOR, NOT for ARM.
- Implements S1xN AnyTrue, AllTrue ops for ARM.
- Eliminates unused SIMD op categories in opcodes.h.

LOG=N
BUG=v8:6020
==========

to

==========
Implement remaining Boolean SIMD operations on ARM.

- Implements Select instructions using a single ARM vbsl instruction.
- Renames boolean machine operators to match renamed S1xN machine types.
- Implements S1xN vector logical ops, AND, OR, XOR, NOT for ARM.
- Implements S1xN AnyTrue, AllTrue ops for ARM.
- Eliminates unused SIMD op categories in opcodes.h.

LOG=N
BUG=v8:6020

Review-Url: https://codereview.chromium.org/2711863002
Cr-Commit-Position: refs/heads/master@{#43556}
Committed:
https://chromium.googlesource.com/v8/v8/+/386e5a1149dcf5d2834773604c6934c0e79...
==========

[commit-bot: I haz the power, 2017-03-02 19:50:40]

Committed patchset #8 (id:140001) as
https://chromium.googlesource.com/v8/v8/+/386e5a1149dcf5d2834773604c6934c0e79...