[Interpreter] Add support for short (16 bit) operands.

[Interpreter] Add support for short (16 bit) operands.

Adds support for short operands, starting with kIdx16. Introduces
BytecodeTraits to enable compile time determination of various traits for a
bytecode, such as size, operands, etc. Reworks BytecodeIterator,
BytecodeArrayBuilder and Bytecodes::Decode to support 16 bit operands. Adds
support to Interpreter to load 16 bit operands.

Also fixes a bug with ToBoolean where it wouldn't get emitted at the start
of a block, and added a test.

BytecodeTraits template magic inspired by oth@chromium.org.

BUG=v8:4280
LOG=N

Committed: https://crrev.com/03369ed2cb95d26e174e603152aace966c866077
Cr-Commit-Position: refs/heads/master@{#31058}

[rmcilroy, 2015-09-25 23:57:51]

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

[commit-bot: I haz the power, 2015-09-25 23:58:04]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1370893002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1370893002/1

[commit-bot: I haz the power, 2015-09-25 23:59:04]

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

[commit-bot: I haz the power, 2015-09-25 23:59:05]

Dry run: Try jobs failed on following builders:
  v8_linux64_asan_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel/builds/...)
  v8_linux64_avx2_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel/builds/...)
  v8_linux_arm64_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel/builds/9968)
  v8_linux_nodcheck_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel/build...)
  v8_linux_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_rel/builds/9961)

[rmcilroy, 2015-09-26 00:02:50]

Patchset #1 (id:1) has been deleted

[rmcilroy, 2015-09-26 00:02:55]

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

[commit-bot: I haz the power, 2015-09-26 00:03:11]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1370893002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1370893002/20001

[commit-bot: I haz the power, 2015-09-26 00:07:13]

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

[commit-bot: I haz the power, 2015-09-26 00:07:14]

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

[rmcilroy, 2015-09-26 11:10:26]

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

[rmcilroy, 2015-09-26 11:10:32]

Patchset #1 (id:20001) has been deleted

[commit-bot: I haz the power, 2015-09-26 11:10:34]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1370893002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1370893002/40001

[commit-bot: I haz the power, 2015-09-26 11:14:23]

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

[commit-bot: I haz the power, 2015-09-26 11:14:23]

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

[rmcilroy, 2015-09-28 08:34:02]

rmcilroy@chromium.org changed reviewers:
+ mstarzinger@chromium.org, oth@chromium.org

[rmcilroy, 2015-09-28 08:34:03]

PTAL, thanks. 

This is needed for the CallRuntime support, since the runtime function_id
doesn't fit in a byte, so we will use a WideIdx for it instead (will update that
CL shortly).

[rmcilroy, 2015-09-28 08:34:09]

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

[commit-bot: I haz the power, 2015-09-28 08:34:26]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1370893002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1370893002/60001

[oth, 2015-09-28 09:21:25]

LGTM. Very tidy, good job! Not a fan of the wide operand name (see comments),
but this is a minor quibble.

https://codereview.chromium.org/1370893002/diff/60001/src/interpreter/bytecod...
File src/interpreter/bytecode-array-builder.cc (right):

https://codereview.chromium.org/1370893002/diff/60001/src/interpreter/bytecod...
src/interpreter/bytecode-array-builder.cc:318: 
Nit - empty line.

https://codereview.chromium.org/1370893002/diff/60001/src/interpreter/bytecod...
File src/interpreter/bytecodes.h (right):

https://codereview.chromium.org/1370893002/diff/60001/src/interpreter/bytecod...
src/interpreter/bytecodes.h:31: V(WideIdx, OperandSize::kWide)
Wide is a loose term. It's currently a 16-bit operand, though there's a 32-bit
operand coming after this lands (ExtraWide).

Open question - can we clean up the names in this CL? Suggest putting the width
in the name. Should be fairly mechanical:

/OperandType::Count/OperandType::Count8/g
/OperandType::Idx/OperandType::Idx8/g
/OperandType::Reg/OperandType::Reg8/g
/OperandType::WideIdx/OperandType::Idx16/g

https://codereview.chromium.org/1370893002/diff/60001/src/interpreter/bytecod...
src/interpreter/bytecodes.h:100: kWide = 2,
More descriptive names to avoid wide? kOneByte kTwoByte

[Michael Starzinger, 2015-09-28 09:29:49]

LGTM on the compiler directory. I'll just pretend I didn't see BytecodeTraits
and won't comment on that. :)

[commit-bot: I haz the power, 2015-09-28 09:50:34]

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

[commit-bot: I haz the power, 2015-09-28 09:50:35]

Dry run: This issue passed the CQ dry run.

[rmcilroy, 2015-10-01 14:03:19]

Patchset #3 (id:80001) has been deleted

[rmcilroy, 2015-10-01 14:09:15]

Changes addressed. Also added support for architecture which don't support
unaligned access (e.g. MIPS). PTAL.

https://codereview.chromium.org/1370893002/diff/60001/src/interpreter/bytecod...
File src/interpreter/bytecode-array-builder.cc (right):

https://codereview.chromium.org/1370893002/diff/60001/src/interpreter/bytecod...
src/interpreter/bytecode-array-builder.cc:318: 
On 2015/09/28 09:21:25, oth wrote:
> Nit - empty line.

Done.

https://codereview.chromium.org/1370893002/diff/60001/src/interpreter/bytecod...
File src/interpreter/bytecodes.h (right):

https://codereview.chromium.org/1370893002/diff/60001/src/interpreter/bytecod...
src/interpreter/bytecodes.h:31: V(WideIdx, OperandSize::kWide)
On 2015/09/28 09:21:25, oth wrote:
> Wide is a loose term. It's currently a 16-bit operand, though there's a 32-bit
> operand coming after this lands (ExtraWide).
>
> Open question - can we clean up the names in this CL? Suggest putting the
width
> in the name. Should be fairly mechanical:
> 
> /OperandType::Count/OperandType::Count8/g
> /OperandType::Idx/OperandType::Idx8/g
> /OperandType::Reg/OperandType::Reg8/g
> /OperandType::WideIdx/OperandType::Idx16/g


Renamed OperandSize::kWide to OperandSize::kShort, and changed operand types to
include size as requested.

https://codereview.chromium.org/1370893002/diff/60001/src/interpreter/bytecod...
src/interpreter/bytecodes.h:100: kWide = 2,
On 2015/09/28 09:21:25, oth wrote:
> More descriptive names to avoid wide? kOneByte kTwoByte

Made it kByte, kShort and will be kLong for 32.

[rmcilroy, 2015-10-01 15:01:16]

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

[commit-bot: I haz the power, 2015-10-01 15:02:22]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1370893002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1370893002/120001

[commit-bot: I haz the power, 2015-10-01 15:06:26]

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

[commit-bot: I haz the power, 2015-10-01 15:06:27]

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

[oth, 2015-10-01 15:12:15]

A few minor nits, but lgtm.

https://codereview.chromium.org/1370893002/diff/100001/src/compiler/interpret...
File src/compiler/interpreter-assembler.cc (right):

https://codereview.chromium.org/1370893002/diff/100001/src/compiler/interpret...
src/compiler/interpreter-assembler.cc:129:
DCHECK(interpreter::OperandSize::kByte ==
DCHECK_EQ

https://codereview.chromium.org/1370893002/diff/100001/src/compiler/interpret...
src/compiler/interpreter-assembler.cc:158:
DCHECK(interpreter::OperandSize::kShort ==
DCHECK_EQ

https://codereview.chromium.org/1370893002/diff/100001/src/interpreter/byteco...
File src/interpreter/bytecodes.h (right):

https://codereview.chromium.org/1370893002/diff/100001/src/interpreter/byteco...
src/interpreter/bytecodes.h:123: // The Count_BYTECODE macro will turn this into
kLast = -1 +1 +1... which will
Intentional?

https://codereview.chromium.org/1370893002/diff/100001/src/interpreter/byteco...
src/interpreter/bytecodes.h:197: // Returns the size of operand.
|operand|

https://codereview.chromium.org/1370893002/diff/100001/test/unittests/interpr...
File test/unittests/interpreter/bytecode-array-iterator-unittest.cc (right):

https://codereview.chromium.org/1370893002/diff/100001/test/unittests/interpr...
test/unittests/interpreter/bytecode-array-iterator-unittest.cc:40: //
TODO(rmcilroy): Add a test for a bytecode with a wide operand when
s/wide/short/

[rmcilroy, 2015-10-01 16:25:07]

https://codereview.chromium.org/1370893002/diff/100001/src/compiler/interpret...
File src/compiler/interpreter-assembler.cc (right):

https://codereview.chromium.org/1370893002/diff/100001/src/compiler/interpret...
src/compiler/interpreter-assembler.cc:129:
DCHECK(interpreter::OperandSize::kByte ==
On 2015/10/01 15:12:15, oth wrote:
> DCHECK_EQ

Done.

https://codereview.chromium.org/1370893002/diff/100001/src/compiler/interpret...
src/compiler/interpreter-assembler.cc:158:
DCHECK(interpreter::OperandSize::kShort ==
On 2015/10/01 15:12:14, oth wrote:
> DCHECK_EQ

Done.

https://codereview.chromium.org/1370893002/diff/100001/src/interpreter/byteco...
File src/interpreter/bytecodes.h (right):

https://codereview.chromium.org/1370893002/diff/100001/src/interpreter/byteco...
src/interpreter/bytecodes.h:123: // The Count_BYTECODE macro will turn this into
kLast = -1 +1 +1... which will
On 2015/10/01 15:12:15, oth wrote:
> Intentional?

Nope, thanks! Done.

https://codereview.chromium.org/1370893002/diff/100001/src/interpreter/byteco...
src/interpreter/bytecodes.h:197: // Returns the size of operand.
On 2015/10/01 15:12:15, oth wrote:
> |operand|

Done.

https://codereview.chromium.org/1370893002/diff/100001/test/unittests/interpr...
File test/unittests/interpreter/bytecode-array-iterator-unittest.cc (right):

https://codereview.chromium.org/1370893002/diff/100001/test/unittests/interpr...
test/unittests/interpreter/bytecode-array-iterator-unittest.cc:40: //
TODO(rmcilroy): Add a test for a bytecode with a wide operand when
On 2015/10/01 15:12:15, oth wrote:
> s/wide/short/

Done.

[rmcilroy, 2015-10-01 16:25:15]

The CQ bit was checked by rmcilroy@chromium.org

[rmcilroy, 2015-10-01 16:25:16]

The patchset sent to the CQ was uploaded after l-g-t-m from
mstarzinger@chromium.org, oth@chromium.org
Link to the patchset: https://codereview.chromium.org/1370893002/#ps160001
(title: "Rebase")

[commit-bot: I haz the power, 2015-10-01 16:25:30]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1370893002/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1370893002/160001

[commit-bot: I haz the power, 2015-10-01 17:23:02]

Committed patchset #6 (id:160001)

[commit-bot: I haz the power, 2015-10-01 17:23:26]

Patchset 6 (id:??) landed as
https://crrev.com/03369ed2cb95d26e174e603152aace966c866077
Cr-Commit-Position: refs/heads/master@{#31058}