[ARM] Add Neon saturating add and subtract instructions.

[ARM] Add Neon saturating add and subtract instructions.
- Adds vqadd.s/u, vqsub.s/u for all integer lane sizes.
- Refactors disassembler and simulator, using switches instead
of long if-else chains.

LOG=N
BUG=v8:4124
Review-Url: https://codereview.chromium.org/2649323012
Cr-Commit-Position: refs/heads/master@{#42865}
Committed: https://chromium.googlesource.com/v8/v8/+/b7df78f363da3f14e8ac54b3498a3556d3fba740

[bbudge, 2017-01-26 23:30:03]

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

[commit-bot: I haz the power, 2017-01-26 23:30:12]

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-01-27 01:02:15]

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

[commit-bot: I haz the power, 2017-01-27 01:02:16]

Dry run: This issue passed the CQ dry run.

[bbudge, 2017-01-27 02:33:28]

Description was changed from

==========
[ARM] Add Neon saturating add and subtract instructions.
- Adds vqadd.s/u, vqsub.s/u for all integer lane sizes.

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

to

==========
[ARM] Add Neon saturating add and subtract instructions.
- Adds vqadd.s/u, vqsub.s/u for all integer lane sizes.
- Refactors disassembler and simulator, using switches instead
of long if-else chains.

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

[bbudge, 2017-01-27 02:33:56]

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

[martyn.capewell, 2017-01-30 13:50:47]

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

https://codereview.chromium.org/2649323012/diff/1/src/arm/simulator-arm.cc#ne...
src/arm/simulator-arm.cc:4011: int8_t src1[16], src2[16];
It might be possible to factor out simulator operations into templated
functions, so the repeated code in each case is minimised. Exploit the templated
register accessors you've already written, and calculate internally at a common
high precision, before casting down to the target type. Something like:

void Simulator::SaturatingAdd(int Vd, int Vm, int Vn) {
  T src1[16], src2[16];
  get_q_register(Vn, src1);
  get_q_register(Vm, src2);
  for (int i = 0; i < arraysize(src1) / (1 << size); i++) {
    int64_t s1 = static_cast<int64_t>(src1[i]);
    int64_t s2 = static_cast<int64_t>(src2[i]);
    int64_t sum = std::max(std::numeric_limits<T>::max(), s1 + s2);
    sum = std::min(std::numeric_limits<T>::min(), sum);
    src1[i] = static_cast<T>(sum);
  }
  set_q_register(Vd, src1);
}

Though this is just a sketch; I haven't built this.

[bbudge, 2017-01-31 19:45:40]

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

[commit-bot: I haz the power, 2017-01-31 19:45:53]

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-01-31 19:47:22]

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

[commit-bot: I haz the power, 2017-01-31 19:47:23]

Dry run: Try jobs failed on following builders:
  v8_linux64_asan_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel_ng/buil...)
  v8_linux64_gyp_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng/build...)

[bbudge, 2017-01-31 19:58:24]

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

https://codereview.chromium.org/2649323012/diff/1/src/arm/simulator-arm.cc#ne...
src/arm/simulator-arm.cc:4011: int8_t src1[16], src2[16];
On 2017/01/30 13:50:47, martyn.capewell wrote:
> It might be possible to factor out simulator operations into templated
> functions, so the repeated code in each case is minimised. Exploit the
templated
> register accessors you've already written, and calculate internally at a
common
> high precision, before casting down to the target type. Something like:
> 
> void Simulator::SaturatingAdd(int Vd, int Vm, int Vn) {
>   T src1[16], src2[16];
>   get_q_register(Vn, src1);
>   get_q_register(Vm, src2);
>   for (int i = 0; i < arraysize(src1) / (1 << size); i++) {
>     int64_t s1 = static_cast<int64_t>(src1[i]);
>     int64_t s2 = static_cast<int64_t>(src2[i]);
>     int64_t sum = std::max(std::numeric_limits<T>::max(), s1 + s2);
>     sum = std::min(std::numeric_limits<T>::min(), sum);
>     src1[i] = static_cast<T>(sum);
>   }
>   set_q_register(Vd, src1);
> }
> 
> Though this is just a sketch; I haven't built this.

Done for vqadd/vqsub, and added a TODO to do this for more NEON operations.

[bbudge, 2017-02-01 00:26:37]

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

[commit-bot: I haz the power, 2017-02-01 00:26:48]

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-02-01 00:48:27]

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

[commit-bot: I haz the power, 2017-02-01 00:48:28]

Dry run: This issue passed the CQ dry run.

[martyn.capewell, 2017-02-01 11:44:15]

lgtm

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

https://codereview.chromium.org/2649323012/diff/20001/src/arm/simulator-arm.c...
src/arm/simulator-arm.cc:3995: return static_cast<int64_t>(value);
Perhaps assert sizeof(T) < sizeof(value), as this is UB for T=uint64_t.
Similarly for Clamp().

[martyn.capewell, 2017-02-01 11:45:49]

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

https://codereview.chromium.org/2649323012/diff/20001/src/arm/simulator-arm.c...
src/arm/simulator-arm.cc:3995: return static_cast<int64_t>(value);
I mean, sizeof(value) < sizeof(int64_t)

[bbudge, 2017-02-01 18:10:48]

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

[commit-bot: I haz the power, 2017-02-01 18:10:57]

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-01 18:12:19]

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

[bbudge, 2017-02-01 18:12:20]

+Ben for OWNERS

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

https://codereview.chromium.org/2649323012/diff/20001/src/arm/simulator-arm.c...
src/arm/simulator-arm.cc:3995: return static_cast<int64_t>(value);
On 2017/02/01 11:45:49, martyn.capewell wrote:
> I mean, sizeof(value) < sizeof(int64_t)

Done.

[titzer, 2017-02-01 18:24:18]

lgtm

[commit-bot: I haz the power, 2017-02-01 18:38:32]

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

[commit-bot: I haz the power, 2017-02-01 18:38:33]

Dry run: This issue passed the CQ dry run.

[bbudge, 2017-02-01 18:54:48]

The CQ bit was checked by bbudge@chromium.org

[bbudge, 2017-02-01 18:54:49]

The patchset sent to the CQ was uploaded after l-g-t-m from
martyn.capewell@arm.com
Link to the patchset: https://codereview.chromium.org/2649323012/#ps40001
(title: "Add static_asserts on Widen, Clamp.")

[commit-bot: I haz the power, 2017-02-01 18:54:52]

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-02-01 18:56:29]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1485975288523290,
"parent_rev": "5a02d3e88da56db72364a28c294f58117945f5a7", "commit_rev":
"b7df78f363da3f14e8ac54b3498a3556d3fba740"}

[commit-bot: I haz the power, 2017-02-01 18:56:36]

Description was changed from

==========
[ARM] Add Neon saturating add and subtract instructions.
- Adds vqadd.s/u, vqsub.s/u for all integer lane sizes.
- Refactors disassembler and simulator, using switches instead
of long if-else chains.

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

to

==========
[ARM] Add Neon saturating add and subtract instructions.
- Adds vqadd.s/u, vqsub.s/u for all integer lane sizes.
- Refactors disassembler and simulator, using switches instead
of long if-else chains.

LOG=N
BUG=v8:4124

Review-Url: https://codereview.chromium.org/2649323012
Cr-Commit-Position: refs/heads/master@{#42865}
Committed:
https://chromium.googlesource.com/v8/v8/+/b7df78f363da3f14e8ac54b3498a3556d3f...
==========

[commit-bot: I haz the power, 2017-02-01 18:56:37]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/v8/v8/+/b7df78f363da3f14e8ac54b3498a3556d3f...