[Interpreter] Add option to trace bytecode execution.

[Interpreter] Add option to trace bytecode execution.

Adds --trace-ignition flag which allows tracing of bytecodes as they
execute. As well as printing out the bytecode, this also prints out the
input and output registers to each operation. The generated output looks
as follows:

 -> 0x350cb46d5264 (139) : 49 fc fb 03 07    Call r4, r5, #3, [7]
      [ accumulator -> 0x177fba00bc99 <JS Array[2]> ]
      [          r4 -> 0x350cb46ce099 <JS Function InstallFunctions (SharedFunctionInfo 0x350cb46470c1)> ]
      [          r5 -> 0x350cb46cddc1 <an Object with map 0x35fdf590a3a9> ]
      [          r6 -> 0x350cb46d3f11 <JS Function Proxy (SharedFunctionInfo 0x350cb46d3e61)> ]
      [          r7 -> 2 ]
      [ accumulator <- 0x350cb4604189 <undefined> ]
 -> 0x350cb46d5978 (47) : 4b f8 00 00 00    CallRuntime [248], r0, #0
      [ accumulator -> 0x350cb4604189 <undefined> ]
      [ accumulator <- 0x350cb4604189 <undefined> ]
 -> 0x350cb46d597d (52) : 23 09             Ldar a0
      [ accumulator -> 0x350cb4604189 <undefined> ]
      [          a0 -> 0x350cb46d3f11 <JS Function Proxy (SharedFunctionInfo 0x350cb46d3e61)> ]
      [ accumulator <- 0x350cb46d3f11 <JS Function Proxy (SharedFunctionInfo 0x350cb46d3e61)> ]
 -> 0x350cb46d597f (54) : 24 fd             Star r3
      [ accumulator -> 0x350cb46d3f11 <JS Function Proxy (SharedFunctionInfo 0x350cb46d3e61)> ]
      [ accumulator <- 0x350cb46d3f11 <JS Function Proxy (SharedFunctionInfo 0x350cb46d3e61)> ]
      [          r3 <- 0x350cb46d3f11 <JS Function Proxy (SharedFunctionInfo 0x350cb46d3e61)> ]

Also adds support for --print_source and --print-ast to the interpreter.

BUG=v8:4280
LOG=N

Committed: https://crrev.com/6399fce56b534cdda130e50858d95d07d78c3754
Cr-Commit-Position: refs/heads/master@{#33594}

[rmcilroy, 2016-01-27 12:15:51]

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

[commit-bot: I haz the power, 2016-01-27 12:15:54]

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

[rmcilroy, 2016-01-27 12:17:11]

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

[rmcilroy, 2016-01-27 12:17:12]

Orion: Please review at the whole CL.
Michi: Please review compiler changes.

[commit-bot: I haz the power, 2016-01-27 12:28:43]

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

[commit-bot: I haz the power, 2016-01-27 12:28:43]

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

[rmcilroy, 2016-01-27 13:24:43]

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

[commit-bot: I haz the power, 2016-01-27 13:24:54]

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

[commit-bot: I haz the power, 2016-01-27 14:02:35]

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

[commit-bot: I haz the power, 2016-01-27 14:02:36]

Dry run: This issue passed the CQ dry run.

[oth, 2016-01-27 15:10:09]

Very nice. lgtm, thanks.

https://codereview.chromium.org/1640213002/diff/20001/src/interpreter/bytecod...
File src/interpreter/bytecode-array-iterator.cc (right):

https://codereview.chromium.org/1640213002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-array-iterator.cc:119: OperandType next_operand_type =
Can you add a TODO(oth) here to update bytecodes so that the kCount8/kCount16
operands have the same semantics for all bytecodes?
ie Call/CallJSRuntime the count argument is for registers 1 after the receiver,
for CallRuntime/New the count argument covers the last register operand.

https://codereview.chromium.org/1640213002/diff/20001/src/runtime/runtime-int...
File src/runtime/runtime-interpreter.cc (right):

https://codereview.chromium.org/1640213002/diff/20001/src/runtime/runtime-int...
src/runtime/runtime-interpreter.cc:160: static const char* kInputColourCode =
"\033[0;36m";
Suggest defaulting this to the empty string on Windows or calling a function to
change the color (on Windows it'd be SetConsoleTextAttribute).

The color coding here is terminal specific. Probably works on most modern unixen
and not suggesting adding term probing.

https://codereview.chromium.org/1640213002/diff/20001/src/runtime/runtime-int...
src/runtime/runtime-interpreter.cc:185: operand_index <
interpreter::Bytecodes::NumberOfOperands(bytecode);
operand_index < operand_count might be more readable. The function is a bit of a
wall of characters to the reader.

https://codereview.chromium.org/1640213002/diff/20001/src/runtime/runtime-int...
src/runtime/runtime-interpreter.cc:199: Address reg_location = register_file -
(reg_index * kPointerSize);
The precedence here should be clear without the parens (reg_index * ...)

[Michael Starzinger, 2016-01-28 12:07:06]

LGTM. Just nits.

https://codereview.chromium.org/1640213002/diff/40001/src/interpreter/interpr...
File src/interpreter/interpreter.cc (right):

https://codereview.chromium.org/1640213002/diff/40001/src/interpreter/interpr...
src/interpreter/interpreter.cc:108: // Regenerate table to add bytecode tracing
operations.
Well played, well played, I like it! :)

https://codereview.chromium.org/1640213002/diff/40001/src/interpreter/interpr...
File src/interpreter/interpreter.h (right):

https://codereview.chromium.org/1640213002/diff/40001/src/interpreter/interpr...
src/interpreter/interpreter.h:120: void
DoTraceBytecodeExit(compiler::InterpreterAssembler* assembler);
nit: Doesn't seem to be used nor implemented. Looks like leftover.

https://codereview.chromium.org/1640213002/diff/40001/src/runtime/runtime-int...
File src/runtime/runtime-interpreter.cc (right):

https://codereview.chromium.org/1640213002/diff/40001/src/runtime/runtime-int...
src/runtime/runtime-interpreter.cc:7: #include "src/runtime/runtime-utils.h"
nit: Let's keep the runtime-utils.h include first.

https://codereview.chromium.org/1640213002/diff/40001/src/runtime/runtime-int...
src/runtime/runtime-interpreter.cc:11: #include "src/frames.h"
nit: For us "frames-inl.h" satisfies "frames.h", only include the former.

https://codereview.chromium.org/1640213002/diff/40001/src/runtime/runtime-int...
src/runtime/runtime-interpreter.cc:156: static void PrintRegisters(std::ostream&
os, bool is_input,
nit: Can we put this into an anonymous namespace instead of making it static?

[Michael Starzinger, 2016-01-28 12:32:26]

https://codereview.chromium.org/1640213002/diff/40001/src/compiler/interprete...
File src/compiler/interpreter-assembler.h (right):

https://codereview.chromium.org/1640213002/diff/40001/src/compiler/interprete...
src/compiler/interpreter-assembler.h:181: void CallEpilogue();
nit: Also the declaration of CallEpilogue can be dropped and the comment above
adapted now.

[Michael Starzinger, 2016-01-28 15:36:21]

https://codereview.chromium.org/1640213002/diff/40001/src/compiler/interprete...
File src/compiler/interpreter-assembler.h (right):

https://codereview.chromium.org/1640213002/diff/40001/src/compiler/interprete...
src/compiler/interpreter-assembler.h:217: Node* bytecode_offset_;
nit: Also, the {bytecode_offset} field can now be fully removed and we can
change InterpreterAssembler::BytecodeOffset to just always return the parameter
holding said value.

[rmcilroy, 2016-01-28 16:39:38]

Patchset #4 (id:60001) has been deleted

[rmcilroy, 2016-01-28 16:39:51]

https://codereview.chromium.org/1640213002/diff/20001/src/interpreter/bytecod...
File src/interpreter/bytecode-array-iterator.cc (right):

https://codereview.chromium.org/1640213002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-array-iterator.cc:119: OperandType next_operand_type =
On 2016/01/27 15:10:09, oth wrote:
> Can you add a TODO(oth) here to update bytecodes so that the kCount8/kCount16
> operands have the same semantics for all bytecodes?
> ie Call/CallJSRuntime the count argument is for registers 1 after the
receiver,
> for CallRuntime/New the count argument covers the last register operand.

Done.

https://codereview.chromium.org/1640213002/diff/20001/src/runtime/runtime-int...
File src/runtime/runtime-interpreter.cc (right):

https://codereview.chromium.org/1640213002/diff/20001/src/runtime/runtime-int...
src/runtime/runtime-interpreter.cc:160: static const char* kInputColourCode =
"\033[0;36m";
On 2016/01/27 15:10:09, oth wrote:
> Suggest defaulting this to the empty string on Windows or calling a function
to
> change the color (on Windows it'd be SetConsoleTextAttribute).
> 
> The color coding here is terminal specific. Probably works on most modern
unixen
> and not suggesting adding term probing.  

Done by setting the default for FLAG_log_colour based on whether it is windows
or not.

https://codereview.chromium.org/1640213002/diff/20001/src/runtime/runtime-int...
src/runtime/runtime-interpreter.cc:185: operand_index <
interpreter::Bytecodes::NumberOfOperands(bytecode);
On 2016/01/27 15:10:09, oth wrote:
> operand_index < operand_count might be more readable. The function is a bit of
a
> wall of characters to the reader.

Done.

https://codereview.chromium.org/1640213002/diff/20001/src/runtime/runtime-int...
src/runtime/runtime-interpreter.cc:199: Address reg_location = register_file -
(reg_index * kPointerSize);
On 2016/01/27 15:10:09, oth wrote:
> The precedence here should be clear without the parens (reg_index * ...)

Done.

https://codereview.chromium.org/1640213002/diff/40001/src/compiler/interprete...
File src/compiler/interpreter-assembler.h (right):

https://codereview.chromium.org/1640213002/diff/40001/src/compiler/interprete...
src/compiler/interpreter-assembler.h:181: void CallEpilogue();
On 2016/01/28 12:32:26, Michael Starzinger wrote:
> nit: Also the declaration of CallEpilogue can be dropped and the comment above
> adapted now.

Done.

https://codereview.chromium.org/1640213002/diff/40001/src/compiler/interprete...
src/compiler/interpreter-assembler.h:217: Node* bytecode_offset_;
On 2016/01/28 15:36:21, Michael Starzinger wrote:
> nit: Also, the {bytecode_offset} field can now be fully removed and we can
> change InterpreterAssembler::BytecodeOffset to just always return the
parameter
> holding said value.

Done.

https://codereview.chromium.org/1640213002/diff/40001/src/interpreter/interpr...
File src/interpreter/interpreter.cc (right):

https://codereview.chromium.org/1640213002/diff/40001/src/interpreter/interpr...
src/interpreter/interpreter.cc:108: // Regenerate table to add bytecode tracing
operations.
On 2016/01/28 12:07:06, Michael Starzinger wrote:
> Well played, well played, I like it! :)

Acknowledged.

https://codereview.chromium.org/1640213002/diff/40001/src/interpreter/interpr...
File src/interpreter/interpreter.h (right):

https://codereview.chromium.org/1640213002/diff/40001/src/interpreter/interpr...
src/interpreter/interpreter.h:120: void
DoTraceBytecodeExit(compiler::InterpreterAssembler* assembler);
On 2016/01/28 12:07:06, Michael Starzinger wrote:
> nit: Doesn't seem to be used nor implemented. Looks like leftover.

Yup, leftover, thanks. Done.

https://codereview.chromium.org/1640213002/diff/40001/src/runtime/runtime-int...
File src/runtime/runtime-interpreter.cc (right):

https://codereview.chromium.org/1640213002/diff/40001/src/runtime/runtime-int...
src/runtime/runtime-interpreter.cc:7: #include "src/runtime/runtime-utils.h"
On 2016/01/28 12:07:06, Michael Starzinger wrote:
> nit: Let's keep the runtime-utils.h include first.

Done.

https://codereview.chromium.org/1640213002/diff/40001/src/runtime/runtime-int...
src/runtime/runtime-interpreter.cc:11: #include "src/frames.h"
On 2016/01/28 12:07:06, Michael Starzinger wrote:
> nit: For us "frames-inl.h" satisfies "frames.h", only include the former.

Done.

https://codereview.chromium.org/1640213002/diff/40001/src/runtime/runtime-int...
src/runtime/runtime-interpreter.cc:156: static void PrintRegisters(std::ostream&
os, bool is_input,
On 2016/01/28 12:07:06, Michael Starzinger wrote:
> nit: Can we put this into an anonymous namespace instead of making it static?

Done.

[rmcilroy, 2016-01-28 16:40:26]

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

[commit-bot: I haz the power, 2016-01-28 16:40:35]

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

[commit-bot: I haz the power, 2016-01-28 17:04:42]

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

[commit-bot: I haz the power, 2016-01-28 17:04:42]

Dry run: This issue passed the CQ dry run.

[rmcilroy, 2016-01-28 18:15:51]

The CQ bit was checked by rmcilroy@chromium.org

[rmcilroy, 2016-01-28 18:15:52]

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

[commit-bot: I haz the power, 2016-01-28 18:15:57]

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

[commit-bot: I haz the power, 2016-01-28 18:17:43]

Committed patchset #4 (id:80001)

[commit-bot: I haz the power, 2016-01-28 18:18:22]

Description was changed from

==========
[Interpreter] Add option to trace bytecode execution.

Adds --trace-ignition flag which allows tracing of bytecodes as they
execute. As well as printing out the bytecode, this also prints out the
input and output registers to each operation. The generated output looks
as follows:

 -> 0x350cb46d5264 (139) : 49 fc fb 03 07    Call r4, r5, #3, [7]
      [ accumulator -> 0x177fba00bc99 <JS Array[2]> ]
      [          r4 -> 0x350cb46ce099 <JS Function InstallFunctions
(SharedFunctionInfo 0x350cb46470c1)> ]
      [          r5 -> 0x350cb46cddc1 <an Object with map 0x35fdf590a3a9> ]
      [          r6 -> 0x350cb46d3f11 <JS Function Proxy (SharedFunctionInfo
0x350cb46d3e61)> ]
      [          r7 -> 2 ]
      [ accumulator <- 0x350cb4604189 <undefined> ]
 -> 0x350cb46d5978 (47) : 4b f8 00 00 00    CallRuntime [248], r0, #0
      [ accumulator -> 0x350cb4604189 <undefined> ]
      [ accumulator <- 0x350cb4604189 <undefined> ]
 -> 0x350cb46d597d (52) : 23 09             Ldar a0
      [ accumulator -> 0x350cb4604189 <undefined> ]
      [          a0 -> 0x350cb46d3f11 <JS Function Proxy (SharedFunctionInfo
0x350cb46d3e61)> ]
      [ accumulator <- 0x350cb46d3f11 <JS Function Proxy (SharedFunctionInfo
0x350cb46d3e61)> ]
 -> 0x350cb46d597f (54) : 24 fd             Star r3
      [ accumulator -> 0x350cb46d3f11 <JS Function Proxy (SharedFunctionInfo
0x350cb46d3e61)> ]
      [ accumulator <- 0x350cb46d3f11 <JS Function Proxy (SharedFunctionInfo
0x350cb46d3e61)> ]
      [          r3 <- 0x350cb46d3f11 <JS Function Proxy (SharedFunctionInfo
0x350cb46d3e61)> ]

Also adds support for --print_source and --print-ast to the interpreter.

BUG=v8:4280
LOG=N
==========

to

==========
[Interpreter] Add option to trace bytecode execution.

Adds --trace-ignition flag which allows tracing of bytecodes as they
execute. As well as printing out the bytecode, this also prints out the
input and output registers to each operation. The generated output looks
as follows:

 -> 0x350cb46d5264 (139) : 49 fc fb 03 07    Call r4, r5, #3, [7]
      [ accumulator -> 0x177fba00bc99 <JS Array[2]> ]
      [          r4 -> 0x350cb46ce099 <JS Function InstallFunctions
(SharedFunctionInfo 0x350cb46470c1)> ]
      [          r5 -> 0x350cb46cddc1 <an Object with map 0x35fdf590a3a9> ]
      [          r6 -> 0x350cb46d3f11 <JS Function Proxy (SharedFunctionInfo
0x350cb46d3e61)> ]
      [          r7 -> 2 ]
      [ accumulator <- 0x350cb4604189 <undefined> ]
 -> 0x350cb46d5978 (47) : 4b f8 00 00 00    CallRuntime [248], r0, #0
      [ accumulator -> 0x350cb4604189 <undefined> ]
      [ accumulator <- 0x350cb4604189 <undefined> ]
 -> 0x350cb46d597d (52) : 23 09             Ldar a0
      [ accumulator -> 0x350cb4604189 <undefined> ]
      [          a0 -> 0x350cb46d3f11 <JS Function Proxy (SharedFunctionInfo
0x350cb46d3e61)> ]
      [ accumulator <- 0x350cb46d3f11 <JS Function Proxy (SharedFunctionInfo
0x350cb46d3e61)> ]
 -> 0x350cb46d597f (54) : 24 fd             Star r3
      [ accumulator -> 0x350cb46d3f11 <JS Function Proxy (SharedFunctionInfo
0x350cb46d3e61)> ]
      [ accumulator <- 0x350cb46d3f11 <JS Function Proxy (SharedFunctionInfo
0x350cb46d3e61)> ]
      [          r3 <- 0x350cb46d3f11 <JS Function Proxy (SharedFunctionInfo
0x350cb46d3e61)> ]

Also adds support for --print_source and --print-ast to the interpreter.

BUG=v8:4280
LOG=N

Committed: https://crrev.com/6399fce56b534cdda130e50858d95d07d78c3754
Cr-Commit-Position: refs/heads/master@{#33594}
==========

[commit-bot: I haz the power, 2016-01-28 18:18:23]

Patchset 4 (id:??) landed as
https://crrev.com/6399fce56b534cdda130e50858d95d07d78c3754
Cr-Commit-Position: refs/heads/master@{#33594}