Commit c03ac1e1 authored by mythria's avatar mythria Committed by Commit bot

[Interpreter] Fixes PreviousBytecodeHelper to check if previous bytecode is in...

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

PreviousBytecodeHelper used to return Bytecode::kLast if the previous bytecode was not
in the same basicblock. Changed it to be instantiated only when the previous bytecode
is in the same basic block.

BUG=v8:4280
LOG=N

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

Cr-Commit-Position: refs/heads/master@{#32702}
parent 6f472db6
...@@ -11,24 +11,29 @@ namespace interpreter { ...@@ -11,24 +11,29 @@ namespace interpreter {
class BytecodeArrayBuilder::PreviousBytecodeHelper { class BytecodeArrayBuilder::PreviousBytecodeHelper {
public: public:
explicit PreviousBytecodeHelper(const BytecodeArrayBuilder& array_builder) explicit PreviousBytecodeHelper(const BytecodeArrayBuilder& array_builder)
: array_builder_(array_builder) {} : array_builder_(array_builder),
previous_bytecode_start_(array_builder_.last_bytecode_start_) {
Bytecode GetBytecode() const { // This helper is expected to be instantiated only when the last bytecode is
// Returns the previous bytecode in the same basicblock. If there is none it // in the same basic block.
// returns Bytecode::kLast. DCHECK(array_builder_.LastBytecodeInSameBlock());
if (!array_builder_.LastBytecodeInSameBlock()) {
return Bytecode::kLast;
} }
// Returns the previous bytecode in the same basic block.
MUST_USE_RESULT Bytecode GetBytecode() const {
DCHECK_EQ(array_builder_.last_bytecode_start_, previous_bytecode_start_);
return Bytecodes::FromByte( return Bytecodes::FromByte(
array_builder_.bytecodes()->at(array_builder_.last_bytecode_start_)); array_builder_.bytecodes()->at(previous_bytecode_start_));
} }
uint32_t GetOperand(int operand_index) const { // 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_EQ(array_builder_.last_bytecode_start_, previous_bytecode_start_);
Bytecode bytecode = GetBytecode(); Bytecode bytecode = GetBytecode();
DCHECK_GE(operand_index, 0); DCHECK_GE(operand_index, 0);
DCHECK_LT(operand_index, Bytecodes::NumberOfOperands(bytecode)); DCHECK_LT(operand_index, Bytecodes::NumberOfOperands(bytecode));
size_t operand_offset = size_t operand_offset =
array_builder_.last_bytecode_start_ + previous_bytecode_start_ +
Bytecodes::GetOperandOffset(bytecode, operand_index); Bytecodes::GetOperandOffset(bytecode, operand_index);
OperandSize size = Bytecodes::GetOperandSize(bytecode, operand_index); OperandSize size = Bytecodes::GetOperandSize(bytecode, operand_index);
switch (size) { switch (size) {
...@@ -52,6 +57,9 @@ class BytecodeArrayBuilder::PreviousBytecodeHelper { ...@@ -52,6 +57,9 @@ class BytecodeArrayBuilder::PreviousBytecodeHelper {
private: private:
const BytecodeArrayBuilder& array_builder_; const BytecodeArrayBuilder& array_builder_;
size_t previous_bytecode_start_;
DISALLOW_COPY_AND_ASSIGN(PreviousBytecodeHelper);
}; };
...@@ -580,6 +588,9 @@ BytecodeArrayBuilder& BytecodeArrayBuilder::PopContext(Register context) { ...@@ -580,6 +588,9 @@ BytecodeArrayBuilder& BytecodeArrayBuilder::PopContext(Register context) {
bool BytecodeArrayBuilder::NeedToBooleanCast() { bool BytecodeArrayBuilder::NeedToBooleanCast() {
if (!LastBytecodeInSameBlock()) {
return true;
}
PreviousBytecodeHelper previous_bytecode(*this); PreviousBytecodeHelper previous_bytecode(*this);
switch (previous_bytecode.GetBytecode()) { switch (previous_bytecode.GetBytecode()) {
case Bytecode::kToBoolean: case Bytecode::kToBoolean:
...@@ -600,8 +611,6 @@ bool BytecodeArrayBuilder::NeedToBooleanCast() { ...@@ -600,8 +611,6 @@ bool BytecodeArrayBuilder::NeedToBooleanCast() {
case Bytecode::kTestIn: case Bytecode::kTestIn:
case Bytecode::kForInDone: case Bytecode::kForInDone:
return false; return false;
// Also handles the case where the previous bytecode was in a different
// block.
default: default:
return true; return true;
} }
...@@ -609,10 +618,14 @@ bool BytecodeArrayBuilder::NeedToBooleanCast() { ...@@ -609,10 +618,14 @@ bool BytecodeArrayBuilder::NeedToBooleanCast() {
BytecodeArrayBuilder& BytecodeArrayBuilder::CastAccumulatorToBoolean() { BytecodeArrayBuilder& BytecodeArrayBuilder::CastAccumulatorToBoolean() {
PreviousBytecodeHelper previous_bytecode(*this); if (!LastBytecodeInSameBlock()) {
Output(Bytecode::kToBoolean);
return *this;
}
// If the previous bytecode puts a boolean in the accumulator // If the previous bytecode puts a boolean in the accumulator
// there is no need to emit an instruction. // there is no need to emit an instruction.
if (NeedToBooleanCast()) { if (NeedToBooleanCast()) {
PreviousBytecodeHelper previous_bytecode(*this);
switch (previous_bytecode.GetBytecode()) { switch (previous_bytecode.GetBytecode()) {
// If the previous bytecode is a constant evaluate it and return false. // If the previous bytecode is a constant evaluate it and return false.
case Bytecode::kLdaZero: { case Bytecode::kLdaZero: {
...@@ -643,20 +656,22 @@ BytecodeArrayBuilder& BytecodeArrayBuilder::CastAccumulatorToJSObject() { ...@@ -643,20 +656,22 @@ BytecodeArrayBuilder& BytecodeArrayBuilder::CastAccumulatorToJSObject() {
BytecodeArrayBuilder& BytecodeArrayBuilder::CastAccumulatorToName() { BytecodeArrayBuilder& BytecodeArrayBuilder::CastAccumulatorToName() {
if (LastBytecodeInSameBlock()) {
PreviousBytecodeHelper previous_bytecode(*this); PreviousBytecodeHelper previous_bytecode(*this);
switch (previous_bytecode.GetBytecode()) { switch (previous_bytecode.GetBytecode()) {
case Bytecode::kToName:
case Bytecode::kTypeOf:
return *this;
case Bytecode::kLdaConstantWide: case Bytecode::kLdaConstantWide:
case Bytecode::kLdaConstant: { case Bytecode::kLdaConstant: {
Handle<Object> object = previous_bytecode.GetConstantForIndexOperand(0); Handle<Object> object = previous_bytecode.GetConstantForIndexOperand(0);
if (object->IsName()) return *this; if (object->IsName()) return *this;
break; break;
} }
case Bytecode::kToName:
case Bytecode::kTypeOf:
return *this;
default: default:
break; break;
} }
}
Output(Bytecode::kToName); Output(Bytecode::kToName);
return *this; return *this;
} }
...@@ -1089,10 +1104,11 @@ bool BytecodeArrayBuilder::LastBytecodeInSameBlock() const { ...@@ -1089,10 +1104,11 @@ bool BytecodeArrayBuilder::LastBytecodeInSameBlock() const {
bool BytecodeArrayBuilder::IsRegisterInAccumulator(Register reg) { bool BytecodeArrayBuilder::IsRegisterInAccumulator(Register reg) {
if (LastBytecodeInSameBlock()) {
PreviousBytecodeHelper previous_bytecode(*this); PreviousBytecodeHelper previous_bytecode(*this);
if (previous_bytecode.GetBytecode() == Bytecode::kLdar || Bytecode bytecode = previous_bytecode.GetBytecode();
previous_bytecode.GetBytecode() == Bytecode::kStar) { if ((bytecode == Bytecode::kLdar || bytecode == Bytecode::kStar) &&
if (reg == Register::FromOperand(previous_bytecode.GetOperand(0))) { (reg == Register::FromOperand(previous_bytecode.GetOperand(0)))) {
return true; return true;
} }
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment