[Intepreter] Addition of BytecodeArrayBuilder and accumulator based bytecodes.

src/interpreter/bytecode-array-builder.cc

+// Copyright 2015 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "src/ast.h"
+#include "src/interpreter/bytecode-array-builder.h"

[rmcilroy, 2015/07/30 10:12:25]

always have header on it's own line first (which means you will need to add
"src/ast.h" to the header) to ensure the header includes all the includes it
needs.

[oth, 2015/07/30 15:38:42]

Done.

+
+namespace v8 {
+namespace internal {
+namespace interpreter {
+
+BytecodeArrayBuilder::BytecodeArrayBuilder(Isolate* isolate)
+    : isolate_(isolate), max_registers_(-1), scratch_register_(-1) {}
+
+
+void BytecodeArrayBuilder::set_stack_slots(int slots) {

[rmcilroy, 2015/07/30 10:12:24]

nit - this would be clearer to me as  this be set_locals_count?

[oth, 2015/07/30 15:38:42]

Done.

+  scratch_register_ = slots;

[rmcilroy, 2015/07/30 10:12:24]

local_register_count_

[oth, 2015/07/30 15:38:42]

Done.

+  max_registers_ = slots;

[rmcilroy, 2015/07/30 10:12:24]

max_temporary_register_count_

[oth, 2015/07/30 15:38:42]

Done.

+}
+
+
+Handle<BytecodeArray> BytecodeArrayBuilder::ToBytecodeArray() const {

[rmcilroy, 2015/07/30 10:12:24]

add a "bytecodes_generated_" bool and DCHECK here that this is not set, before
setting it at the end of the function.

[oth, 2015/07/30 15:38:42]

Done.

+  int bytecode_size = static_cast<int>(bytecodes_.size());
+  int frame_size = max_registers_ * sizeof(intptr_t);
+  return isolate_->factory()->NewBytecodeArray(bytecode_size,
+                                               &bytecodes_.front(), frame_size);
+}
+
+
+void BytecodeArrayBuilder::BinaryOperation(Token::Value binop, int reg) {
+  Output(BytecodeFor(binop), reg);
+}
+
+
+void BytecodeArrayBuilder::LoadLiteral(v8::internal::Smi* smi) {
+  int32_t raw_smi = smi->value();
+  if (raw_smi == 0) {
+    Output(Bytecode::kLoadLiteral0);
+  } else if (raw_smi > -128 && raw_smi <= 128) {
+    Output(Bytecode::kLoadSmi8, static_cast<uint8_t>(raw_smi));
+  } else {
+    // TODO(oth): Put Smi in constant pool.
+    UNIMPLEMENTED();
+  }
+}
+
+
+void BytecodeArrayBuilder::LoadUndefined() { Output(Bytecode::kLdaUndefined); }
+
+
+void BytecodeArrayBuilder::LoadNull() { Output(Bytecode::kLdaNull); }
+
+
+void BytecodeArrayBuilder::LoadTheHole() { Output(Bytecode::kLdaTheHole); }
+
+
+void BytecodeArrayBuilder::LoadTrue() { Output(Bytecode::kLdaTrue); }
+
+
+void BytecodeArrayBuilder::LoadFalse() { Output(Bytecode::kLdaFalse); }
+
+
+void BytecodeArrayBuilder::LoadAccumulatorWithRegister(int reg) {
+  Output(Bytecode::kLdar, reg);
+}
+
+
+void BytecodeArrayBuilder::StoreAccumulatorInRegister(int reg) {
+  Output(Bytecode::kStra, reg);
+}
+
+
+void BytecodeArrayBuilder::Return() { Output(Bytecode::kReturn); }
+
+
+int BytecodeArrayBuilder::AllocateScratchRegister() {

[rmcilroy, 2015/07/30 10:12:24]

These should be Pop/PushScratchRegister if they require that the scratches are
pushed and popped in order. Also could we call them TempRegister instead
(scratch is typically something which is only live for a very short time, where
these might be live for the whole of an expression).

[oth, 2015/07/30 15:38:42]

Done.

[rmcilroy, 2015/07/31 09:56:19]

Missed rename to Push/Pop?

+  int reg = scratch_register_++;

[rmcilroy, 2015/07/30 10:12:24]

nit - add CHECK(locals_register_count_ >= 0)

[oth, 2015/07/30 15:38:42]

Done.

+  if (scratch_register_ > max_registers_) {
+    max_registers_ = scratch_register_;
+  }
+  return reg;
+}
+
+
+void BytecodeArrayBuilder::FreeScratchRegister(int reg) {
+  DCHECK(reg == scratch_register_ - 1);
+  scratch_register_ = reg;
+}
+
+
+void BytecodeArrayBuilder::Output(Bytecode bytecode, uint8_t r0, uint8_t r1,

[rmcilroy, 2015/07/30 10:12:24]

nit - operand0, operand1, etc. (instead of r0, r1...)

[oth, 2015/07/30 15:38:42]

Done.

+                                  uint8_t r2) {
+  CHECK(Bytecodes::NumberOfOperands(bytecode) == 3);
+  bytecodes_.push_back(Bytecodes::ToByte(bytecode));
+  bytecodes_.push_back(r0);
+  bytecodes_.push_back(r1);
+  bytecodes_.push_back(r2);
+}
+
+
+void BytecodeArrayBuilder::Output(Bytecode bytecode, uint8_t r0, uint8_t r1) {
+  CHECK(Bytecodes::NumberOfOperands(bytecode) == 2);
+  bytecodes_.push_back(Bytecodes::ToByte(bytecode));
+  bytecodes_.push_back(r0);
+  bytecodes_.push_back(r1);
+}
+
+
+void BytecodeArrayBuilder::Output(Bytecode bytecode, uint8_t r0) {
+  CHECK(Bytecodes::NumberOfOperands(bytecode) == 1);
+  bytecodes_.push_back(Bytecodes::ToByte(bytecode));
+  bytecodes_.push_back(r0);
+}
+
+
+void BytecodeArrayBuilder::Output(Bytecode bytecode) {
+  CHECK(Bytecodes::NumberOfOperands(bytecode) == 0);
+  bytecodes_.push_back(Bytecodes::ToByte(bytecode));
+}
+
+
+// static
+Bytecode BytecodeArrayBuilder::BytecodeFor(Token::Value op) {

[rmcilroy, 2015/07/30 10:12:24]

Let's make this BytecodeForBinaryOp and remove the ASSIGN case (which I'm not
sure would every be called in the current code?)

[oth, 2015/07/30 15:38:42]

Done.

+  switch (op) {
+    case Token::Value::ASSIGN:
+      return Bytecode::kStra;
+    case Token::Value::ADD:
+      return Bytecode::kAdd;
+    case Token::Value::SUB:
+      return Bytecode::kSub;
+    case Token::Value::MUL:
+      return Bytecode::kMul;
+    case Token::Value::DIV:
+      return Bytecode::kDiv;
+    default:
+      UNIMPLEMENTED();
+      return static_cast<Bytecode>(-1);
+  }
+}
+
+}  // namespace interpreter
+}  // namespace internal
+}  // namespace v8