[wasm] Swap the implementation of SIMD compare ops using Gt/Ge insteas of Lt/Le

[wasm] Swap the implementation of SIMD compare ops using Gt/Ge insteas of Lt/Le

Currently SIMD integer comparison ops are implemented using Lt/Le, this is
sub-optimal on Intel, because all compares are done using pcmpgt(d/w/b) that
clobber the destination register, and will need additional instructions to
when using Lt/Le as the base implementation. This CL proposes moving to Gt/Ge
as the underlying implementation as this will only require swapping operands
on MIPS and is consistent with x86/ARM instructions.

BUG=v8:6020
R=bbudge@chromium.org, bmeurer@chromium.org, bradnelson@chromium.org

Review-Url: https://codereview.chromium.org/2874403002
Cr-Commit-Position: refs/heads/master@{#45440}
Committed: https://chromium.googlesource.com/v8/v8/+/eeefc74a1189aeed19228f728b60b0f187b5b1bd

[gdeepti, 2017-05-12 05:48:12]

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

[commit-bot: I haz the power, 2017-05-12 05:48:16]

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-05-12 05:51:05]

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

[commit-bot: I haz the power, 2017-05-12 05:51:06]

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

[gdeepti, 2017-05-12 06:07:03]

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

[commit-bot: I haz the power, 2017-05-12 06:07:08]

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-05-12 06:38:51]

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

[commit-bot: I haz the power, 2017-05-12 06:38:52]

Dry run: This issue passed the CQ dry run.

[bbudge, 2017-05-16 19:38:32]

lgtm

[gdeepti, 2017-05-16 19:44:05]

Description was changed from

==========
[wasm] Swap the implementation of SIMD compare ops using Gt/Ge insteas of Lt/Le

Currently SIMD integer comparison ops are implemented using Lt/Le, this is
sub-optimal on Intel, because all compares are done using pcmpgt(d/w/b) that
clobber the destination register, and will need additional instructions to
when using Lt/Le as the base implementation. This CL proposes moving to Gt/Ge
as the underlying implementation as this will only require swapping operands
on MIPS and is consistent with x86/ARM instructions.

BUG=v8:6020

R=bbudge@chromium.org, bmeurer@chromium.org, bradnelson@chromium.org
==========

to

==========
[wasm] Swap the implementation of SIMD compare ops using Gt/Ge insteas of Lt/Le

Currently SIMD integer comparison ops are implemented using Lt/Le, this is
sub-optimal on Intel, because all compares are done using pcmpgt(d/w/b) that
clobber the destination register, and will need additional instructions to
when using Lt/Le as the base implementation. This CL proposes moving to Gt/Ge
as the underlying implementation as this will only require swapping operands
on MIPS and is consistent with x86/ARM instructions.

BUG=v8:6020

R=bbudge@chromium.org, bmeurer@chromium.org, bradnelson@chromium.org
==========

[gdeepti, 2017-05-17 19:59:17]

gdeepti@chromium.org changed reviewers:
+ mtrofin@chromium.org

[gdeepti, 2017-05-17 19:59:18]

+mtrofin for OWNERS in src/compiler

[Mircea Trofin, 2017-05-17 21:30:35]

On 2017/05/17 19:59:18, gdeepti wrote:
> +mtrofin for OWNERS in src/compiler

lgtm

[gdeepti, 2017-05-17 22:00:21]

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

[commit-bot: I haz the power, 2017-05-17 22:00:26]

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-05-17 22:05:45]

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

[commit-bot: I haz the power, 2017-05-17 22:05:46]

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

[gdeepti, 2017-05-17 22:16:25]

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

[commit-bot: I haz the power, 2017-05-17 22:16: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-05-17 22:21:45]

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

[commit-bot: I haz the power, 2017-05-17 22:21:46]

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

[gdeepti, 2017-05-17 23:24:38]

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

[commit-bot: I haz the power, 2017-05-17 23:24:47]

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-05-17 23:33:59]

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

[commit-bot: I haz the power, 2017-05-17 23:34:01]

Dry run: Try jobs failed on following builders:
  v8_linux_arm64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel_ng/build...)
  v8_linux_arm64_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel_ng_trigg...)

[zvi, 2017-05-18 06:25:45]

The patch LGTM.
Newb question: are there tests that cover code generation of vector-compare
operations to X86 instructions? I tried looking under test/ but couldn't find
any.

[gdeepti, 2017-05-19 21:20:28]

On 2017/05/18 06:25:45, zvi wrote:
> The patch LGTM.
> Newb question: are there tests that cover code generation of vector-compare
> operations to X86 instructions? I tried looking under test/ but couldn't find
> any.

As far as I know, V8 doesn't have tests that cover code generation of specific
instructions, mostly relies on end-to-end compiler tests, or WebAssembly tests
in some cases,  and arch specific disassembly tests( in
test/cctest/test-disasm-x64.cc). Though I've found that the disassembly tests do
not have the best coverage for vector instructions pcmpgtd/b/w for example does
not have a disassembly test, these have assembler support, but are not being
used for code gen, I'll add a test in a follow up patch with the x64
implementation for x64 Gt/Ge ops.

[gdeepti, 2017-05-19 23:27:15]

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

[commit-bot: I haz the power, 2017-05-19 23:27:18]

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-05-19 23:30:27]

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

[commit-bot: I haz the power, 2017-05-19 23:30:28]

Dry run: Try jobs failed on following builders:
  v8_linux64_avx2_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng/buil...)
  v8_linux_dbg_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng/builds/26267)
  v8_linux_nodcheck_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel_ng/bu...)
  v8_linux_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_rel_ng/builds/26177)

[gdeepti, 2017-05-19 23:31:44]

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

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

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-05-19 23:33:55]

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

[commit-bot: I haz the power, 2017-05-19 23:33:56]

Dry run: Try jobs failed on following builders:
  v8_linux_arm64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel_ng/build...)
  v8_linux_nodcheck_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel_ng/bu...)
  v8_linux_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_rel_ng/builds/26178)

[gdeepti, 2017-05-19 23:41:39]

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

[commit-bot: I haz the power, 2017-05-19 23:41:45]

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-05-20 00:11:37]

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

[commit-bot: I haz the power, 2017-05-20 00:11:38]

Dry run: This issue passed the CQ dry run.

[gdeepti, 2017-05-20 00:33:21]

Filed bug - https://bugs.chromium.org/p/v8/issues/detail?id=6421, as this CL
disables I8x16 scalar lowering compare tests on ARM64.

[gdeepti, 2017-05-20 00:34:27]

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

[commit-bot: I haz the power, 2017-05-20 00:34:31]

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-05-20 01:02:55]

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

[commit-bot: I haz the power, 2017-05-20 01:02:56]

Dry run: This issue passed the CQ dry run.

[gdeepti, 2017-05-21 22:09:51]

The CQ bit was checked by gdeepti@chromium.org

[gdeepti, 2017-05-21 22:09:52]

The patchset sent to the CQ was uploaded after l-g-t-m from bbudge@chromium.org,
mtrofin@chromium.org, zvi.rackover@intel.com
Link to the patchset: https://codereview.chromium.org/2874403002/#ps180001
(title: "Add Todo with bug reference")

[commit-bot: I haz the power, 2017-05-21 22:10:00]

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-05-21 22:40:47]

CQ is committing da patch.

Bot data: {"patchset_id": 180001, "attempt_start_ts": 1495404591632850,
"parent_rev": "12e6f1cb5cd9616da7b9d4a7655c088778a6d415", "commit_rev":
"eeefc74a1189aeed19228f728b60b0f187b5b1bd"}

[commit-bot: I haz the power, 2017-05-21 22:40:56]

Description was changed from

==========
[wasm] Swap the implementation of SIMD compare ops using Gt/Ge insteas of Lt/Le

Currently SIMD integer comparison ops are implemented using Lt/Le, this is
sub-optimal on Intel, because all compares are done using pcmpgt(d/w/b) that
clobber the destination register, and will need additional instructions to
when using Lt/Le as the base implementation. This CL proposes moving to Gt/Ge
as the underlying implementation as this will only require swapping operands
on MIPS and is consistent with x86/ARM instructions.

BUG=v8:6020

R=bbudge@chromium.org, bmeurer@chromium.org, bradnelson@chromium.org
==========

to

==========
[wasm] Swap the implementation of SIMD compare ops using Gt/Ge insteas of Lt/Le

Currently SIMD integer comparison ops are implemented using Lt/Le, this is
sub-optimal on Intel, because all compares are done using pcmpgt(d/w/b) that
clobber the destination register, and will need additional instructions to
when using Lt/Le as the base implementation. This CL proposes moving to Gt/Ge
as the underlying implementation as this will only require swapping operands
on MIPS and is consistent with x86/ARM instructions.

BUG=v8:6020

R=bbudge@chromium.org, bmeurer@chromium.org, bradnelson@chromium.org

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

[commit-bot: I haz the power, 2017-05-21 22:40:58]

Committed patchset #10 (id:180001) as
https://chromium.googlesource.com/v8/v8/+/eeefc74a1189aeed19228f728b60b0f187b...