[Interpreter] Adds logical and/or and comma operators to interpreter

Adds support for following operators
 -Logical and
 -Logical or
 -Comma

Adds the above bytecodes, support to BytecodeGenerator and BytecodeArrayBuilder
to enable it's use, it's implementation and tests.

BUG=v8:4280
LOG=N

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

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

src/compiler/bytecode-graph-builder.cc

@@ -560,6 +560,30 @@ void BytecodeGraphBuilder::VisitJumpIfFalseConstant(
 }
 
 
+void BytecodeGraphBuilder::VisitJumpIfToBooleanTrue(
+    const interpreter::BytecodeArrayIterator& iterator) {
+  UNIMPLEMENTED();
+}
+
+
+void BytecodeGraphBuilder::VisitJumpIfToBooleanTrueConstant(
+    const interpreter::BytecodeArrayIterator& iterator) {
+  UNIMPLEMENTED();
+}
+
+
+void BytecodeGraphBuilder::VisitJumpIfToBooleanFalse(
+    const interpreter::BytecodeArrayIterator& iterator) {
+  UNIMPLEMENTED();
+}
+
+
+void BytecodeGraphBuilder::VisitJumpIfToBooleanFalseConstant(
+    const interpreter::BytecodeArrayIterator& iterator) {
+  UNIMPLEMENTED();
+}
+
+
 void BytecodeGraphBuilder::VisitReturn(
     const interpreter::BytecodeArrayIterator& iterator) {
   Node* control =

src/interpreter/bytecode-array-builder.cc

@@ -458,6 +458,10 @@ Bytecode BytecodeArrayBuilder::GetJumpWithConstantOperand(
       return Bytecode::kJumpIfTrueConstant;
     case Bytecode::kJumpIfFalse:
       return Bytecode::kJumpIfFalseConstant;
+    case Bytecode::kJumpIfToBooleanTrue:
+      return Bytecode::kJumpIfToBooleanTrueConstant;
+    case Bytecode::kJumpIfToBooleanFalse:
+      return Bytecode::kJumpIfToBooleanFalseConstant;
     default:
       UNREACHABLE();
       return Bytecode::kJumpConstant;
@@ -546,6 +550,18 @@ 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::Return() {
   Output(Bytecode::kReturn);
   return_seen_in_block_ = true;

src/interpreter/bytecode-array-builder.h

@@ -129,6 +129,11 @@ class BytecodeArrayBuilder {
   BytecodeArrayBuilder& Jump(BytecodeLabel* label);
   BytecodeArrayBuilder& JumpIfTrue(BytecodeLabel* label);
   BytecodeArrayBuilder& JumpIfFalse(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& Return();
 
   BytecodeArrayBuilder& EnterBlock();

src/interpreter/bytecode-generator.cc

@@ -1059,9 +1059,13 @@ void BytecodeGenerator::VisitCountOperation(CountOperation* expr) {
 void BytecodeGenerator::VisitBinaryOperation(BinaryOperation* binop) {
   switch (binop->op()) {
     case Token::COMMA:
+      VisitCommaExpression(binop);
+      break;
     case Token::OR:
+      VisitLogicalOrExpression(binop);
+      break;
     case Token::AND:
-      UNIMPLEMENTED();
+      VisitLogicalAndExpression(binop);
       break;
     default:
       VisitArithmeticExpression(binop);
@@ -1194,6 +1198,53 @@ void BytecodeGenerator::VisitSetHomeObject(Register value, Register home_object,
 }
 
 
+void BytecodeGenerator::VisitCommaExpression(BinaryOperation* binop) {
+  Expression* left = binop->left();
+  Expression* right = binop->right();
+
+  Visit(left);
+  Visit(right);
+}
+
+
+void BytecodeGenerator::VisitLogicalOrExpression(BinaryOperation* binop) {
+  Expression* left = binop->left();
+  Expression* right = binop->right();
+
+  // Short-circuit evaluation- If it is known that left is always true,
+  // no need to visit right
+  if (left->ToBooleanIsTrue()) {
+    Visit(left);
+  } else {
+    BytecodeLabel end_label;
+
+    Visit(left);
+    builder()->JumpIfToBooleanTrue(&end_label);
+    Visit(right);
+    builder()->Bind(&end_label);
+  }
+}
+
+
+void BytecodeGenerator::VisitLogicalAndExpression(BinaryOperation* binop) {
+  Expression* left = binop->left();
+  Expression* right = binop->right();
+
+  // Short-circuit evaluation- If it is known that left is always false,
+  // no need to visit right
+  if (left->ToBooleanIsFalse()) {
+    Visit(left);
+  } else {
+    BytecodeLabel end_label;
+
+    Visit(left);
+    builder()->JumpIfToBooleanFalse(&end_label);
+    Visit(right);
+    builder()->Bind(&end_label);
+  }
+}
+
+
 LanguageMode BytecodeGenerator::language_mode() const {
   return info()->language_mode();
 }

src/interpreter/bytecode-generator.h

@@ -37,6 +37,9 @@ class BytecodeGenerator : public AstVisitor {
   DEFINE_AST_VISITOR_SUBCLASS_MEMBERS();
 
   void VisitArithmeticExpression(BinaryOperation* binop);
+  void VisitCommaExpression(BinaryOperation* binop);
+  void VisitLogicalOrExpression(BinaryOperation* binop);
+  void VisitLogicalAndExpression(BinaryOperation* binop);
   void VisitPropertyLoad(Register obj, Property* expr);
   void VisitVariableLoad(Variable* variable, FeedbackVectorSlot slot);
   void VisitVariableAssignment(Variable* variable, FeedbackVectorSlot slot);

src/interpreter/bytecodes.cc

@@ -161,7 +161,9 @@ OperandSize Bytecodes::SizeOfOperand(OperandType operand_type) {
 // static
 bool Bytecodes::IsJump(Bytecode bytecode) {
   return bytecode == Bytecode::kJump || bytecode == Bytecode::kJumpIfTrue ||
-         bytecode == Bytecode::kJumpIfFalse;
+         bytecode == Bytecode::kJumpIfFalse ||
+         bytecode == Bytecode::kJumpIfToBooleanTrue ||
+         bytecode == Bytecode::kJumpIfToBooleanFalse;
 }
 
 
@@ -169,7 +171,9 @@ bool Bytecodes::IsJump(Bytecode bytecode) {
 bool Bytecodes::IsJumpConstant(Bytecode bytecode) {
   return bytecode == Bytecode::kJumpConstant ||
          bytecode == Bytecode::kJumpIfTrueConstant ||
-         bytecode == Bytecode::kJumpIfFalseConstant;
+         bytecode == Bytecode::kJumpIfFalseConstant ||
+         bytecode == Bytecode::kJumpIfToBooleanTrueConstant ||
+         bytecode == Bytecode::kJumpIfToBooleanFalseConstant;
 }
 
 

src/interpreter/bytecodes.h

@@ -124,6 +124,10 @@ namespace interpreter {
   V(JumpIfTrueConstant, OperandType::kIdx8)                                    \
   V(JumpIfFalse, OperandType::kImm8)                                           \
   V(JumpIfFalseConstant, OperandType::kIdx8)                                   \
+  V(JumpIfToBooleanTrue, OperandType::kImm8)                                   \
+  V(JumpIfToBooleanTrueConstant, OperandType::kIdx8)                           \
+  V(JumpIfToBooleanFalse, OperandType::kImm8)                                  \
+  V(JumpIfToBooleanFalseConstant, OperandType::kIdx8)                          \
   V(Return, OperandType::kNone)
 
 

src/interpreter/interpreter.cc

@@ -738,6 +738,72 @@ void Interpreter::DoCreateLiteral(Runtime::FunctionId function_id,
 }
 
 
+// JumpIfToBooleanTrue <imm8>
+//
+// Jump by number of bytes represented by an immediate operand if the object
+// referenced by the accumulator is true when the object is cast to boolean.
+void Interpreter::DoJumpIfToBooleanTrue(
+    compiler::InterpreterAssembler* assembler) {
+  Node* accumulator = __ GetAccumulator();
+  Node* relative_jump = __ BytecodeOperandImm8(0);
+  Node* to_boolean_value =
+      __ CallRuntime(Runtime::kInterpreterToBoolean, accumulator);
+  Node* true_value = __ BooleanConstant(true);
+  __ JumpIfWordEqual(to_boolean_value, true_value, relative_jump);
+}
+
+
+// JumpIfToBooleanTrueConstant <idx>
+//
+// Jump by number of bytes in the Smi in the |idx| entry in the constant pool
+// if the object referenced by the accumulator is true when the object is cast
+// to boolean.
+void Interpreter::DoJumpIfToBooleanTrueConstant(
+    compiler::InterpreterAssembler* assembler) {
+  Node* accumulator = __ GetAccumulator();
+  Node* to_boolean_value =
+      __ CallRuntime(Runtime::kInterpreterToBoolean, accumulator);
+  Node* index = __ BytecodeOperandIdx8(0);
+  Node* constant = __ LoadConstantPoolEntry(index);
+  Node* relative_jump = __ SmiUntag(constant);
+  Node* true_value = __ BooleanConstant(true);
+  __ JumpIfWordEqual(to_boolean_value, true_value, relative_jump);
+}
+
+
+// JumpIfToBooleanFalse <imm8>
+//
+// Jump by number of bytes represented by an immediate operand if the object
+// referenced by the accumulator is false when the object is cast to boolean.
+void Interpreter::DoJumpIfToBooleanFalse(
+    compiler::InterpreterAssembler* assembler) {
+  Node* accumulator = __ GetAccumulator();
+  Node* relative_jump = __ BytecodeOperandImm8(0);
+  Node* to_boolean_value =
+      __ CallRuntime(Runtime::kInterpreterToBoolean, accumulator);
+  Node* false_value = __ BooleanConstant(false);
+  __ JumpIfWordEqual(to_boolean_value, false_value, relative_jump);
+}
+
+
+// JumpIfToBooleanFalseConstant <idx>
+//
+// Jump by number of bytes in the Smi in the |idx| entry in the constant pool
+// if the object referenced by the accumulator is false when the object is cast
+// to boolean.
+void Interpreter::DoJumpIfToBooleanFalseConstant(
+    compiler::InterpreterAssembler* assembler) {
+  Node* accumulator = __ GetAccumulator();
+  Node* to_boolean_value =
+      __ CallRuntime(Runtime::kInterpreterToBoolean, accumulator);
+  Node* index = __ BytecodeOperandIdx8(0);
+  Node* constant = __ LoadConstantPoolEntry(index);
+  Node* relative_jump = __ SmiUntag(constant);
+  Node* false_value = __ BooleanConstant(false);
+  __ JumpIfWordEqual(to_boolean_value, false_value, relative_jump);
+}
+
+
 // CreateArrayLiteral <idx> <flags>
 //
 // Creates an array literal for literal index <idx> with flags <flags> and

test/cctest/interpreter/test-interpreter.cc

@@ -1725,3 +1725,89 @@ TEST(InterpreterObjectLiterals) {
     CHECK(return_value->SameValue(*literals[i].second));
   }
 }
+
+
+TEST(InterpreterComma) {
+  HandleAndZoneScope handles;
+  i::Isolate* isolate = handles.main_isolate();
+  i::Factory* factory = isolate->factory();
+
+  std::pair<const char*, Handle<Object>> literals[6] = {
+      std::make_pair("var a; return 0, a;\n", factory->undefined_value()),
+      std::make_pair("return 'a', 2.2, 3;\n",
+                     Handle<Object>(Smi::FromInt(3), isolate)),
+      std::make_pair("return 'a', 'b', 'c';\n",
+                     factory->NewStringFromStaticChars("c")),
+      std::make_pair("return 3.2, 2.3, 4.5;\n", factory->NewNumber(4.5)),
+      std::make_pair("var a = 10; return b = a, b = b+1;\n",
+                     Handle<Object>(Smi::FromInt(11), isolate)),
+      std::make_pair("var a = 10; return b = a, b = b+1, b + 10;\n",
+                     Handle<Object>(Smi::FromInt(21), isolate))};
+
+  for (size_t i = 0; i < arraysize(literals); i++) {
+    std::string source(InterpreterTester::SourceForBody(literals[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(*literals[i].second));
+  }
+}
+
+
+TEST(InterpreterLogicalOr) {
+  HandleAndZoneScope handles;
+  i::Isolate* isolate = handles.main_isolate();
+  i::Factory* factory = isolate->factory();
+
+  std::pair<const char*, Handle<Object>> literals[5] = {
+      std::make_pair("var a, b; return a || b;\n", factory->undefined_value()),
+      std::make_pair("var a, b = 10; return a || b;\n",
+                     Handle<Object>(Smi::FromInt(10), isolate)),
+      std::make_pair("var a = '0', b = 10; return a || b;\n",
+                     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"))};
+
+  for (size_t i = 0; i < arraysize(literals); i++) {
+    std::string source(InterpreterTester::SourceForBody(literals[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(*literals[i].second));
+  }
+}
+
+
+TEST(InterpreterLogicalAnd) {
+  HandleAndZoneScope handles;
+  i::Isolate* isolate = handles.main_isolate();
+  i::Factory* factory = isolate->factory();
+
+  std::pair<const char*, Handle<Object>> literals[7] = {
+      std::make_pair("var a, b = 10; return a && b;\n",
+                     factory->undefined_value()),
+      std::make_pair("var a = 0, b = 10; return a && b / a;\n",
+                     Handle<Object>(Smi::FromInt(0), isolate)),
+      std::make_pair("var a = '0', b = 10; return a && b;\n",
+                     Handle<Object>(Smi::FromInt(10), isolate)),
+      std::make_pair("return 0.0 && 3.2;\n",
+                     Handle<Object>(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<Object>(Smi::FromInt(1), isolate))};
+
+  for (size_t i = 0; i < arraysize(literals); i++) {
+    std::string source(InterpreterTester::SourceForBody(literals[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(*literals[i].second));
+  }
+}

test/unittests/interpreter/bytecode-array-builder-unittest.cc

@@ -111,13 +111,21 @@ TEST_F(BytecodeArrayBuilderTest, AllBytecodesGenerated) {
   BytecodeLabel start;
   builder.Bind(&start);
   // Short jumps with Imm8 operands
-  builder.Jump(&start).JumpIfTrue(&start).JumpIfFalse(&start);
+  builder.Jump(&start)
+      .JumpIfTrue(&start)
+      .JumpIfFalse(&start)
+      .JumpIfToBooleanTrue(&start)
+      .JumpIfToBooleanFalse(&start);
   // Insert dummy ops to force longer jumps
   for (int i = 0; i < 128; i++) {
     builder.LoadTrue();
   }
   // Longer jumps requiring Constant operand
-  builder.Jump(&start).JumpIfTrue(&start).JumpIfFalse(&start);
+  builder.Jump(&start)
+      .JumpIfTrue(&start)
+      .JumpIfFalse(&start)
+      .JumpIfToBooleanTrue(&start)
+      .JumpIfToBooleanFalse(&start);
   builder.Return();
 
   // Generate BytecodeArray.