[Interpreter] Add support for cast operators to bytecode graph builder and

src/interpreter/bytecode-array-builder.cc

Index: src/interpreter/bytecode-array-builder.cc
diff --git a/src/interpreter/bytecode-array-builder.cc b/src/interpreter/bytecode-array-builder.cc
index f7efb103063c921ab10dd8bbea7296065ef63452..594d8faa25e02dec182eae96e94f802eaa01dd58 100644
--- a/src/interpreter/bytecode-array-builder.cc
+++ b/src/interpreter/bytecode-array-builder.cc
@@ -8,6 +8,53 @@ namespace v8 {
 namespace internal {
 namespace interpreter {
 
+class BytecodeArrayBuilder::PreviousBytecodeHelper {
+ public:
+  explicit PreviousBytecodeHelper(const BytecodeArrayBuilder& array_builder)
+      : array_builder_(array_builder) {}
+
+  Bytecode GetBytecode() const {
+    // Returns the previous bytecode in the same basicblock. If there is none it
+    // returns Bytecode::kLast.
+    if (!array_builder_.LastBytecodeInSameBlock()) {
+      return Bytecode::kLast;

[rmcilroy, 2015/12/07 11:56:03]

Bytecode::kLast is the same as the last valid bytecode declared in bytecodes.h -
i.e., Bytecode::kReturn right now. Are you sure you want to return this as a
'none' value? It seems like it could cause bugs or confusion if you want to do a
test against kReturn or if the bytecode declaration order changes such that
kLast is equal to one of the bytecodes being checked below.

+    }
+    return Bytecodes::FromByte(
+        array_builder_.bytecodes()->at(array_builder_.last_bytecode_start_));
+  }
+
+  uint32_t GetOperand(int operand_index) const {
+    Bytecode bytecode = GetBytecode();
+    DCHECK_GE(operand_index, 0);
+    DCHECK_LT(operand_index, Bytecodes::NumberOfOperands(bytecode));
+    size_t operand_offset =
+        array_builder_.last_bytecode_start_ +
+        Bytecodes::GetOperandOffset(bytecode, operand_index);
+    OperandSize size = Bytecodes::GetOperandSize(bytecode, operand_index);
+    switch (size) {
+      default:
+      case OperandSize::kNone:
+        UNREACHABLE();
+      case OperandSize::kByte:
+        return static_cast<uint32_t>(
+            array_builder_.bytecodes()->at(operand_offset));
+      case OperandSize::kShort:
+        uint16_t operand =
+            (array_builder_.bytecodes()->at(operand_offset) << 8) +
+            array_builder_.bytecodes()->at(operand_offset + 1);
+        return static_cast<uint32_t>(operand);
+    }
+  }
+
+  Handle<Object> GetConstantForIndexOperand(int operand_index) const {
+    return array_builder_.constants_.at(GetOperand(operand_index));
+  }
+
+ private:
+  const BytecodeArrayBuilder& array_builder_;
+};
+
+
 BytecodeArrayBuilder::BytecodeArrayBuilder(Isolate* isolate, Zone* zone)
     : isolate_(isolate),
       zone_(zone),
@@ -277,6 +324,16 @@ BytecodeArrayBuilder& BytecodeArrayBuilder::LoadFalse() {
 }
 
 
+BytecodeArrayBuilder& BytecodeArrayBuilder::LoadBooleanConstant(bool value) {
+  if (value) {
+    LoadTrue();
+  } else {
+    LoadFalse();
+  }
+  return *this;
+}
+
+
 BytecodeArrayBuilder& BytecodeArrayBuilder::LoadAccumulatorWithRegister(
     Register reg) {
   if (!IsRegisterInAccumulator(reg)) {
@@ -523,15 +580,11 @@ BytecodeArrayBuilder& BytecodeArrayBuilder::PopContext(Register context) {
 
 
 bool BytecodeArrayBuilder::NeedToBooleanCast() {
-  if (!LastBytecodeInSameBlock()) {
-    // If the previous bytecode was from a different block return false.
-    return true;
-  }
-
-  // If the previous bytecode puts a boolean in the accumulator return true.
-  switch (Bytecodes::FromByte(bytecodes()->at(last_bytecode_start_))) {
+  PreviousBytecodeHelper previous_bytecode(*this);
+  switch (previous_bytecode.GetBytecode()) {
     case Bytecode::kToBoolean:
       UNREACHABLE();
+    // If the previous bytecode puts a boolean in the accumulator return true.
     case Bytecode::kLdaTrue:
     case Bytecode::kLdaFalse:
     case Bytecode::kLogicalNot:
@@ -547,6 +600,8 @@ bool BytecodeArrayBuilder::NeedToBooleanCast() {
     case Bytecode::kTestIn:
     case Bytecode::kForInDone:
       return false;
+    // Also handles the case where the previous bytecode was in a different
+    // block.
     default:
       return true;
   }
@@ -554,10 +609,28 @@ bool BytecodeArrayBuilder::NeedToBooleanCast() {
 
 
 BytecodeArrayBuilder& BytecodeArrayBuilder::CastAccumulatorToBoolean() {
+  PreviousBytecodeHelper previous_bytecode(*this);
   // If the previous bytecode puts a boolean in the accumulator
   // there is no need to emit an instruction.
   if (NeedToBooleanCast()) {
-    Output(Bytecode::kToBoolean);
+    switch (previous_bytecode.GetBytecode()) {

[rmcilroy, 2015/12/07 11:56:03]

Optional nit - It would be nice to have some tests of these cases below in
test-bytecode-generator.

+      // If the previous bytecode is a constant evaluate it and return false.
+      case Bytecode::kLdaZero: {
+        LoadFalse();
+        break;
+      }
+      case Bytecode::kLdaSmi8: {
+        LoadBooleanConstant(previous_bytecode.GetOperand(0) != 0);
+        break;
+      }
+      case Bytecode::kLdaConstant: {
+        Handle<Object> object = previous_bytecode.GetConstantForIndexOperand(0);
+        LoadBooleanConstant(object->BooleanValue());
+        break;
+      }
+      default:
+        Output(Bytecode::kToBoolean);
+    }
   }
   return *this;
 }
@@ -570,6 +643,20 @@ BytecodeArrayBuilder& BytecodeArrayBuilder::CastAccumulatorToJSObject() {
 
 
 BytecodeArrayBuilder& BytecodeArrayBuilder::CastAccumulatorToName() {
+  PreviousBytecodeHelper previous_bytecode(*this);
+  switch (previous_bytecode.GetBytecode()) {
+    case Bytecode::kLdaConstantWide:
+    case Bytecode::kLdaConstant: {
+      Handle<Object> object = previous_bytecode.GetConstantForIndexOperand(0);
+      if (object->IsName()) return *this;
+      break;
+    }
+    case Bytecode::kToName:
+    case Bytecode::kTypeOf:
+      return *this;
+    default:
+      break;
+  }
   Output(Bytecode::kToName);
   return *this;
 }
@@ -1002,14 +1089,10 @@ bool BytecodeArrayBuilder::LastBytecodeInSameBlock() const {
 
 
 bool BytecodeArrayBuilder::IsRegisterInAccumulator(Register reg) {
-  if (!LastBytecodeInSameBlock()) return false;
-  Bytecode previous_bytecode =
-      Bytecodes::FromByte(bytecodes()->at(last_bytecode_start_));
-  if (previous_bytecode == Bytecode::kLdar ||
-      previous_bytecode == Bytecode::kStar) {
-    size_t operand_offset = last_bytecode_start_ +
-                            Bytecodes::GetOperandOffset(previous_bytecode, 0);
-    if (reg == Register::FromOperand(bytecodes()->at(operand_offset))) {
+  PreviousBytecodeHelper previous_bytecode(*this);
+  if (previous_bytecode.GetBytecode() == Bytecode::kLdar ||
+      previous_bytecode.GetBytecode() == Bytecode::kStar) {
+    if (reg == Register::FromOperand(previous_bytecode.GetOperand(0))) {
       return true;
     }
   }