[Interpreter] Add an unsigned immediate operand type

[Interpreter] Add an unsigned immediate operand type

Committed: https://crrev.com/5a9eac3a640f6a610bf2aec2c6a31a2a1d679602
Cr-Commit-Position: refs/heads/master@{#39388}

[Leszek Swirski, 2016-09-13 10:39:30]

leszeks@chromium.org changed reviewers:
+ klaasb@google.com, rmcilroy@chromium.org

[klaasb, 2016-09-13 10:54:40]

lgtm, except for single comment

https://codereview.chromium.org/2336203002/diff/1/src/interpreter/interpreter.cc
File src/interpreter/interpreter.cc (right):

https://codereview.chromium.org/2336203002/diff/1/src/interpreter/interpreter...
src/interpreter/interpreter.cc:2013: Node* slots = __ BytecodeOperandIdx(0);
Please also change to UImm here.

[rmcilroy, 2016-09-13 11:05:20]

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

[commit-bot: I haz the power, 2016-09-13 11:05:23]

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, 2016-09-13 11:09:32]

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

[commit-bot: I haz the power, 2016-09-13 11:09:33]

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

[rmcilroy, 2016-09-13 12:05:57]

Please also update BytecodeArrayIterator, thanks.

https://codereview.chromium.org/2336203002/diff/1/src/interpreter/bytecode-ar...
File src/interpreter/bytecode-array-builder.cc (right):

https://codereview.chromium.org/2336203002/diff/1/src/interpreter/bytecode-ar...
src/interpreter/bytecode-array-builder.cc:785: // TODO(leszeks): Check constant
pool index.
These still aren't always constant pool indexes (they are also used for context
slots and type-feedback-vector slots I think), so we would still need to split
it if we wanted this check.

https://codereview.chromium.org/2336203002/diff/1/src/interpreter/interpreter.cc
File src/interpreter/interpreter.cc (right):

https://codereview.chromium.org/2336203002/diff/1/src/interpreter/interpreter...
src/interpreter/interpreter.cc:2013: Node* slots = __ BytecodeOperandIdx(0);
On 2016/09/13 10:54:40, klaasb wrote:
> Please also change to UImm here.

+1. This should cause a DCHECK in debug mode as-is.

[rmcilroy, 2016-09-13 12:08:54]

On 2016/09/13 12:05:57, rmcilroy wrote:
> Please also update BytecodeArrayIterator, thanks.

and bytecode-expectations-printer.cc please.

[Leszek Swirski, 2016-09-13 14:03:23]

Thanks, all done.

https://codereview.chromium.org/2336203002/diff/1/src/interpreter/bytecode-ar...
File src/interpreter/bytecode-array-builder.cc (right):

https://codereview.chromium.org/2336203002/diff/1/src/interpreter/bytecode-ar...
src/interpreter/bytecode-array-builder.cc:785: // TODO(leszeks): Check constant
pool index.
On 2016/09/13 12:05:57, rmcilroy wrote:
> These still aren't always constant pool indexes (they are also used for
context
> slots and type-feedback-vector slots I think), so we would still need to split
> it if we wanted this check.

Done, updated the todo.

https://codereview.chromium.org/2336203002/diff/1/src/interpreter/interpreter.cc
File src/interpreter/interpreter.cc (right):

https://codereview.chromium.org/2336203002/diff/1/src/interpreter/interpreter...
src/interpreter/interpreter.cc:2013: Node* slots = __ BytecodeOperandIdx(0);
On 2016/09/13 12:05:57, rmcilroy wrote:
> On 2016/09/13 10:54:40, klaasb wrote:
> > Please also change to UImm here.
> 
> +1. This should cause a DCHECK in debug mode as-is.

Done, and I ran the tests in debug this time to check for any others.

[Leszek Swirski, 2016-09-13 14:05:58]

leszeks@chromium.org changed reviewers:
+ mstarzinger@chromium.org

[Leszek Swirski, 2016-09-13 14:05:59]

mstarzinger@chromium.org: Please review changes in

[Michael Starzinger, 2016-09-13 14:13:52]

LGTM (rubber-stamp of "compiler", didn't review the rest).

[Leszek Swirski, 2016-09-13 14:20:22]

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

[commit-bot: I haz the power, 2016-09-13 14:20:32]

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

[rmcilroy, 2016-09-13 14:21:50]

LGTM, thanks.

[Leszek Swirski, 2016-09-13 14:22:35]

The CQ bit was unchecked by leszeks@chromium.org

[Leszek Swirski, 2016-09-13 14:22:35]

The CQ bit was checked by leszeks@chromium.org

[Leszek Swirski, 2016-09-13 14:22:36]

The patchset sent to the CQ was uploaded after l-g-t-m from klaasb@google.com
Link to the patchset: https://codereview.chromium.org/2336203002/#ps20001
(title: "Address comments")

[commit-bot: I haz the power, 2016-09-13 14:22:45]

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, 2016-09-13 14:48:43]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-09-13 14:49:16]

Description was changed from

==========
[Interpreter] Add an unsigned immediate operand type
==========

to

==========
[Interpreter] Add an unsigned immediate operand type

Committed: https://crrev.com/5a9eac3a640f6a610bf2aec2c6a31a2a1d679602
Cr-Commit-Position: refs/heads/master@{#39388}
==========

[commit-bot: I haz the power, 2016-09-13 14:49:17]

Patchset 2 (id:??) landed as
https://crrev.com/5a9eac3a640f6a610bf2aec2c6a31a2a1d679602
Cr-Commit-Position: refs/heads/master@{#39388}