[Interpreter] Merges ToBoolean and JumpIfTrue/False bytecodes

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

 // Copyright 2014 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/v8.h"
 
 #include "src/interpreter/bytecode-array-builder.h"
 #include "src/interpreter/bytecode-array-iterator.h"
 #include "test/unittests/test-utils.h"
 
@@ skipping 114 matching lines @@
 
   // Emit cast operator invocations.
   builder.CastAccumulatorToNumber()
       .CastAccumulatorToBoolean()
       .CastAccumulatorToName();
 
   // Emit control flow. Return must be the last instruction.
   BytecodeLabel start;
   builder.Bind(&start);
   // Short jumps with Imm8 operands
-  builder.Jump(&start)
+  builder.Jump(&start);
+  // Perform an operation that returns boolean value to
+  // generate JumpIfTrue/False
+  builder.CompareOperation(Token::Value::EQ, reg, Strength::WEAK)
       .JumpIfTrue(&start)
-      .JumpIfFalse(&start)
-      .JumpIfToBooleanTrue(&start)
-      .JumpIfToBooleanFalse(&start);
+      .CompareOperation(Token::Value::EQ, reg, Strength::WEAK)
+      .JumpIfFalse(&start);
+  // Perform an operation that returns a non-boolean operation to
+  // generate JumpIfToBooleanTrue/False.
+  builder.BinaryOperation(Token::Value::ADD, reg, Strength::WEAK)
+      .JumpIfTrue(&start)
+      .BinaryOperation(Token::Value::ADD, reg, Strength::WEAK)
+      .JumpIfFalse(&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)
+  builder.Jump(&start);
+  // Perform an operation that returns boolean value to
+  // generate JumpIfTrue/False
+  builder.CompareOperation(Token::Value::EQ, reg, Strength::WEAK)
       .JumpIfTrue(&start)
-      .JumpIfFalse(&start)
-      .JumpIfToBooleanTrue(&start)
-      .JumpIfToBooleanFalse(&start);
+      .CompareOperation(Token::Value::EQ, reg, Strength::WEAK)
+      .JumpIfFalse(&start);
+  // Perform an operation that returns a non-boolean operation to
+  // generate JumpIfToBooleanTrue/False.
+  builder.JumpIfTrue(&start).JumpIfFalse(&start);
 
   builder.EnterBlock()
       .Throw()
       .LeaveBlock();
 
   builder.Return();
 
   // Generate BytecodeArray.
   Handle<BytecodeArray> the_array = builder.ToBytecodeArray();
   CHECK_EQ(the_array->frame_size(),
@@ skipping 147 matching lines @@
   // Should only have one entry for each identical constant.
   CHECK_EQ(array->constant_pool()->length(), 3);
 }
 
 
 TEST_F(BytecodeArrayBuilderTest, ForwardJumps) {
   static const int kFarJumpDistance = 256;
 
   BytecodeArrayBuilder builder(isolate(), zone());
   builder.set_parameter_count(0);
-  builder.set_locals_count(0);
+  builder.set_locals_count(1);
   builder.set_context_count(0);
 
-  BytecodeLabel far0, far1, far2;
-  BytecodeLabel near0, near1, near2;
+  Register reg(0);
+  BytecodeLabel far0, far1, far2, far3, far4;
+  BytecodeLabel near0, near1, near2, near3, near4;
 
   builder.Jump(&near0)
+      .CompareOperation(Token::Value::EQ, reg, Strength::WEAK)
       .JumpIfTrue(&near1)
+      .CompareOperation(Token::Value::EQ, reg, Strength::WEAK)
       .JumpIfFalse(&near2)
+      .BinaryOperation(Token::Value::ADD, reg, Strength::WEAK)
+      .JumpIfTrue(&near3)
+      .BinaryOperation(Token::Value::ADD, reg, Strength::WEAK)
+      .JumpIfFalse(&near4)
       .Bind(&near0)
       .Bind(&near1)
       .Bind(&near2)
+      .Bind(&near3)
+      .Bind(&near4)
       .Jump(&far0)
+      .CompareOperation(Token::Value::EQ, reg, Strength::WEAK)
       .JumpIfTrue(&far1)
-      .JumpIfFalse(&far2);
-  for (int i = 0; i < kFarJumpDistance - 6; i++) {
+      .CompareOperation(Token::Value::EQ, reg, Strength::WEAK)
+      .JumpIfFalse(&far2)
+      .BinaryOperation(Token::Value::ADD, reg, Strength::WEAK)
+      .JumpIfTrue(&far3)
+      .BinaryOperation(Token::Value::ADD, reg, Strength::WEAK)
+      .JumpIfFalse(&far4);
+  for (int i = 0; i < kFarJumpDistance - 18; i++) {
     builder.LoadUndefined();
   }
-  builder.Bind(&far0).Bind(&far1).Bind(&far2);
+  builder.Bind(&far0).Bind(&far1).Bind(&far2).Bind(&far3).Bind(&far4);
   builder.Return();
 
   Handle<BytecodeArray> array = builder.ToBytecodeArray();
-  DCHECK_EQ(array->length(), 12 + kFarJumpDistance - 6 + 1);
+  DCHECK_EQ(array->length(), 36 + kFarJumpDistance - 18 + 1);
 
   BytecodeArrayIterator iterator(array);
   CHECK_EQ(iterator.current_bytecode(), Bytecode::kJump);
+  CHECK_EQ(iterator.GetImmediateOperand(0), 18);
+  iterator.Advance();
+
+  // Ignore compare operation.
+  iterator.Advance();
+
+  CHECK_EQ(iterator.current_bytecode(), Bytecode::kJumpIfTrue);
+  CHECK_EQ(iterator.GetImmediateOperand(0), 14);
+  iterator.Advance();
+
+  // Ignore compare operation.
+  iterator.Advance();
+
+  CHECK_EQ(iterator.current_bytecode(), Bytecode::kJumpIfFalse);
+  CHECK_EQ(iterator.GetImmediateOperand(0), 10);
+  iterator.Advance();
+
+  // Ignore add operation.
+  iterator.Advance();
+
+  CHECK_EQ(iterator.current_bytecode(), Bytecode::kJumpIfToBooleanTrue);
   CHECK_EQ(iterator.GetImmediateOperand(0), 6);
   iterator.Advance();
 
-  CHECK_EQ(iterator.current_bytecode(), Bytecode::kJumpIfTrue);
-  CHECK_EQ(iterator.GetImmediateOperand(0), 4);
+  // Ignore add operation.
   iterator.Advance();
 
-  CHECK_EQ(iterator.current_bytecode(), Bytecode::kJumpIfFalse);
+  CHECK_EQ(iterator.current_bytecode(), Bytecode::kJumpIfToBooleanFalse);
   CHECK_EQ(iterator.GetImmediateOperand(0), 2);
   iterator.Advance();
 
+
   CHECK_EQ(iterator.current_bytecode(), Bytecode::kJumpConstant);
   CHECK_EQ(*iterator.GetConstantForIndexOperand(0),
            Smi::FromInt(kFarJumpDistance));
   CHECK_EQ(
       array->get(iterator.current_offset() +
                  Smi::cast(*iterator.GetConstantForIndexOperand(0))->value()),
       Bytecodes::ToByte(Bytecode::kReturn));
   iterator.Advance();
 
+  // Ignore compare operation.
+  iterator.Advance();
+
   CHECK_EQ(iterator.current_bytecode(), Bytecode::kJumpIfTrueConstant);
   CHECK_EQ(*iterator.GetConstantForIndexOperand(0),
-           Smi::FromInt(kFarJumpDistance - 2));
+           Smi::FromInt(kFarJumpDistance - 4));
   CHECK_EQ(
       array->get(iterator.current_offset() +
                  Smi::cast(*iterator.GetConstantForIndexOperand(0))->value()),
       Bytecodes::ToByte(Bytecode::kReturn));
   iterator.Advance();
 
+  // Ignore compare operation.
+  iterator.Advance();
+
   CHECK_EQ(iterator.current_bytecode(), Bytecode::kJumpIfFalseConstant);
   CHECK_EQ(*iterator.GetConstantForIndexOperand(0),
-           Smi::FromInt(kFarJumpDistance - 4));
+           Smi::FromInt(kFarJumpDistance - 8));
   CHECK_EQ(
       array->get(iterator.current_offset() +
                  Smi::cast(*iterator.GetConstantForIndexOperand(0))->value()),
+      Bytecodes::ToByte(Bytecode::kReturn));
+  iterator.Advance();
+
+  // Ignore add operation.
+  iterator.Advance();
+
+  CHECK_EQ(iterator.current_bytecode(), Bytecode::kJumpIfToBooleanTrueConstant);
+  CHECK_EQ(*iterator.GetConstantForIndexOperand(0),
+           Smi::FromInt(kFarJumpDistance - 12));
+  CHECK_EQ(
+      array->get(iterator.current_offset() +
+                 Smi::cast(*iterator.GetConstantForIndexOperand(0))->value()),
+      Bytecodes::ToByte(Bytecode::kReturn));

[rmcilroy, 2015/10/30 10:36:00]

I'm not convinced this check is necessary (given it's implicitly checked above),
however if you want to keep it could you pull some of the logic out of the CHECK
to make it more managable - e.g., pulling out:

Handle<Object> jump_offset = iterator.GetConstantForIndexOperand(0);
CHECK_EQ(*jump_offset, Smi::FromInt(kFarJumpDistance - 12));
CHECK_EQ(array->get(iterator.current_offset(),
Smi::cast(*jump_offset->value())), 
         Bytecodes::ToByte(Bytecode::kReturn));

[mythria, 2015/10/30 11:44:47]

Done.

+  iterator.Advance();
+
+  // Ignore add operation.
+  iterator.Advance();
+
+  CHECK_EQ(iterator.current_bytecode(),
+           Bytecode::kJumpIfToBooleanFalseConstant);
+  CHECK_EQ(*iterator.GetConstantForIndexOperand(0),
+           Smi::FromInt(kFarJumpDistance - 16));
+  CHECK_EQ(
+      array->get(iterator.current_offset() +
+                 Smi::cast(*iterator.GetConstantForIndexOperand(0))->value()),

[rmcilroy, 2015/10/30 10:36:00]

ditto

[mythria, 2015/10/30 11:44:47]

Done.

       Bytecodes::ToByte(Bytecode::kReturn));
   iterator.Advance();
 }
 
 
 TEST_F(BytecodeArrayBuilderTest, BackwardJumps) {
   BytecodeArrayBuilder builder(isolate(), zone());
   builder.set_parameter_count(0);
-  builder.set_locals_count(0);
+  builder.set_locals_count(1);
   builder.set_context_count(0);
+  Register reg(0);
 
-  BytecodeLabel label0, label1, label2;
+  BytecodeLabel label0, label1, label2, label3, label4;
   builder.Bind(&label0)
       .Jump(&label0)
+      .CompareOperation(Token::Value::EQ, reg, Strength::WEAK)
       .Bind(&label1)
       .JumpIfTrue(&label1)
+      .CompareOperation(Token::Value::EQ, reg, Strength::WEAK)
       .Bind(&label2)
+      .JumpIfFalse(&label2)
+      .BinaryOperation(Token::Value::ADD, reg, Strength::WEAK)
+      .Bind(&label3)
+      .JumpIfTrue(&label3)
+      .BinaryOperation(Token::Value::ADD, reg, Strength::WEAK)
+      .Bind(&label4)
+      .JumpIfFalse(&label4);
+  for (int i = 0; i < 64; i++) {
+    builder.Jump(&label4);
+  }
+  builder.BinaryOperation(Token::Value::ADD, reg, Strength::WEAK)
+      .JumpIfFalse(&label4);
+  builder.BinaryOperation(Token::Value::ADD, reg, Strength::WEAK)
+      .JumpIfTrue(&label3);
+  builder.CompareOperation(Token::Value::EQ, reg, Strength::WEAK)
       .JumpIfFalse(&label2);
-  for (int i = 0; i < 64; i++) {
-    builder.Jump(&label2);
-  }
-  builder.JumpIfFalse(&label2);
-  builder.JumpIfTrue(&label1);
+  builder.CompareOperation(Token::Value::EQ, reg, Strength::WEAK)
+      .JumpIfTrue(&label1);
   builder.Jump(&label0);
   builder.Return();
 
   Handle<BytecodeArray> array = builder.ToBytecodeArray();
   BytecodeArrayIterator iterator(array);
   CHECK_EQ(iterator.current_bytecode(), Bytecode::kJump);
   CHECK_EQ(iterator.GetImmediateOperand(0), 0);
   iterator.Advance();
+  // Ignore compare operation.
+  iterator.Advance();
   CHECK_EQ(iterator.current_bytecode(), Bytecode::kJumpIfTrue);
   CHECK_EQ(iterator.GetImmediateOperand(0), 0);
   iterator.Advance();
+  // Ignore compare operation.
+  iterator.Advance();
   CHECK_EQ(iterator.current_bytecode(), Bytecode::kJumpIfFalse);
   CHECK_EQ(iterator.GetImmediateOperand(0), 0);
   iterator.Advance();
+  // Ignore binary operation.
+  iterator.Advance();
+  CHECK_EQ(iterator.current_bytecode(), Bytecode::kJumpIfToBooleanTrue);
+  CHECK_EQ(iterator.GetImmediateOperand(0), 0);
+  iterator.Advance();
+  // Ignore binary operation.
+  iterator.Advance();
+  CHECK_EQ(iterator.current_bytecode(), Bytecode::kJumpIfToBooleanFalse);
+  CHECK_EQ(iterator.GetImmediateOperand(0), 0);
+  iterator.Advance();
   for (int i = 0; i < 64; i++) {
     CHECK_EQ(iterator.current_bytecode(), Bytecode::kJump);
     CHECK_EQ(iterator.GetImmediateOperand(0), -i * 2 - 2);
     iterator.Advance();
   }
+  // Ignore binary operation.
+  iterator.Advance();
+  CHECK_EQ(iterator.current_bytecode(),
+           Bytecode::kJumpIfToBooleanFalseConstant);
+  CHECK_EQ(Smi::cast(*iterator.GetConstantForIndexOperand(0))->value(), -132);
+  iterator.Advance();
+  // Ignore binary operation.
+  iterator.Advance();
+  CHECK_EQ(iterator.current_bytecode(), Bytecode::kJumpIfToBooleanTrueConstant);
+  CHECK_EQ(Smi::cast(*iterator.GetConstantForIndexOperand(0))->value(), -140);
+  iterator.Advance();
+  // Ignore compare operation.
+  iterator.Advance();
   CHECK_EQ(iterator.current_bytecode(), Bytecode::kJumpIfFalseConstant);
-  CHECK_EQ(Smi::cast(*iterator.GetConstantForIndexOperand(0))->value(), -130);
+  CHECK_EQ(Smi::cast(*iterator.GetConstantForIndexOperand(0))->value(), -148);
+  iterator.Advance();
+  // Ignore compare operation.
   iterator.Advance();
   CHECK_EQ(iterator.current_bytecode(), Bytecode::kJumpIfTrueConstant);
-  CHECK_EQ(Smi::cast(*iterator.GetConstantForIndexOperand(0))->value(), -134);
+  CHECK_EQ(Smi::cast(*iterator.GetConstantForIndexOperand(0))->value(), -156);
   iterator.Advance();
   CHECK_EQ(iterator.current_bytecode(), Bytecode::kJumpConstant);
-  CHECK_EQ(Smi::cast(*iterator.GetConstantForIndexOperand(0))->value(), -138);
+  CHECK_EQ(Smi::cast(*iterator.GetConstantForIndexOperand(0))->value(), -162);
   iterator.Advance();
   CHECK_EQ(iterator.current_bytecode(), Bytecode::kReturn);
   iterator.Advance();
   CHECK(iterator.done());
 }
 
 
 TEST_F(BytecodeArrayBuilderTest, LabelReuse) {
   BytecodeArrayBuilder builder(isolate(), zone());
   builder.set_parameter_count(0);
@@ skipping 101 matching lines @@
 
   CHECK_EQ(iterator.current_bytecode(), Bytecode::kReturn);
   iterator.Advance();
   CHECK(iterator.done());
 }
 
 
 }  // namespace interpreter
 }  // namespace internal
 }  // namespace v8