[Interpreter] Fixes PreviousBytecodeHelper to check if previous bytecode is in the same basic block.

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 {
 
 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;
-    }
-    return Bytecodes::FromByte(
-        array_builder_.bytecodes()->at(array_builder_.last_bytecode_start_));
+      : array_builder_(array_builder),
+        previous_bytecode_start_(array_builder_.last_bytecode_start_) {
+    // This helper is expected to be instantiated only when the last bytecode is
+    // in the same basic block.
+    DCHECK(array_builder_.LastBytecodeInSameBlock());
   }
 
-  uint32_t GetOperand(int operand_index) const {
+  // Returns the previous bytecode in the same basic block.
+  MUST_USE_RESULT Bytecode GetBytecode() const {
+    DCHECK(array_builder_.last_bytecode_start_ == previous_bytecode_start_);

[rmcilroy, 2015/12/08 15:25:50]

DCHECK_EQ

[mythria, 2015/12/09 09:10:27]

Done.

+    return Bytecodes::FromByte(
+        array_builder_.bytecodes()->at(previous_bytecode_start_));
+  }
+
+  // Returns the operand at operand_index for the previous bytecode in the
+  // same basic block.
+  MUST_USE_RESULT uint32_t GetOperand(int operand_index) const {
+    DCHECK(array_builder_.last_bytecode_start_ == previous_bytecode_start_);

[rmcilroy, 2015/12/08 15:25:50]

ditto

[mythria, 2015/12/09 09:10:26]

Done.

     Bytecode bytecode = GetBytecode();
     DCHECK_GE(operand_index, 0);
     DCHECK_LT(operand_index, Bytecodes::NumberOfOperands(bytecode));
     size_t operand_offset =
-        array_builder_.last_bytecode_start_ +
+        previous_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_;
+  size_t previous_bytecode_start_;
+
+  DISALLOW_COPY_AND_ASSIGN(PreviousBytecodeHelper);
 };
 
 
 BytecodeArrayBuilder::BytecodeArrayBuilder(Isolate* isolate, Zone* zone)
     : isolate_(isolate),
       zone_(zone),
       bytecodes_(zone),
       bytecode_generated_(false),
       last_block_end_(0),
       last_bytecode_start_(~0),
@@ skipping 508 matching lines @@
 }
 
 
 BytecodeArrayBuilder& BytecodeArrayBuilder::PopContext(Register context) {
   Output(Bytecode::kPopContext, context.ToOperand());
   return *this;
 }
 
 
 bool BytecodeArrayBuilder::NeedToBooleanCast() {
+  if (!LastBytecodeInSameBlock()) {
+    return true;
+  }
   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:
     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;
-    // Also handles the case where the previous bytecode was in a different
-    // block.
     default:
       return true;
   }
 }
 
 
 BytecodeArrayBuilder& BytecodeArrayBuilder::CastAccumulatorToBoolean() {
-  PreviousBytecodeHelper previous_bytecode(*this);
+  if (!LastBytecodeInSameBlock()) {
+    Output(Bytecode::kToBoolean);
+    return *this;
+  }
   // If the previous bytecode puts a boolean in the accumulator
   // there is no need to emit an instruction.
   if (NeedToBooleanCast()) {
+    PreviousBytecodeHelper previous_bytecode(*this);
     switch (previous_bytecode.GetBytecode()) {
       // 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;
       }
@@ skipping 10 matching lines @@
 }
 
 
 BytecodeArrayBuilder& BytecodeArrayBuilder::CastAccumulatorToJSObject() {
   Output(Bytecode::kToObject);
   return *this;
 }
 
 
 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;
+  if (LastBytecodeInSameBlock()) {
+    PreviousBytecodeHelper previous_bytecode(*this);
+    switch (previous_bytecode.GetBytecode()) {
+      case Bytecode::kToName:
+      case Bytecode::kTypeOf:
+        return *this;
+      case Bytecode::kLdaConstantWide:
+      case Bytecode::kLdaConstant: {
+        Handle<Object> object = previous_bytecode.GetConstantForIndexOperand(0);
+        if (object->IsName()) return *this;
+        break;
+      }
+      default:
+        break;
     }
-    case Bytecode::kToName:
-    case Bytecode::kTypeOf:
-      return *this;
-    default:
-      break;
   }
   Output(Bytecode::kToName);
   return *this;
 }
 
 
 BytecodeArrayBuilder& BytecodeArrayBuilder::CastAccumulatorToNumber() {
   // TODO(rmcilroy): consider omitting if the preceeding bytecode always returns
   // a number.
   Output(Bytecode::kToNumber);
@@ skipping 413 matching lines @@
 }
 
 
 bool BytecodeArrayBuilder::LastBytecodeInSameBlock() const {
   return last_bytecode_start_ < bytecodes()->size() &&
          last_bytecode_start_ >= last_block_end_;
 }
 
 
 bool BytecodeArrayBuilder::IsRegisterInAccumulator(Register reg) {
-  PreviousBytecodeHelper previous_bytecode(*this);
-  if (previous_bytecode.GetBytecode() == Bytecode::kLdar ||
-      previous_bytecode.GetBytecode() == Bytecode::kStar) {
-    if (reg == Register::FromOperand(previous_bytecode.GetOperand(0))) {
+  if (LastBytecodeInSameBlock()) {
+    PreviousBytecodeHelper previous_bytecode(*this);
+    Bytecode bytecode = previous_bytecode.GetBytecode();
+    if ((bytecode == Bytecode::kLdar || bytecode == Bytecode::kStar) &&
+        (reg == Register::FromOperand(previous_bytecode.GetOperand(0)))) {
       return true;
     }
   }
   return false;
 }
 
 
 // static
 Bytecode BytecodeArrayBuilder::BytecodeForBinaryOperation(Token::Value op) {
   switch (op) {
@@ skipping 318 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