[Interpreter] Merges ToBoolean and JumpIfTrue/False 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/interpreter/bytecode-array-builder.h"
 
 namespace v8 {
 namespace internal {
 namespace interpreter {
 
@@ skipping 432 matching lines @@
   return *this;
 }
 
 
 BytecodeArrayBuilder& BytecodeArrayBuilder::PopContext(Register context) {
   Output(Bytecode::kPopContext, context.ToOperand());
   return *this;
 }
 
 
-BytecodeArrayBuilder& BytecodeArrayBuilder::CastAccumulatorToBoolean() {
+bool BytecodeArrayBuilder::IsAccumulatorBoolean() {
   if (LastBytecodeInSameBlock()) {
-    // If the previous bytecode puts a boolean in the accumulator
-    // there is no need to emit an instruction.
+    // If the previous bytecode puts a boolean in
+    // the accumulator return true.
     switch (Bytecodes::FromByte(bytecodes()->at(last_bytecode_start_))) {
       case Bytecode::kToBoolean:
         UNREACHABLE();
       case Bytecode::kLdaTrue:
       case Bytecode::kLdaFalse:
       case Bytecode::kLogicalNot:
       case Bytecode::kTestEqual:
       case Bytecode::kTestNotEqual:
       case Bytecode::kTestEqualStrict:
       case Bytecode::kTestNotEqualStrict:
       case Bytecode::kTestLessThan:
       case Bytecode::kTestLessThanOrEqual:
       case Bytecode::kTestGreaterThan:
       case Bytecode::kTestGreaterThanOrEqual:
       case Bytecode::kTestInstanceOf:
       case Bytecode::kTestIn:
-        return *this;
+        return true;
       default:
-        // Fall through to output kToBoolean.
-        break;
+        return false;
     }
   }
-  Output(Bytecode::kToBoolean);
+  // If the previous bytecode was from a different block return false.
+  return false;

[rmcilroy, 2015/10/29 10:40:24]

nit - do this as an early out at the to, i.e.: 
if (LastBytecodeInSameBlock()) {
  // If the previous bytecode was from a different block return false.
  return false;
}

switch....

[mythria, 2015/10/29 17:58:39]

Done.

+}
+
+
+BytecodeArrayBuilder& BytecodeArrayBuilder::CastAccumulatorToBoolean() {
+  // If the previous bytecode puts a boolean in the accumulator
+  // there is no need to emit an instruction.
+  if (!IsAccumulatorBoolean()) {
+    Output(Bytecode::kToBoolean);
+  }
   return *this;
 }
 
 
 BytecodeArrayBuilder& BytecodeArrayBuilder::CastAccumulatorToName() {
   Output(Bytecode::kToName);
   return *this;
 }
 
 
@@ skipping 76 matching lines @@
       // to match the size of the code it's replacing. In future,
       // there will probably be a jump with 32-bit operand for cases
       // when constant pool is full, but that needs to be emitted in
       // OutputJump too.
       UNIMPLEMENTED();
     }
   }
 }
 
 
+// static
+Bytecode BytecodeArrayBuilder::GetJumpWithToBoolean(Bytecode jump_bytecode,
+                                                    bool is_cast_required) {
+  if (is_cast_required) {
+    switch (jump_bytecode) {
+      case Bytecode::kJump:
+        return Bytecode::kJump;
+      case Bytecode::kJumpIfTrue:
+        return Bytecode::kJumpIfToBooleanTrue;
+      case Bytecode::kJumpIfFalse:
+        return Bytecode::kJumpIfToBooleanFalse;
+      default:
+        UNREACHABLE();
+    }
+  }
+  return jump_bytecode;
+}
+
+
 BytecodeArrayBuilder& BytecodeArrayBuilder::OutputJump(Bytecode jump_bytecode,
                                                        BytecodeLabel* label) {
+  // Check if the value in accumulator is boolean, if not choose an
+  // appropriate JumpIfToBoolean bytecode.
+  bool is_cast_required = !IsAccumulatorBoolean();
+  Bytecode jump_boolean_bytecode =
+      GetJumpWithToBoolean(jump_bytecode, is_cast_required);

[rmcilroy, 2015/10/29 10:40:24]

Avoid the need for the is_cast_required argument and extra variable by just
doing:

if (!IsAccumulatorBoolean()) {
  jump_bytecode = GetJumpWithToBoolean(jump_bytecode);
}

[mythria, 2015/10/29 17:58:39]

Done.

+
   int delta;
   if (label->is_bound()) {
     // Label has been bound already so this is a backwards jump.
     CHECK_GE(bytecodes()->size(), label->offset());
     CHECK_LE(bytecodes()->size(), static_cast<size_t>(kMaxInt));
     size_t abs_delta = bytecodes()->size() - label->offset();
     delta = -static_cast<int>(abs_delta);
   } else {
     // Label has not yet been bound so this is a forward reference
     // that will be patched when the label is bound.
     label->set_referrer(bytecodes()->size());
     delta = 0;
   }
 
   if (FitsInImm8Operand(delta)) {
-    Output(jump_bytecode, static_cast<uint8_t>(delta));
+    Output(jump_boolean_bytecode, static_cast<uint8_t>(delta));
   } else {
     size_t entry = GetConstantPoolEntry(handle(Smi::FromInt(delta), isolate()));
     if (FitsInIdx8Operand(entry)) {
-      Output(GetJumpWithConstantOperand(jump_bytecode),
+      Output(GetJumpWithConstantOperand(jump_boolean_bytecode),
              static_cast<uint8_t>(entry));
     } else {
       UNIMPLEMENTED();
     }
   }
   return *this;
 }
 
 
 BytecodeArrayBuilder& BytecodeArrayBuilder::Jump(BytecodeLabel* label) {
   return OutputJump(Bytecode::kJump, label);
 }
 
 
 BytecodeArrayBuilder& BytecodeArrayBuilder::JumpIfTrue(BytecodeLabel* label) {
   return OutputJump(Bytecode::kJumpIfTrue, label);
 }
 
 
 BytecodeArrayBuilder& BytecodeArrayBuilder::JumpIfFalse(BytecodeLabel* label) {
   return OutputJump(Bytecode::kJumpIfFalse, label);
 }
 
 
-BytecodeArrayBuilder& BytecodeArrayBuilder::JumpIfToBooleanTrue(
-    BytecodeLabel* label) {
-  return OutputJump(Bytecode::kJumpIfToBooleanTrue, label);
-}
-
-
-BytecodeArrayBuilder& BytecodeArrayBuilder::JumpIfToBooleanFalse(
-    BytecodeLabel* label) {
-  return OutputJump(Bytecode::kJumpIfToBooleanFalse, label);
-}
-
-
 BytecodeArrayBuilder& BytecodeArrayBuilder::Throw() {
   Output(Bytecode::kThrow);
   exit_seen_in_block_ = true;
   return *this;
 }
 
 
 BytecodeArrayBuilder& BytecodeArrayBuilder::Return() {
   Output(Bytecode::kReturn);
   exit_seen_in_block_ = true;
@@ skipping 463 matching lines @@
   DCHECK_GT(next_consecutive_count_, 0);
   builder_->BorrowConsecutiveTemporaryRegister(next_consecutive_register_);
   allocated_.push_back(next_consecutive_register_);
   next_consecutive_count_--;
   return Register(next_consecutive_register_++);
 }
 
 }  // namespace interpreter
 }  // namespace internal
 }  // namespace v8