[Interpreter] Merges ToBoolean and JumpIfTrue/False bytecodes

Adds an optimization to emit JumpIfToBooleanTrue/False instead
of ToBoolean followed by JumpIfTrue/False if the value in the
accumulator is not boolean.

BUG=v8:4280
LOG=N

Review URL: https://codereview.chromium.org/1426913002

Cr-Commit-Position: refs/heads/master@{#31697}

src/interpreter/bytecode-array-builder.cc

@@ -480,33 +480,43 @@ 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_))) {
+    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:
+    case Bytecode::kForInDone:
+      return false;
+    default:
+      return true;
+  }
+}
+
+
 BytecodeArrayBuilder& BytecodeArrayBuilder::CastAccumulatorToBoolean() {
-  if (LastBytecodeInSameBlock()) {
-    // If the previous bytecode puts a boolean in the accumulator
-    // there is no need to emit an instruction.
-    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;
-      default:
-        // Fall through to output kToBoolean.
-        break;
-    }
+  // If the previous bytecode puts a boolean in the accumulator
+  // there is no need to emit an instruction.
+  if (NeedToBooleanCast()) {
+    Output(Bytecode::kToBoolean);
   }
-  Output(Bytecode::kToBoolean);
   return *this;
 }
 
@@ -613,8 +623,32 @@ void BytecodeArrayBuilder::PatchJump(
 }
 
 
+// static
+Bytecode BytecodeArrayBuilder::GetJumpWithToBoolean(Bytecode jump_bytecode) {
+  switch (jump_bytecode) {
+    case Bytecode::kJump:
+    case Bytecode::kJumpIfNull:
+    case Bytecode::kJumpIfUndefined:
+      return jump_bytecode;
+    case Bytecode::kJumpIfTrue:
+      return Bytecode::kJumpIfToBooleanTrue;
+    case Bytecode::kJumpIfFalse:
+      return Bytecode::kJumpIfToBooleanFalse;
+    default:
+      UNREACHABLE();
+  }
+  return static_cast<Bytecode>(-1);
+}
+
+
 BytecodeArrayBuilder& BytecodeArrayBuilder::OutputJump(Bytecode jump_bytecode,
                                                        BytecodeLabel* label) {
+  // Check if the value in accumulator is boolean, if not choose an
+  // appropriate JumpIfToBoolean bytecode.
+  if (NeedToBooleanCast()) {
+    jump_bytecode = GetJumpWithToBoolean(jump_bytecode);
+  }
+
   int delta;
   if (label->is_bound()) {
     // Label has been bound already so this is a backwards jump.
@@ -659,18 +693,6 @@ BytecodeArrayBuilder& BytecodeArrayBuilder::JumpIfFalse(BytecodeLabel* label) {
 }
 
 
-BytecodeArrayBuilder& BytecodeArrayBuilder::JumpIfToBooleanTrue(
-    BytecodeLabel* label) {
-  return OutputJump(Bytecode::kJumpIfToBooleanTrue, label);
-}
-
-
-BytecodeArrayBuilder& BytecodeArrayBuilder::JumpIfToBooleanFalse(
-    BytecodeLabel* label) {
-  return OutputJump(Bytecode::kJumpIfToBooleanFalse, label);
-}
-
-
 BytecodeArrayBuilder& BytecodeArrayBuilder::JumpIfNull(BytecodeLabel* label) {
   return OutputJump(Bytecode::kJumpIfNull, label);
 }

src/interpreter/bytecode-array-builder.h

@@ -183,11 +183,6 @@ class BytecodeArrayBuilder {
   BytecodeArrayBuilder& JumpIfFalse(BytecodeLabel* label);
   BytecodeArrayBuilder& JumpIfNull(BytecodeLabel* label);
   BytecodeArrayBuilder& JumpIfUndefined(BytecodeLabel* label);
-  // TODO(mythria) The following two functions should be merged into
-  // JumpIfTrue/False. These bytecodes should be automatically chosen rather
-  // than explicitly using them.
-  BytecodeArrayBuilder& JumpIfToBooleanTrue(BytecodeLabel* label);
-  BytecodeArrayBuilder& JumpIfToBooleanFalse(BytecodeLabel* label);
 
   BytecodeArrayBuilder& Throw();
   BytecodeArrayBuilder& Return();
@@ -228,6 +223,7 @@ class BytecodeArrayBuilder {
   static bool FitsInIdx16Operand(size_t value);
 
   static Bytecode GetJumpWithConstantOperand(Bytecode jump_with_smi8_operand);
+  static Bytecode GetJumpWithToBoolean(Bytecode jump);
 
   template <size_t N>
   INLINE(void Output(Bytecode bytecode, uint32_t(&oprands)[N]));
@@ -248,6 +244,8 @@ class BytecodeArrayBuilder {
                       uint32_t operand_value) const;
   bool LastBytecodeInSameBlock() const;
 
+  bool NeedToBooleanCast();
+
   int BorrowTemporaryRegister();
   void ReturnTemporaryRegister(int reg_index);
   int PrepareForConsecutiveTemporaryRegisters(size_t count);

src/interpreter/bytecode-generator.cc

@@ -526,7 +526,6 @@ void BytecodeGenerator::VisitIfStatement(IfStatement* stmt) {
   BytecodeLabel else_label, end_label;
 
   VisitForAccumulatorValue(stmt->condition());
-  builder()->CastAccumulatorToBoolean();
   builder()->JumpIfFalse(&else_label);
   Visit(stmt->then_statement());
   if (stmt->HasElseStatement()) {
@@ -869,7 +868,6 @@ void BytecodeGenerator::VisitConditional(Conditional* expr) {
   BytecodeLabel else_label, end_label;
 
   VisitForAccumulatorValue(expr->condition());
-  builder()->CastAccumulatorToBoolean();
   builder()->JumpIfFalse(&else_label);
 
   VisitForAccumulatorValue(expr->then_expression());
@@ -1869,7 +1867,7 @@ void BytecodeGenerator::VisitLogicalOrExpression(BinaryOperation* binop) {
   } else {
     BytecodeLabel end_label;
     VisitForAccumulatorValue(left);
-    builder()->JumpIfToBooleanTrue(&end_label);
+    builder()->JumpIfTrue(&end_label);
     VisitForAccumulatorValue(right);
     builder()->Bind(&end_label);
   }
@@ -1888,7 +1886,7 @@ void BytecodeGenerator::VisitLogicalAndExpression(BinaryOperation* binop) {
   } else {
     BytecodeLabel end_label;
     VisitForAccumulatorValue(left);
-    builder()->JumpIfToBooleanFalse(&end_label);
+    builder()->JumpIfFalse(&end_label);
     VisitForAccumulatorValue(right);
     builder()->Bind(&end_label);
   }

test/cctest/interpreter/test-interpreter.cc

@@ -2027,7 +2027,9 @@ TEST(InterpreterLogicalOr) {
                      factory->NewStringFromStaticChars("0")),
       std::make_pair("return 0 || 3.2;\n", factory->NewNumber(3.2)),
       std::make_pair("return 'a' || 0;\n",
-                     factory->NewStringFromStaticChars("a"))};
+                     factory->NewStringFromStaticChars("a")),
+      std::make_pair("var a = '0', b = 10; return (a == 0) || b;\n",
+                     factory->true_value())};
 
   for (size_t i = 0; i < arraysize(literals); i++) {
     std::string source(InterpreterTester::SourceForBody(literals[i].first));
@@ -2052,14 +2054,15 @@ TEST(InterpreterLogicalAnd) {
                      handle(Smi::FromInt(0), isolate)),
       std::make_pair("var a = '0', b = 10; return a && b;\n",
                      handle(Smi::FromInt(10), isolate)),
-      std::make_pair("return 0.0 && 3.2;\n",
-                     handle(Smi::FromInt(0), isolate)),
+      std::make_pair("return 0.0 && 3.2;\n", handle(Smi::FromInt(0), isolate)),
       std::make_pair("return 'a' && 'b';\n",
                      factory->NewStringFromStaticChars("b")),
       std::make_pair("return 'a' && 0 || 'b', 'c';\n",
                      factory->NewStringFromStaticChars("c")),
       std::make_pair("var x = 1, y = 3; return x && 0 + 1 || y;\n",
-                     handle(Smi::FromInt(1), isolate))};
+                     handle(Smi::FromInt(1), isolate)),
+      std::make_pair("var x = 1, y = 3; return (x == 1) && (3 == 3) || y;\n",
+                     factory->true_value())};
 
   for (size_t i = 0; i < arraysize(literals); i++) {
     std::string source(InterpreterTester::SourceForBody(literals[i].first));
@@ -2529,6 +2532,64 @@ TEST(InterpreterGlobalDelete) {
 }
 
 
+TEST(InterpreterBasicLoops) {
+  HandleAndZoneScope handles;
+  i::Isolate* isolate = handles.main_isolate();
+  i::Factory* factory = isolate->factory();
+
+  std::pair<const char*, Handle<Object>> loops[] = {
+      std::make_pair("var a = 10; var b = 1;\n"
+                     "while (a) {\n"
+                     "  b = b * 2;\n"
+                     "  a = a - 1;\n"
+                     "};\n"
+                     "return b;\n",
+                     factory->NewHeapNumber(1024)),
+      std::make_pair("var a = 1; var b = 1;\n"
+                     "do {\n"
+                     "  b = b * 2;\n"
+                     "  --a;\n"
+                     "} while(a);\n"
+                     "return b;\n",
+                     handle(Smi::FromInt(2), isolate)),
+      std::make_pair("var b = 1;\n"
+                     "for ( var a = 10; a; a--) {\n"
+                     "  b *= 2;\n"
+                     "}\n"
+                     "return b;",
+                     factory->NewHeapNumber(1024)),
+      std::make_pair("var a = 10; var b = 1;\n"
+                     "while (a > 0) {\n"
+                     "  b = b * 2;\n"
+                     "  a = a - 1;\n"
+                     "};\n"
+                     "return b;\n",
+                     factory->NewHeapNumber(1024)),
+      std::make_pair("var a = 1; var b = 1;\n"
+                     "do {\n"
+                     "  b = b * 2;\n"
+                     "  --a;\n"
+                     "} while(a);\n"
+                     "return b;\n",
+                     handle(Smi::FromInt(2), isolate)),
+      std::make_pair("var b = 1;\n"
+                     "for ( var a = 10; a > 0; a--) {\n"
+                     "  b *= 2;\n"
+                     "}\n"
+                     "return b;",
+                     factory->NewHeapNumber(1024))};
+
+  for (size_t i = 0; i < arraysize(loops); i++) {
+    std::string source(InterpreterTester::SourceForBody(loops[i].first));
+    InterpreterTester tester(handles.main_isolate(), source.c_str());
+    auto callable = tester.GetCallable<>();
+
+    Handle<i::Object> return_value = callable().ToHandleChecked();
+    CHECK(return_value->SameValue(*loops[i].second));
+  }
+}
+
+
 TEST(InterpreterForIn) {
   HandleAndZoneScope handles;