Index: src/interpreter/bytecode-generator.cc |
diff --git a/src/interpreter/bytecode-generator.cc b/src/interpreter/bytecode-generator.cc |
index 309b9294297674b1b6ff75e4214df11865624620..5cf1286545db5cba812d02a8c4ab5d773c26cfd0 100644 |
--- a/src/interpreter/bytecode-generator.cc |
+++ b/src/interpreter/bytecode-generator.cc |
@@ -4,8 +4,6 @@ |
#include "src/interpreter/bytecode-generator.h" |
-#include <stack> |
- |
#include "src/compiler.h" |
#include "src/interpreter/control-flow-builders.h" |
#include "src/objects.h" |
@@ -146,6 +144,132 @@ void BytecodeGenerator::ControlScope::PerformCommand(Command command, |
} |
+// Scoped base class for determining where the result of an expression |
+// is stored. |
+class BytecodeGenerator::ExpressionResultScope { |
+ public: |
+ explicit ExpressionResultScope(BytecodeGenerator* generator) |
+ : generator_(generator), |
+ parent_(generator->result_scope()), |
+ allocator_(builder()), |
+ result_identified_(false) { |
+ generator_->set_result_scope(this); |
+ } |
+ |
+ virtual ~ExpressionResultScope() { |
+ generator_->set_result_scope(parent_); |
+ DCHECK_EQ(result_identified(), true); |
+ } |
+ |
+ virtual void SetResultInAccumulator() = 0; |
+ virtual void SetResultInRegister(Register reg) = 0; |
+ |
+ BytecodeGenerator* generator() const { return generator_; } |
+ BytecodeArrayBuilder* builder() const { return generator()->builder(); } |
+ ExpressionResultScope* parent() const { return parent_; } |
rmcilroy
2015/10/19 12:56:22
nit - outer() and outer_ to be consistent with oth
oth
2015/10/20 15:28:52
Done.
|
+ |
+ Register NewRegister() { return allocator_.NewRegister(); } |
+ void PrepareForConsecutiveAllocations(size_t count) { |
+ allocator_.PrepareForConsecutiveAllocations(count); |
+ } |
+ |
+ protected: |
+ void set_result_identified() { |
+ DCHECK(!result_identified()); |
+ result_identified_ = true; |
+ } |
+ |
+ bool result_identified() const { return result_identified_; } |
+ |
+ private: |
+ BytecodeGenerator* generator_; |
+ ExpressionResultScope* parent_; |
+ TemporaryRegisterScope allocator_; |
+ bool result_identified_; |
+ DISALLOW_COPY_AND_ASSIGN(ExpressionResultScope); |
+}; |
+ |
+ |
+// Scoped class used for the top-level management of an expression scope. |
+// Specifically mapping registers across an expresion. |
rmcilroy
2015/10/19 12:56:22
expression / expresion (although this class should
oth
2015/10/20 15:28:52
Done.
|
+class BytecodeGenerator::TopLevelResultScope final |
+ : public ExpressionResultScope { |
+ public: |
+ explicit TopLevelResultScope(BytecodeGenerator* generator) |
+ : ExpressionResultScope(generator) { |
+ generator->set_top_level_result_scope(this); |
+ set_result_identified(); |
+ } |
+ virtual ~TopLevelResultScope() { |
rmcilroy
2015/10/19 12:56:22
nit newline above
oth
2015/10/20 15:28:52
Class removed.
|
+ generator()->set_top_level_result_scope(nullptr); |
+ } |
+ |
+ virtual void SetResultInAccumulator() { set_result_identified(); } |
+ virtual void SetResultInRegister(Register reg) { set_result_identified(); } |
+}; |
+ |
+ |
+// Scoped class used when the result of the current expression to be |
+// evaluated should go into the interpreter's accumulator register. |
+class BytecodeGenerator::AccumulatorResultScope final |
+ : public ExpressionResultScope { |
+ public: |
+ explicit AccumulatorResultScope(BytecodeGenerator* generator) |
+ : ExpressionResultScope(generator) {} |
+ virtual void SetResultInAccumulator() { |
+ if (!result_identified()) { |
+ // TODO(oth): REVIEW. |
rmcilroy
2015/10/19 12:56:22
Can we remove the "if (!result_identified())" now?
oth
2015/10/20 15:28:52
Done.
|
+ set_result_identified(); |
+ } |
+ } |
+ virtual void SetResultInRegister(Register reg) { |
rmcilroy
2015/10/19 12:56:23
nit - newline above
oth
2015/10/20 15:28:52
Done.
|
+ builder()->LoadAccumulatorWithRegister(reg); |
+ set_result_identified(); |
+ } |
+}; |
+ |
+ |
+// Scoped class used when the result of the current expression is not |
+// expected to produce a result. |
+class BytecodeGenerator::EffectResultScope final |
+ : public ExpressionResultScope { |
+ public: |
+ explicit EffectResultScope(BytecodeGenerator* generator) |
+ : ExpressionResultScope(generator) { |
+ set_result_identified(); |
+ } |
+ virtual void SetResultInAccumulator() {} |
+ virtual void SetResultInRegister(Register reg) {} |
+}; |
rmcilroy
2015/10/19 12:56:22
nit - could you move this above BytecodeGenerator:
oth
2015/10/20 15:28:52
Done.
|
+ |
+ |
+// Scoped class used when the result of the current expression to be |
+// evaluated should go into an interpreter register. |
+class BytecodeGenerator::RegisterResultScope final |
+ : public ExpressionResultScope { |
+ public: |
+ explicit RegisterResultScope(BytecodeGenerator* generator) |
+ : ExpressionResultScope(generator) {} |
+ ~RegisterResultScope() {} |
+ |
+ virtual void SetResultInAccumulator() { |
+ result_register_ = parent()->NewRegister(); |
+ builder()->StoreAccumulatorInRegister(result_register_); |
+ set_result_identified(); |
+ } |
+ |
+ virtual void SetResultInRegister(Register reg) { |
+ result_register_ = reg; |
+ set_result_identified(); |
+ } |
+ |
+ Register ResultRegister() const { return result_register_; } |
+ |
+ private: |
+ Register result_register_; |
+}; |
+ |
+ |
BytecodeGenerator::BytecodeGenerator(Isolate* isolate, Zone* zone) |
: isolate_(isolate), |
zone_(zone), |
@@ -154,7 +278,11 @@ BytecodeGenerator::BytecodeGenerator(Isolate* isolate, Zone* zone) |
scope_(nullptr), |
globals_(0, zone), |
execution_control_(nullptr), |
- execution_context_(nullptr) { |
+ execution_context_(nullptr), |
+ result_scope_(nullptr), |
+ top_level_result_scope_(nullptr), |
+ binary_expression_depth_(0), |
+ binary_expression_hazard_set_(zone) { |
InitializeAstVisitor(isolate); |
} |
@@ -162,6 +290,35 @@ BytecodeGenerator::BytecodeGenerator(Isolate* isolate, Zone* zone) |
BytecodeGenerator::~BytecodeGenerator() {} |
+void BytecodeGenerator::set_result_scope(ExpressionResultScope* result_scope) { |
+ result_scope_ = result_scope; |
rmcilroy
2015/10/19 12:56:22
nit - just inline this in header
oth
2015/10/20 15:28:52
Done.
|
+} |
+ |
+ |
+BytecodeGenerator::ExpressionResultScope* BytecodeGenerator::result_scope() |
rmcilroy
2015/10/19 12:56:23
ditto
oth
2015/10/20 15:28:52
Done.
|
+ const { |
+ return result_scope_; |
+} |
+ |
+ |
+BytecodeGenerator::ExpressionResultScope* |
+BytecodeGenerator::parent_result_scope() const { |
rmcilroy
2015/10/19 12:56:22
Unused, please remove (callers could just do resul
oth
2015/10/20 15:28:52
Done.
|
+ return result_scope_->parent(); |
+} |
+ |
+void BytecodeGenerator::set_top_level_result_scope( |
+ TopLevelResultScope* result_scope) { |
+ DCHECK((result_scope == nullptr && top_level_result_scope_ != nullptr) || |
+ (result_scope != nullptr && top_level_result_scope_ == nullptr)); |
+ top_level_result_scope_ = result_scope; |
+} |
+ |
+BytecodeGenerator::TopLevelResultScope* |
+BytecodeGenerator::top_level_result_scope() const { |
+ return top_level_result_scope_; |
+} |
+ |
+ |
Handle<BytecodeArray> BytecodeGenerator::MakeBytecode(CompilationInfo* info) { |
set_info(info); |
set_scope(info->scope()); |
@@ -324,12 +481,12 @@ void BytecodeGenerator::VisitDeclarations( |
void BytecodeGenerator::VisitExpressionStatement(ExpressionStatement* stmt) { |
- Visit(stmt->expression()); |
+ TopLevelResultScope result_scope(this); |
+ VisitForAccumulatorValue(stmt->expression()); |
rmcilroy
2015/10/19 12:56:23
VisitForEffect here I think? Following statements
oth
2015/10/20 15:28:53
Done.
|
} |
void BytecodeGenerator::VisitEmptyStatement(EmptyStatement* stmt) { |
- // TODO(oth): For control-flow it could be useful to signal empty paths here. |
} |
@@ -340,7 +497,7 @@ void BytecodeGenerator::VisitIfStatement(IfStatement* stmt) { |
BytecodeLabel else_label, end_label; |
- Visit(stmt->condition()); |
+ VisitForAccumulatorValue(stmt->condition()); |
builder()->CastAccumulatorToBoolean(); |
builder()->JumpIfFalse(&else_label); |
Visit(stmt->then_statement()); |
@@ -372,7 +529,8 @@ void BytecodeGenerator::VisitBreakStatement(BreakStatement* stmt) { |
void BytecodeGenerator::VisitReturnStatement(ReturnStatement* stmt) { |
- Visit(stmt->expression()); |
+ TopLevelResultScope scope(this); |
rmcilroy
2015/10/19 12:56:22
Given my comment above about removing TopLevelResu
oth
2015/10/20 15:28:53
Ack. Removed for now, the newer name is fine.
|
+ VisitForAccumulatorValue(stmt->expression()); |
builder()->Return(); |
} |
@@ -398,7 +556,7 @@ void BytecodeGenerator::VisitDoWhileStatement(DoWhileStatement* stmt) { |
builder()->Bind(&body_label); |
Visit(stmt->body()); |
builder()->Bind(&condition_label); |
- Visit(stmt->cond()); |
+ VisitForAccumulatorValue(stmt->cond()); |
builder()->JumpIfTrue(&body_label); |
builder()->Bind(&done_label); |
@@ -416,7 +574,7 @@ void BytecodeGenerator::VisitWhileStatement(WhileStatement* stmt) { |
builder()->Bind(&body_label); |
Visit(stmt->body()); |
builder()->Bind(&condition_label); |
- Visit(stmt->cond()); |
+ VisitForAccumulatorValue(stmt->cond()); |
builder()->JumpIfTrue(&body_label); |
builder()->Bind(&done_label); |
@@ -445,7 +603,7 @@ void BytecodeGenerator::VisitForStatement(ForStatement* stmt) { |
} |
if (stmt->cond()) { |
builder()->Bind(&condition_label); |
- Visit(stmt->cond()); |
+ VisitForAccumulatorValue(stmt->cond()); |
builder()->JumpIfTrue(&body_label); |
} else { |
builder()->Jump(&body_label); |
@@ -500,6 +658,7 @@ void BytecodeGenerator::VisitFunctionLiteral(FunctionLiteral* expr) { |
builder() |
->LoadLiteral(shared_info) |
.CreateClosure(expr->pretenure() ? TENURED : NOT_TENURED); |
+ result_scope()->SetResultInAccumulator(); |
} |
@@ -534,6 +693,7 @@ void BytecodeGenerator::VisitLiteral(Literal* expr) { |
} else { |
builder()->LoadLiteral(value); |
} |
+ result_scope()->SetResultInAccumulator(); |
} |
@@ -546,6 +706,7 @@ void BytecodeGenerator::VisitRegExpLiteral(RegExpLiteral* expr) { |
.StoreAccumulatorInRegister(flags) |
.LoadLiteral(expr->pattern()) |
.CreateRegExpLiteral(expr->literal_index(), flags); |
+ result_scope()->SetResultInAccumulator(); |
} |
@@ -590,21 +751,24 @@ void BytecodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) { |
builder() |
->LoadLiteral(literal_key->AsPropertyName()) |
.StoreAccumulatorInRegister(name); |
- Visit(property->value()); |
+ VisitForAccumulatorValue(property->value()); |
builder()->StoreNamedProperty(literal, name, |
feedback_index(property->GetSlot(0)), |
language_mode()); |
} else { |
- Visit(property->value()); |
+ VisitForAccumulatorValue(property->value()); |
rmcilroy
2015/10/19 12:56:22
VisitForEffect here
oth
2015/10/20 15:28:53
Done.
|
} |
} else { |
Register key = inner_temporary_register_scope.NewRegister(); |
Register value = inner_temporary_register_scope.NewRegister(); |
Register language = inner_temporary_register_scope.NewRegister(); |
+ // TODO(oth): This is problematic - can't assume contiguous here. |
+ // literal is allocated in temporary_register_scope, whereas |
+ // key, value, language are in another. |
rmcilroy
2015/10/19 12:56:22
Can we use PrepareForConsecutiveAllocations at the
oth
2015/10/20 15:28:53
Done.
|
DCHECK(Register::AreContiguous(literal, key, value, language)); |
- Visit(property->key()); |
+ VisitForAccumulatorValue(property->key()); |
builder()->StoreAccumulatorInRegister(key); |
- Visit(property->value()); |
+ VisitForAccumulatorValue(property->value()); |
builder()->StoreAccumulatorInRegister(value); |
if (property->emit_store()) { |
builder() |
@@ -620,7 +784,7 @@ void BytecodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) { |
DCHECK(property->emit_store()); |
Register value = inner_temporary_register_scope.NewRegister(); |
DCHECK(Register::AreContiguous(literal, value)); |
- Visit(property->value()); |
+ VisitForAccumulatorValue(property->value()); |
builder()->StoreAccumulatorInRegister(value).CallRuntime( |
Runtime::kInternalSetPrototype, literal, 2); |
break; |
@@ -648,7 +812,7 @@ void BytecodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) { |
Register setter = inner_temporary_register_scope.NewRegister(); |
Register attr = inner_temporary_register_scope.NewRegister(); |
DCHECK(Register::AreContiguous(literal, name, getter, setter, attr)); |
- Visit(it->first); |
+ VisitForAccumulatorValue(it->first); |
builder()->StoreAccumulatorInRegister(name); |
VisitObjectLiteralAccessor(literal, it->second->getter, getter); |
VisitObjectLiteralAccessor(literal, it->second->setter, setter); |
@@ -671,6 +835,7 @@ void BytecodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) { |
ObjectLiteral::Property* property = expr->properties()->at(property_index); |
if (literal_in_accumulator) { |
+ temporary_register_scope.PrepareForConsecutiveAllocations(4); |
literal = temporary_register_scope.NewRegister(); |
builder()->StoreAccumulatorInRegister(literal); |
literal_in_accumulator = false; |
@@ -681,21 +846,22 @@ void BytecodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) { |
TemporaryRegisterScope inner_temporary_register_scope(builder()); |
Register value = inner_temporary_register_scope.NewRegister(); |
DCHECK(Register::AreContiguous(literal, value)); |
- Visit(property->value()); |
+ VisitForAccumulatorValue(property->value()); |
builder()->StoreAccumulatorInRegister(value).CallRuntime( |
Runtime::kInternalSetPrototype, literal, 2); |
continue; |
} |
TemporaryRegisterScope inner_temporary_register_scope(builder()); |
+ inner_temporary_register_scope.PrepareForConsecutiveAllocations(3); |
Register key = inner_temporary_register_scope.NewRegister(); |
Register value = inner_temporary_register_scope.NewRegister(); |
Register attr = inner_temporary_register_scope.NewRegister(); |
DCHECK(Register::AreContiguous(literal, key, value, attr)); |
- Visit(property->key()); |
+ VisitForAccumulatorValue(property->key()); |
builder()->CastAccumulatorToName().StoreAccumulatorInRegister(key); |
- Visit(property->value()); |
+ VisitForAccumulatorValue(property->value()); |
builder()->StoreAccumulatorInRegister(value); |
VisitSetHomeObject(value, literal, property); |
builder()->LoadLiteral(Smi::FromInt(NONE)).StoreAccumulatorInRegister(attr); |
@@ -729,6 +895,7 @@ void BytecodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) { |
// Restore literal array into accumulator. |
builder()->LoadAccumulatorWithRegister(literal); |
} |
+ result_scope()->SetResultInAccumulator(); |
} |
@@ -763,7 +930,7 @@ void BytecodeGenerator::VisitArrayLiteral(ArrayLiteral* expr) { |
builder() |
->LoadLiteral(Smi::FromInt(array_index)) |
.StoreAccumulatorInRegister(index); |
- Visit(subexpr); |
+ VisitForAccumulatorValue(subexpr); |
FeedbackVectorSlot slot = expr->LiteralFeedbackSlot(); |
rmcilroy
2015/10/19 12:56:22
nit - (I know this was my code..) could you move t
oth
2015/10/20 15:28:52
Done.
|
builder()->StoreKeyedProperty(literal, index, feedback_index(slot), |
language_mode()); |
@@ -773,6 +940,7 @@ void BytecodeGenerator::VisitArrayLiteral(ArrayLiteral* expr) { |
// Restore literal array into accumulator. |
builder()->LoadAccumulatorWithRegister(literal); |
} |
+ result_scope()->SetResultInAccumulator(); |
} |
@@ -785,17 +953,15 @@ void BytecodeGenerator::VisitVariableLoad(Variable* variable, |
FeedbackVectorSlot slot) { |
switch (variable->location()) { |
case VariableLocation::LOCAL: { |
- Register source(variable->index()); |
- builder()->LoadAccumulatorWithRegister(source); |
- // TODO(rmcilroy): Perform check for uninitialized legacy const, const and |
- // let variables. |
+ Register source(Register(variable->index())); |
+ result_scope()->SetResultInRegister(source); |
break; |
} |
case VariableLocation::PARAMETER: { |
// The parameter indices are shifted by 1 (receiver is variable |
// index -1 but is parameter index 0 in BytecodeArrayBuilder). |
- Register source(builder()->Parameter(variable->index() + 1)); |
- builder()->LoadAccumulatorWithRegister(source); |
+ Register source = builder()->Parameter(variable->index() + 1); |
+ result_scope()->SetResultInRegister(source); |
break; |
} |
case VariableLocation::GLOBAL: { |
@@ -805,6 +971,7 @@ void BytecodeGenerator::VisitVariableLoad(Variable* variable, |
// runtime. |
DCHECK(variable->IsStaticGlobalObjectProperty()); |
builder()->LoadGlobal(variable->index()); |
+ result_scope()->SetResultInAccumulator(); |
break; |
} |
case VariableLocation::UNALLOCATED: { |
@@ -815,6 +982,7 @@ void BytecodeGenerator::VisitVariableLoad(Variable* variable, |
builder()->StoreAccumulatorInRegister(obj); |
builder()->LoadLiteral(variable->name()); |
builder()->LoadNamedProperty(obj, feedback_index(slot), language_mode()); |
+ result_scope()->SetResultInAccumulator(); |
break; |
} |
case VariableLocation::CONTEXT: { |
@@ -824,6 +992,7 @@ void BytecodeGenerator::VisitVariableLoad(Variable* variable, |
} else { |
UNIMPLEMENTED(); |
} |
+ result_scope()->SetResultInAccumulator(); |
// TODO(rmcilroy): Perform check for uninitialized legacy const, const and |
// let variables. |
break; |
@@ -841,6 +1010,7 @@ void BytecodeGenerator::VisitVariableAssignment(Variable* variable, |
// TODO(rmcilroy): support const mode initialization. |
Register destination(variable->index()); |
builder()->StoreAccumulatorInRegister(destination); |
+ StoreForBinaryExpression(destination); |
rmcilroy
2015/10/19 12:56:22
I don't like this being called StoreForBinaryExpre
oth
2015/10/20 15:28:52
RecordStoreToRegister()? It'll certainly cover par
rmcilroy
2015/10/20 16:39:20
Good point, SGTM
|
break; |
} |
case VariableLocation::PARAMETER: { |
@@ -848,6 +1018,7 @@ void BytecodeGenerator::VisitVariableAssignment(Variable* variable, |
// index -1 but is parameter index 0 in BytecodeArrayBuilder). |
Register destination(builder()->Parameter(variable->index() + 1)); |
builder()->StoreAccumulatorInRegister(destination); |
+ StoreForBinaryExpression(destination); |
break; |
} |
case VariableLocation::GLOBAL: { |
@@ -860,10 +1031,10 @@ void BytecodeGenerator::VisitVariableAssignment(Variable* variable, |
break; |
} |
case VariableLocation::UNALLOCATED: { |
- TemporaryRegisterScope temporary_register_scope(builder()); |
- Register value = temporary_register_scope.NewRegister(); |
- Register obj = temporary_register_scope.NewRegister(); |
- Register name = temporary_register_scope.NewRegister(); |
+ Register value = result_scope()->NewRegister(); |
+ Register obj = result_scope()->NewRegister(); |
+ Register name = result_scope()->NewRegister(); |
+ |
// TODO(rmcilroy): Investigate whether we can avoid having to stash the |
// value in a register. |
builder()->StoreAccumulatorInRegister(value); |
@@ -895,7 +1066,6 @@ void BytecodeGenerator::VisitVariableAssignment(Variable* variable, |
void BytecodeGenerator::VisitAssignment(Assignment* expr) { |
DCHECK(expr->target()->IsValidReferenceExpression()); |
- TemporaryRegisterScope temporary_register_scope(builder()); |
Register object, key; |
// Left-hand side can only be a property, a global or a variable slot. |
@@ -907,22 +1077,18 @@ void BytecodeGenerator::VisitAssignment(Assignment* expr) { |
case VARIABLE: |
// Nothing to do to evaluate variable assignment LHS. |
break; |
- case NAMED_PROPERTY: |
- object = temporary_register_scope.NewRegister(); |
- key = temporary_register_scope.NewRegister(); |
- Visit(property->obj()); |
- builder()->StoreAccumulatorInRegister(object); |
+ case NAMED_PROPERTY: { |
+ object = VisitForRegisterValue(property->obj()); |
+ key = result_scope()->NewRegister(); |
builder()->LoadLiteral(property->key()->AsLiteral()->AsPropertyName()); |
builder()->StoreAccumulatorInRegister(key); |
break; |
- case KEYED_PROPERTY: |
- object = temporary_register_scope.NewRegister(); |
- key = temporary_register_scope.NewRegister(); |
- Visit(property->obj()); |
- builder()->StoreAccumulatorInRegister(object); |
- Visit(property->key()); |
- builder()->StoreAccumulatorInRegister(key); |
+ } |
+ case KEYED_PROPERTY: { |
+ object = VisitForRegisterValue(property->obj()); |
+ key = VisitForRegisterValue(property->key()); |
break; |
+ } |
case NAMED_SUPER_PROPERTY: |
case KEYED_SUPER_PROPERTY: |
UNIMPLEMENTED(); |
@@ -933,7 +1099,8 @@ void BytecodeGenerator::VisitAssignment(Assignment* expr) { |
if (expr->is_compound()) { |
UNIMPLEMENTED(); |
} else { |
- Visit(expr->value()); |
+ VisitForAccumulatorValue(expr->value()); |
+ result_scope()->SetResultInAccumulator(); |
} |
// Store the value. |
@@ -974,11 +1141,13 @@ void BytecodeGenerator::VisitPropertyLoad(Register obj, Property* expr) { |
case NAMED_PROPERTY: { |
builder()->LoadLiteral(expr->key()->AsLiteral()->AsPropertyName()); |
builder()->LoadNamedProperty(obj, feedback_index(slot), language_mode()); |
+ result_scope()->SetResultInAccumulator(); |
break; |
} |
case KEYED_PROPERTY: { |
- Visit(expr->key()); |
+ VisitForAccumulatorValue(expr->key()); |
builder()->LoadKeyedProperty(obj, feedback_index(slot), language_mode()); |
+ result_scope()->SetResultInAccumulator(); |
break; |
} |
case NAMED_SUPER_PROPERTY: |
@@ -989,25 +1158,21 @@ void BytecodeGenerator::VisitPropertyLoad(Register obj, Property* expr) { |
void BytecodeGenerator::VisitProperty(Property* expr) { |
- TemporaryRegisterScope temporary_register_scope(builder()); |
- Register obj = temporary_register_scope.NewRegister(); |
- Visit(expr->obj()); |
- builder()->StoreAccumulatorInRegister(obj); |
+ Register obj = VisitForRegisterValue(expr->obj()); |
VisitPropertyLoad(obj, expr); |
} |
-Register BytecodeGenerator::VisitArguments( |
- ZoneList<Expression*>* args, TemporaryRegisterScope* register_scope) { |
+Register BytecodeGenerator::VisitArguments(ZoneList<Expression*>* args) { |
// Visit arguments and place in a contiguous block of temporary registers. |
// Return the first temporary register corresponding to the first argument. |
DCHECK_GT(args->length(), 0); |
- Register first_arg = register_scope->NewRegister(); |
- Visit(args->at(0)); |
+ Register first_arg = result_scope()->NewRegister(); |
+ VisitForAccumulatorValue(args->at(0)); |
builder()->StoreAccumulatorInRegister(first_arg); |
for (int i = 1; i < static_cast<int>(args->length()); i++) { |
- Register ith_arg = register_scope->NewRegister(); |
- Visit(args->at(i)); |
+ Register ith_arg = result_scope()->NewRegister(); |
+ VisitForAccumulatorValue(args->at(i)); |
builder()->StoreAccumulatorInRegister(ith_arg); |
DCHECK(ith_arg.index() - i == first_arg.index()); |
} |
@@ -1017,14 +1182,21 @@ Register BytecodeGenerator::VisitArguments( |
void BytecodeGenerator::VisitCall(Call* expr) { |
+ static int hits = 0; |
+ hits++; |
rmcilroy
2015/10/19 12:56:23
leftover debugging code?
oth
2015/10/20 15:28:53
Done.
|
Expression* callee_expr = expr->expression(); |
Call::CallType call_type = expr->GetCallType(isolate()); |
// Prepare the callee and the receiver to the function call. This depends on |
// the semantics of the underlying call type. |
- TemporaryRegisterScope temporary_register_scope(builder()); |
- Register callee = temporary_register_scope.NewRegister(); |
- Register receiver = temporary_register_scope.NewRegister(); |
+ Register callee = result_scope()->NewRegister(); |
+ |
+ // The receiver and arguments need to be allocated consecutively for |
+ // Call(). Future optimizations could avoid this there are no |
+ // arguments or the receiver and arguments are already consecutive. |
+ ZoneList<Expression*>* args = expr->arguments(); |
+ result_scope()->PrepareForConsecutiveAllocations(args->length() + 1); |
+ Register receiver = result_scope()->NewRegister(); |
switch (call_type) { |
case Call::PROPERTY_CALL: { |
@@ -1032,8 +1204,11 @@ void BytecodeGenerator::VisitCall(Call* expr) { |
if (property->IsSuperAccess()) { |
UNIMPLEMENTED(); |
} |
- Visit(property->obj()); |
+ VisitForAccumulatorValue(property->obj()); |
builder()->StoreAccumulatorInRegister(receiver); |
+ // Need a result scope here to keep our consecutive |
+ // temporaries. |
+ AccumulatorResultScope accumulator_result_scope(this); |
// Perform a property load of the callee. |
VisitPropertyLoad(receiver, property); |
builder()->StoreAccumulatorInRegister(callee); |
@@ -1050,7 +1225,7 @@ void BytecodeGenerator::VisitCall(Call* expr) { |
} |
case Call::OTHER_CALL: { |
builder()->LoadUndefined().StoreAccumulatorInRegister(receiver); |
- Visit(callee_expr); |
+ VisitForAccumulatorValue(callee_expr); |
builder()->StoreAccumulatorInRegister(callee); |
break; |
} |
@@ -1062,26 +1237,30 @@ void BytecodeGenerator::VisitCall(Call* expr) { |
// Evaluate all arguments to the function call and store in sequential |
// registers. |
- ZoneList<Expression*>* args = expr->arguments(); |
- if (args->length() > 0) { |
- Register first_arg = VisitArguments(args, &temporary_register_scope); |
- CHECK_EQ(first_arg.index(), receiver.index() + 1); |
+ result_scope()->PrepareForConsecutiveAllocations(args->length()); |
rmcilroy
2015/10/19 12:56:22
nit - could we move PrepareForConsecutiveAllocatio
oth
2015/10/20 15:28:52
Done.
oth
2015/10/20 15:28:52
Done.
|
+ for (int i = 0; i < args->length(); ++i) { |
+ VisitForAccumulatorValue(args->at(i)); |
+ Register arg = result_scope()->NewRegister(); |
+ DCHECK(arg.index() - i == receiver.index() + 1); |
rmcilroy
2015/10/19 12:56:22
nit - move Register arg and DCHECK lines above Vis
oth
2015/10/20 15:28:52
Done.
|
+ builder()->StoreAccumulatorInRegister(arg); |
} |
// TODO(rmcilroy): Deal with possible direct eval here? |
// TODO(rmcilroy): Use CallIC to allow call type feedback. |
builder()->Call(callee, receiver, args->length()); |
+ result_scope()->SetResultInAccumulator(); |
} |
void BytecodeGenerator::VisitCallNew(CallNew* expr) { |
- TemporaryRegisterScope temporary_register_scope(builder()); |
- Register constructor = temporary_register_scope.NewRegister(); |
- Visit(expr->expression()); |
+ Register constructor = result_scope()->NewRegister(); |
+ VisitForAccumulatorValue(expr->expression()); |
builder()->StoreAccumulatorInRegister(constructor); |
+ |
ZoneList<Expression*>* args = expr->arguments(); |
if (args->length() > 0) { |
- Register first_arg = VisitArguments(args, &temporary_register_scope); |
+ result_scope()->PrepareForConsecutiveAllocations(args->length()); |
+ Register first_arg = VisitArguments(args); |
builder()->New(constructor, first_arg, args->length()); |
} else { |
// The second argument here will be ignored as there are zero |
@@ -1089,6 +1268,8 @@ void BytecodeGenerator::VisitCallNew(CallNew* expr) { |
// allocating a temporary just to fill the operands. |
builder()->New(constructor, constructor, 0); |
} |
+ |
+ result_scope()->SetResultInAccumulator(); |
} |
@@ -1098,40 +1279,43 @@ void BytecodeGenerator::VisitCallRuntime(CallRuntime* expr) { |
} |
// Evaluate all arguments to the runtime call. |
- TemporaryRegisterScope temporary_register_scope(&builder_); |
- |
- // TODO(rmcilroy): support multiple return values. |
DCHECK_LE(expr->function()->result_size, 1); |
Runtime::FunctionId function_id = expr->function()->function_id; |
ZoneList<Expression*>* args = expr->arguments(); |
+ result_scope()->PrepareForConsecutiveAllocations(args->length()); |
Register first_arg; |
if (args->length() > 0) { |
- first_arg = VisitArguments(args, &temporary_register_scope); |
+ first_arg = VisitArguments(args); |
} else { |
// Allocation here is just to fullfil the requirement that there |
// is a register operand for the start of the arguments though |
// there are zero when this is generated. |
- first_arg = temporary_register_scope.NewRegister(); |
+ first_arg = result_scope()->NewRegister(); |
} |
builder()->CallRuntime(function_id, first_arg, args->length()); |
+ // TODO(rmcilroy): support multiple return values. |
rmcilroy
2015/10/19 12:56:22
nit - move TODO back up to DCHECK_LE (that is what
oth
2015/10/20 15:28:52
Done.
|
+ result_scope()->SetResultInAccumulator(); |
} |
void BytecodeGenerator::VisitVoid(UnaryOperation* expr) { |
- Visit(expr->expression()); |
+ VisitForEffect(expr->expression()); |
builder()->LoadUndefined(); |
+ result_scope()->SetResultInAccumulator(); |
} |
void BytecodeGenerator::VisitTypeOf(UnaryOperation* expr) { |
- Visit(expr->expression()); |
+ VisitForAccumulatorValue(expr->expression()); |
builder()->TypeOf(); |
+ result_scope()->SetResultInAccumulator(); |
} |
void BytecodeGenerator::VisitNot(UnaryOperation* expr) { |
- Visit(expr->expression()); |
+ VisitForAccumulatorValue(expr->expression()); |
builder()->LogicalNot(); |
+ result_scope()->SetResultInAccumulator(); |
} |
@@ -1179,17 +1363,20 @@ void BytecodeGenerator::VisitBinaryOperation(BinaryOperation* binop) { |
void BytecodeGenerator::VisitCompareOperation(CompareOperation* expr) { |
- Token::Value op = expr->op(); |
- Expression* left = expr->left(); |
- Expression* right = expr->right(); |
- |
- TemporaryRegisterScope temporary_register_scope(builder()); |
- Register temporary = temporary_register_scope.NewRegister(); |
- |
- Visit(left); |
- builder()->StoreAccumulatorInRegister(temporary); |
- Visit(right); |
- builder()->CompareOperation(op, temporary, language_mode_strength()); |
+ // This is a potentially two pass operation if the first pass |
+ // detects loads-and-stores to locals and params used in the |
+ // expression. This is hopefully rare. |
+ PrepareForBinaryExpression(); |
rmcilroy
2015/10/19 12:56:22
As mentioned, this should probably be a scope allo
oth
2015/10/20 15:28:52
Done.
|
+ Register lhs = VisitForRegisterValue(expr->left()); |
+ if (builder()->RegisterIsParameterOrLocal(lhs)) { |
+ // Result was returned in an existing local or parameter. See if |
+ // it needs to be moved to a temporary. |
+ lhs = LoadForBinaryExpression(lhs); |
+ } |
+ VisitForAccumulatorValue(expr->right()); |
+ builder()->CompareOperation(expr->op(), lhs, language_mode_strength()); |
+ result_scope()->SetResultInAccumulator(); |
+ CompleteBinaryExpression(); |
} |
@@ -1281,18 +1468,82 @@ void BytecodeGenerator::VisitNewLocalBlockContext(Scope* scope) { |
} |
-void BytecodeGenerator::VisitArithmeticExpression(BinaryOperation* binop) { |
- Token::Value op = binop->op(); |
- Expression* left = binop->left(); |
- Expression* right = binop->right(); |
+void BytecodeGenerator::PrepareForBinaryExpression() { |
+ if (binary_expression_depth_++ == 0) { |
+ binary_expression_hazard_set_.clear(); |
+ } |
+} |
+ |
+ |
+Register BytecodeGenerator::LoadForBinaryExpression(Register reg) { |
rmcilroy
2015/10/19 12:56:22
I think this should be done in VisitVariableLoad a
oth
2015/10/20 15:28:53
The method has been renamed and there's a TODO to
|
+ // TODO(oth): Follow on CL to load from re-map here. |
+ DCHECK(builder()->RegisterIsParameterOrLocal(reg)); |
+ if (binary_expression_depth_ > 0) { |
+ binary_expression_hazard_set_.insert(reg.index()); |
+ } |
+ return reg; |
+} |
+ |
+ |
+void BytecodeGenerator::StoreForBinaryExpression(Register reg) { |
+ DCHECK(builder()->RegisterIsParameterOrLocal(reg)); |
+ if (binary_expression_depth_ > 0) { |
+ // TODO(oth): a store to a register that's be loaded needs to be |
+ // remapped. |
+ DCHECK(binary_expression_hazard_set_.find(reg.index()) == |
+ binary_expression_hazard_set_.end()); |
+ } |
+} |
- TemporaryRegisterScope temporary_register_scope(builder()); |
- Register temporary = temporary_register_scope.NewRegister(); |
- Visit(left); |
- builder()->StoreAccumulatorInRegister(temporary); |
- Visit(right); |
- builder()->BinaryOperation(op, temporary, language_mode_strength()); |
+void BytecodeGenerator::CompleteBinaryExpression() { |
+ DCHECK(binary_expression_depth_ > 0); |
+ binary_expression_depth_ -= 1; |
+ // TODO(oth): spill remapped registers into origins. |
+} |
+ |
+ |
+void BytecodeGenerator::VisitArithmeticExpression(BinaryOperation* expr) { |
+ // This is a potentially two pass operation if the first pass |
+ // detects loads-and-stores to locals and params used in the |
+ // expression. This is hopefully rare. |
rmcilroy
2015/10/19 12:56:22
Comment is out-of-date. Alsoc could we move this b
oth
2015/10/20 15:28:52
Done.
|
+ PrepareForBinaryExpression(); |
+ Register lhs = VisitForRegisterValue(expr->left()); |
+ if (builder()->RegisterIsParameterOrLocal(lhs)) { |
+ // Result was returned in an existing local or parameter. See if |
+ // it needs to be moved to a temporary. |
+ lhs = LoadForBinaryExpression(lhs); |
+ } |
+ VisitForAccumulatorValue(expr->right()); |
+ builder()->BinaryOperation(expr->op(), lhs, language_mode_strength()); |
+ result_scope()->SetResultInAccumulator(); |
+ CompleteBinaryExpression(); |
+} |
+ |
+ |
+// Visits the expression |expr| and places the result in the accumulator. |
+void BytecodeGenerator::VisitForAccumulatorValue(Expression* expr) { |
+ AccumulatorResultScope accumulator_scope(this); |
+ Visit(expr); |
+} |
+ |
+ |
+// Visits the expression |expr| and discards the result. |
+void BytecodeGenerator::VisitForEffect(Expression* expr) { |
+ EffectResultScope effect_scope(this); |
+ Visit(expr); |
+} |
+ |
+ |
+// Visits the expression |expr| and places the result in |temporary_register| |
+// if the result is not already a register value. The return value is |
+// the register containing the result. The caller should check whether |
+// this is the temporary register and consider releasing the temporary |
+// register if not. |
+Register BytecodeGenerator::VisitForRegisterValue(Expression* expr) { |
+ RegisterResultScope register_scope(this); |
+ Visit(expr); |
+ return register_scope.ResultRegister(); |
} |
@@ -1300,8 +1551,9 @@ void BytecodeGenerator::VisitCommaExpression(BinaryOperation* binop) { |
Expression* left = binop->left(); |
Expression* right = binop->right(); |
- Visit(left); |
- Visit(right); |
+ VisitForAccumulatorValue(left); |
rmcilroy
2015/10/19 12:56:22
this can be VisitForEffect
oth
2015/10/20 15:28:53
Yes, and interestingly the RHS visitor should just
rmcilroy
2015/10/20 16:39:20
Nice catch :).
|
+ VisitForAccumulatorValue(right); |
+ result_scope()->SetResultInAccumulator(); |
} |
@@ -1312,15 +1564,15 @@ void BytecodeGenerator::VisitLogicalOrExpression(BinaryOperation* binop) { |
// Short-circuit evaluation- If it is known that left is always true, |
// no need to visit right |
if (left->ToBooleanIsTrue()) { |
- Visit(left); |
+ VisitForAccumulatorValue(left); |
} else { |
BytecodeLabel end_label; |
- |
- Visit(left); |
+ VisitForAccumulatorValue(left); |
builder()->JumpIfToBooleanTrue(&end_label); |
- Visit(right); |
+ VisitForAccumulatorValue(right); |
builder()->Bind(&end_label); |
} |
+ result_scope()->SetResultInAccumulator(); |
} |
@@ -1331,15 +1583,15 @@ void BytecodeGenerator::VisitLogicalAndExpression(BinaryOperation* binop) { |
// Short-circuit evaluation- If it is known that left is always false, |
// no need to visit right |
if (left->ToBooleanIsFalse()) { |
- Visit(left); |
+ VisitForAccumulatorValue(left); |
} else { |
BytecodeLabel end_label; |
- |
- Visit(left); |
+ VisitForAccumulatorValue(left); |
builder()->JumpIfToBooleanFalse(&end_label); |
- Visit(right); |
+ VisitForAccumulatorValue(right); |
builder()->Bind(&end_label); |
} |
+ result_scope()->SetResultInAccumulator(); |
} |
@@ -1349,7 +1601,7 @@ void BytecodeGenerator::VisitObjectLiteralAccessor( |
if (property == nullptr) { |
builder()->LoadNull().StoreAccumulatorInRegister(value_out); |
} else { |
- Visit(property->value()); |
+ VisitForAccumulatorValue(property->value()); |
builder()->StoreAccumulatorInRegister(value_out); |
VisitSetHomeObject(value_out, home_object, property); |
} |