Add pshufw instruction, fix inconsistencies with pextrw instruction.

Add pshufw instruction, fix inconsistencies with pextrw instruction.

Current implementation of the pextrw instruction is the legacy SSE2 instruction in the assembler (66 0F C5), and SSE4 implementation(66 0F 3A 15) in disasm-x64.cc, this causes incorrect instruction encodings to be printed when using --print-code flag for debug, in this case, causes over flow of bytes, and subsequent instructions to be incorrectly disassembled. Fixing to use SSE4 encodings in the assembler cosistent with pextrb, pextrd.

R=bbudge@chromium.org, mtrofin@chromium.org

Review-Url: https://codereview.chromium.org/2771513002
Cr-Commit-Position: refs/heads/master@{#44047}
Committed: https://chromium.googlesource.com/v8/v8/+/9d8d4dfa7dfdd226f63a58e054082817e8f27dc6

[gdeepti, 2017-03-22 18:31:10]

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

[commit-bot: I haz the power, 2017-03-22 18:31:14]

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

[gdeepti, 2017-03-22 18:52:59]

Description was changed from

==========
Add pshufw instruction, fix inconsistencies with pextrw instruction.

Current implementation of the pextrw instruction is the legacy SSE2 instruction
in the assembler (66 0F C5), and SSE4 implementation(66 0F 3A 15) in
disasm-x64.cc, this causes incorrect instruction encodings to be printed when
using --print-code flag for debug, in this case, causes over flow of bytes, and
subsequent instructions to be incorrectly disassembled. Fixing to use SSE4
encodings in the assembler cosistent with pextrb, pextrd.

R=bbudge@chromium.org, mtrofin@chromium.org
==========

to

==========
Add pshufw instruction, fix inconsistencies with pextrw instruction.

Current implementation of the pextrw instruction is the legacy SSE2 instruction
in the assembler (66 0F C5), and SSE4 implementation(66 0F 3A 15) in
disasm-x64.cc, this causes incorrect instruction encodings to be printed when
using --print-code flag for debug, in this case, causes over flow of bytes, and
subsequent instructions to be incorrectly disassembled. Fixing to use SSE4
encodings in the assembler cosistent with pextrb, pextrd.

R=bbudge@chromium.org, mtrofin@chromium.org
==========

[gdeepti, 2017-03-22 18:53:00]

gdeepti@chromium.org changed reviewers:
+ jing.bao@intel.com

[commit-bot: I haz the power, 2017-03-22 18:55:53]

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

[commit-bot: I haz the power, 2017-03-22 18:55:54]

Dry run: This issue passed the CQ dry run.

[bbudge, 2017-03-22 19:17:05]

LGTM w/comment

https://codereview.chromium.org/2771513002/diff/1/src/x64/assembler-x64.cc
File src/x64/assembler-x64.cc (right):

https://codereview.chromium.org/2771513002/diff/1/src/x64/assembler-x64.cc#ne...
src/x64/assembler-x64.cc:2892: DCHECK(is_uint8(imm8));
If I'm reading the docs correctly, this could be even stronger: DCHECK_GT(8,
imm8);

[gdeepti, 2017-03-22 20:45:34]

https://codereview.chromium.org/2771513002/diff/1/src/x64/assembler-x64.cc
File src/x64/assembler-x64.cc (right):

https://codereview.chromium.org/2771513002/diff/1/src/x64/assembler-x64.cc#ne...
src/x64/assembler-x64.cc:2892: DCHECK(is_uint8(imm8));
On 2017/03/22 19:17:05, bbudge wrote:
> If I'm reading the docs correctly, this could be even stronger: DCHECK_GT(8,
> imm8);

A agree that this check could be stronger, but I'm not sure if the assembler is
the right place for the check as the instructions only specifies that the count
operand is an 8-bit immediate, and only the 3 least-significant bits are used
for location. We already check this in the validate function on decode, will add
a check in the code generator before it comes to the assembler.

[gdeepti, 2017-03-22 20:48:09]

The CQ bit was checked by gdeepti@chromium.org

[commit-bot: I haz the power, 2017-03-22 20:48:14]

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-22 20:49:28]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1490215689921730, "parent_rev":
"8d708c6593ec83a7adf35491db98fb07023bb819", "commit_rev":
"9d8d4dfa7dfdd226f63a58e054082817e8f27dc6"}

[commit-bot: I haz the power, 2017-03-22 20:49:34]

Description was changed from

==========
Add pshufw instruction, fix inconsistencies with pextrw instruction.

Current implementation of the pextrw instruction is the legacy SSE2 instruction
in the assembler (66 0F C5), and SSE4 implementation(66 0F 3A 15) in
disasm-x64.cc, this causes incorrect instruction encodings to be printed when
using --print-code flag for debug, in this case, causes over flow of bytes, and
subsequent instructions to be incorrectly disassembled. Fixing to use SSE4
encodings in the assembler cosistent with pextrb, pextrd.

R=bbudge@chromium.org, mtrofin@chromium.org
==========

to

==========
Add pshufw instruction, fix inconsistencies with pextrw instruction.

Current implementation of the pextrw instruction is the legacy SSE2 instruction
in the assembler (66 0F C5), and SSE4 implementation(66 0F 3A 15) in
disasm-x64.cc, this causes incorrect instruction encodings to be printed when
using --print-code flag for debug, in this case, causes over flow of bytes, and
subsequent instructions to be incorrectly disassembled. Fixing to use SSE4
encodings in the assembler cosistent with pextrb, pextrd.

R=bbudge@chromium.org, mtrofin@chromium.org

Review-Url: https://codereview.chromium.org/2771513002
Cr-Commit-Position: refs/heads/master@{#44047}
Committed:
https://chromium.googlesource.com/v8/v8/+/9d8d4dfa7dfdd226f63a58e054082817e8f...
==========

[commit-bot: I haz the power, 2017-03-22 20:49:35]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/v8/v8/+/9d8d4dfa7dfdd226f63a58e054082817e8f...

[bbudge, 2017-03-22 21:09:40]

https://codereview.chromium.org/2771513002/diff/1/src/x64/assembler-x64.cc
File src/x64/assembler-x64.cc (right):

https://codereview.chromium.org/2771513002/diff/1/src/x64/assembler-x64.cc#ne...
src/x64/assembler-x64.cc:2892: DCHECK(is_uint8(imm8));
On 2017/03/22 20:45:34, gdeepti wrote:
> On 2017/03/22 19:17:05, bbudge wrote:
> > If I'm reading the docs correctly, this could be even stronger: DCHECK_GT(8,
> > imm8);
> 
> A agree that this check could be stronger, but I'm not sure if the assembler
is
> the right place for the check as the instructions only specifies that the
count
> operand is an 8-bit immediate, and only the 3 least-significant bits are used
> for location. We already check this in the validate function on decode, will
add
> a check in the code generator before it comes to the assembler.  

OK