Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(133)

Unified Diff: src/interpreter/bytecode-generator.cc

Issue 1392933002: [Interpreter] Reduce temporary register usage in generated bytecode. (Closed) Base URL: https://chromium.googlesource.com/v8/v8.git@master
Patch Set: Missed comments. Created 5 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: src/interpreter/bytecode-generator.cc
diff --git a/src/interpreter/bytecode-generator.cc b/src/interpreter/bytecode-generator.cc
index 309b9294297674b1b6ff75e4214df11865624620..9ceab67ab4f6b658a26fb3f00dc75add2b6ec3a6 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,118 @@ 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),
+ outer_(generator->result_scope()),
+ allocator_(builder()),
+ result_identified_(false) {
+ generator_->set_result_scope(this);
+ }
+
+ virtual ~ExpressionResultScope() {
+ generator_->set_result_scope(outer_);
+ 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* outer() const { return outer_; }
+
+ Register NewRegister() { return allocator_.NewRegister(); }
+
+ void PrepareForConsecutiveAllocations(size_t count) {
+ allocator_.PrepareForConsecutiveAllocations(count);
+ }
+
+ Register NextConsecutiveRegister() {
+ return allocator_.NextConsecutiveRegister();
+ }
+
+ protected:
+ void set_result_identified() {
+ DCHECK(!result_identified());
+ result_identified_ = true;
+ }
+
+ bool result_identified() const { return result_identified_; }
+
+ private:
+ BytecodeGenerator* generator_;
+ ExpressionResultScope* outer_;
+ TemporaryRegisterScope allocator_;
+ bool result_identified_;
+ DISALLOW_COPY_AND_ASSIGN(ExpressionResultScope);
+};
+
+
+// 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) {}
+};
+
+
+// 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() { set_result_identified(); }
+
+ virtual void SetResultInRegister(Register reg) {
+ builder()->LoadAccumulatorWithRegister(reg);
+ set_result_identified();
+ }
+};
+
+
+// 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_ = outer()->NewRegister();
+ builder()->StoreAccumulatorInRegister(result_register_);
+ set_result_identified();
+ }
+
+ virtual void SetResultInRegister(Register reg) {
+ DCHECK(builder()->RegisterIsParameterOrLocal(reg) ||
+ builder()->RegisterIsTemporary(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 +264,10 @@ BytecodeGenerator::BytecodeGenerator(Isolate* isolate, Zone* zone)
scope_(nullptr),
globals_(0, zone),
execution_control_(nullptr),
- execution_context_(nullptr) {
+ execution_context_(nullptr),
+ result_scope_(nullptr),
+ binary_expression_depth_(0),
+ binary_expression_hazard_set_(zone) {
InitializeAstVisitor(isolate);
}
@@ -324,12 +437,11 @@ void BytecodeGenerator::VisitDeclarations(
void BytecodeGenerator::VisitExpressionStatement(ExpressionStatement* stmt) {
- Visit(stmt->expression());
+ VisitForEffect(stmt->expression());
}
void BytecodeGenerator::VisitEmptyStatement(EmptyStatement* stmt) {
- // TODO(oth): For control-flow it could be useful to signal empty paths here.
}
@@ -340,7 +452,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 +484,7 @@ void BytecodeGenerator::VisitBreakStatement(BreakStatement* stmt) {
void BytecodeGenerator::VisitReturnStatement(ReturnStatement* stmt) {
- Visit(stmt->expression());
+ VisitForAccumulatorValue(stmt->expression());
builder()->Return();
}
@@ -398,7 +510,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 +528,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 +557,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 +612,7 @@ void BytecodeGenerator::VisitFunctionLiteral(FunctionLiteral* expr) {
builder()
->LoadLiteral(shared_info)
.CreateClosure(expr->pretenure() ? TENURED : NOT_TENURED);
+ result_scope()->SetResultInAccumulator();
}
@@ -534,6 +647,7 @@ void BytecodeGenerator::VisitLiteral(Literal* expr) {
} else {
builder()->LoadLiteral(value);
}
+ result_scope()->SetResultInAccumulator();
}
@@ -546,6 +660,7 @@ void BytecodeGenerator::VisitRegExpLiteral(RegExpLiteral* expr) {
.StoreAccumulatorInRegister(flags)
.LoadLiteral(expr->pattern())
.CreateRegExpLiteral(expr->literal_index(), flags);
+ result_scope()->SetResultInAccumulator();
}
@@ -590,21 +705,28 @@ 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());
+ VisitForEffect(property->value());
}
} else {
- Register key = inner_temporary_register_scope.NewRegister();
- Register value = inner_temporary_register_scope.NewRegister();
- Register language = inner_temporary_register_scope.NewRegister();
+ inner_temporary_register_scope.PrepareForConsecutiveAllocations(3);
+ Register key =
+ inner_temporary_register_scope.NextConsecutiveRegister();
+ Register value =
+ inner_temporary_register_scope.NextConsecutiveRegister();
+ Register language =
+ inner_temporary_register_scope.NextConsecutiveRegister();
+ // 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/20 16:39:20 Hmm, this is tricky. One option would be to do the
oth 2015/10/20 20:58:42 We may have exactly the right mix of allocation sc
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()
@@ -617,10 +739,12 @@ void BytecodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) {
break;
}
case ObjectLiteral::Property::PROTOTYPE: {
+ inner_temporary_register_scope.PrepareForConsecutiveAllocations(1);
DCHECK(property->emit_store());
- Register value = inner_temporary_register_scope.NewRegister();
+ Register value =
+ inner_temporary_register_scope.NextConsecutiveRegister();
DCHECK(Register::AreContiguous(literal, value));
- Visit(property->value());
+ VisitForAccumulatorValue(property->value());
builder()->StoreAccumulatorInRegister(value).CallRuntime(
Runtime::kInternalSetPrototype, literal, 2);
break;
@@ -643,12 +767,13 @@ void BytecodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) {
for (AccessorTable::Iterator it = accessor_table.begin();
it != accessor_table.end(); ++it) {
TemporaryRegisterScope inner_temporary_register_scope(builder());
- Register name = inner_temporary_register_scope.NewRegister();
- Register getter = inner_temporary_register_scope.NewRegister();
- Register setter = inner_temporary_register_scope.NewRegister();
- Register attr = inner_temporary_register_scope.NewRegister();
+ inner_temporary_register_scope.PrepareForConsecutiveAllocations(4);
+ Register name = inner_temporary_register_scope.NextConsecutiveRegister();
+ Register getter = inner_temporary_register_scope.NextConsecutiveRegister();
+ Register setter = inner_temporary_register_scope.NextConsecutiveRegister();
+ Register attr = inner_temporary_register_scope.NextConsecutiveRegister();
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,7 +796,8 @@ void BytecodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) {
ObjectLiteral::Property* property = expr->properties()->at(property_index);
if (literal_in_accumulator) {
- literal = temporary_register_scope.NewRegister();
+ temporary_register_scope.PrepareForConsecutiveAllocations(4);
+ literal = temporary_register_scope.NextConsecutiveRegister();
builder()->StoreAccumulatorInRegister(literal);
literal_in_accumulator = false;
}
@@ -681,21 +807,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());
- Register key = inner_temporary_register_scope.NewRegister();
- Register value = inner_temporary_register_scope.NewRegister();
- Register attr = inner_temporary_register_scope.NewRegister();
+ inner_temporary_register_scope.PrepareForConsecutiveAllocations(3);
+ Register key = inner_temporary_register_scope.NextConsecutiveRegister();
+ Register value = inner_temporary_register_scope.NextConsecutiveRegister();
+ Register attr = inner_temporary_register_scope.NextConsecutiveRegister();
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 +856,7 @@ void BytecodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) {
// Restore literal array into accumulator.
builder()->LoadAccumulatorWithRegister(literal);
}
+ result_scope()->SetResultInAccumulator();
}
@@ -760,11 +888,11 @@ void BytecodeGenerator::VisitArrayLiteral(ArrayLiteral* expr) {
literal_in_accumulator = false;
}
+ FeedbackVectorSlot slot = expr->LiteralFeedbackSlot();
builder()
->LoadLiteral(Smi::FromInt(array_index))
.StoreAccumulatorInRegister(index);
- Visit(subexpr);
- FeedbackVectorSlot slot = expr->LiteralFeedbackSlot();
+ VisitForAccumulatorValue(subexpr);
builder()->StoreKeyedProperty(literal, index, feedback_index(slot),
language_mode());
}
@@ -773,6 +901,7 @@ void BytecodeGenerator::VisitArrayLiteral(ArrayLiteral* expr) {
// Restore literal array into accumulator.
builder()->LoadAccumulatorWithRegister(literal);
}
+ result_scope()->SetResultInAccumulator();
}
@@ -785,17 +914,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 +932,7 @@ void BytecodeGenerator::VisitVariableLoad(Variable* variable,
// runtime.
DCHECK(variable->IsStaticGlobalObjectProperty());
builder()->LoadGlobal(variable->index());
+ result_scope()->SetResultInAccumulator();
break;
}
case VariableLocation::UNALLOCATED: {
@@ -815,6 +943,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 +953,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 +971,7 @@ void BytecodeGenerator::VisitVariableAssignment(Variable* variable,
// TODO(rmcilroy): support const mode initialization.
Register destination(variable->index());
builder()->StoreAccumulatorInRegister(destination);
+ RecordStoreToRegister(destination);
break;
}
case VariableLocation::PARAMETER: {
@@ -848,6 +979,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);
+ RecordStoreToRegister(destination);
break;
}
case VariableLocation::GLOBAL: {
@@ -860,10 +992,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 +1027,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 +1038,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 +1060,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 +1102,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,29 +1119,28 @@ 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));
+ result_scope()->PrepareForConsecutiveAllocations(args->length());
+
+ // Visit for first argument that goes into returned register
+ Register first_arg = result_scope()->NextConsecutiveRegister();
+ VisitForAccumulatorValue(args->at(0));
builder()->StoreAccumulatorInRegister(first_arg);
+
+ // Visit remaining arguments
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()->NextConsecutiveRegister();
+ VisitForAccumulatorValue(args->at(i));
builder()->StoreAccumulatorInRegister(ith_arg);
DCHECK(ith_arg.index() - i == first_arg.index());
}
-
return first_arg;
}
@@ -1022,9 +1151,14 @@ void BytecodeGenerator::VisitCall(Call* expr) {
// 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);
rmcilroy 2015/10/20 16:39:20 You call PrepareForConsecutiveAllocations(args->le
oth 2015/10/20 20:58:42 Comment added to VisitArguments. The weirdness is
+ Register receiver = result_scope()->NextConsecutiveRegister();
switch (call_type) {
case Call::PROPERTY_CALL: {
@@ -1032,14 +1166,18 @@ 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);
break;
}
case Call::GLOBAL_CALL: {
+ AccumulatorResultScope accumulator_result_scope(this);
rmcilroy 2015/10/20 16:39:20 Not sure why this is needed? Could you add a comme
oth 2015/10/20 20:58:42 Comment added and scope moved immediately before V
// Receiver is undefined for global calls.
builder()->LoadUndefined().StoreAccumulatorInRegister(receiver);
// Load callee as a global variable.
@@ -1050,7 +1188,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 +1200,26 @@ 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);
+ Register arg = VisitArguments(args);
+ CHECK(arg.index() == receiver.index() + 1);
}
// 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);
+ 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 +1227,8 @@ void BytecodeGenerator::VisitCallNew(CallNew* expr) {
// allocating a temporary just to fill the operands.
builder()->New(constructor, constructor, 0);
}
+
+ result_scope()->SetResultInAccumulator();
}
@@ -1097,41 +1237,44 @@ void BytecodeGenerator::VisitCallRuntime(CallRuntime* expr) {
UNIMPLEMENTED();
}
- // 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;
+
+ // Evaluate all arguments to the runtime call.
ZoneList<Expression*>* args = expr->arguments();
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());
+ 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 +1322,40 @@ 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());
+ // TODO(oth): add binary operation scope here to replace
rmcilroy 2015/10/20 16:39:20 As mentioned in the previous review, I don't think
oth 2015/10/20 20:58:42 Done - grumble, grumble :-) We can go conservative
+ // {Prepare|Complete}BinaryExpression. This will maintain a counter in the
+ // statement or top-level expression scope.
+ 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.
+ // TODO(oth) LoadFromAliasedRegister call into VisitVariableLoad().
+ lhs = LoadFromAliasedRegister(lhs);
+ }
+ VisitForAccumulatorValue(expr->right());
+ builder()->CompareOperation(expr->op(), lhs, language_mode_strength());
+ result_scope()->SetResultInAccumulator();
+ CompleteBinaryExpression();
+}
+
+
+void BytecodeGenerator::VisitArithmeticExpression(BinaryOperation* expr) {
+ // TODO(oth): add binary operation scope here to replace
+ // {Prepare|Complete}BinaryExpression. This will maintain a counter in the
+ // statement or top-level expression scope.
rmcilroy 2015/10/20 16:39:20 ditto
oth 2015/10/20 20:58:42 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.
+ // TODO(oth) LoadFromAliasedRegister call into VisitVariableLoad().
+ lhs = LoadFromAliasedRegister(lhs);
+ }
+ VisitForAccumulatorValue(expr->right());
+ builder()->BinaryOperation(expr->op(), lhs, language_mode_strength());
+ result_scope()->SetResultInAccumulator();
+ CompleteBinaryExpression();
}
@@ -1281,27 +1447,9 @@ void BytecodeGenerator::VisitNewLocalBlockContext(Scope* scope) {
}
-void BytecodeGenerator::VisitArithmeticExpression(BinaryOperation* binop) {
- Token::Value op = binop->op();
- Expression* left = binop->left();
- Expression* right = binop->right();
-
- 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::VisitCommaExpression(BinaryOperation* binop) {
- Expression* left = binop->left();
- Expression* right = binop->right();
-
- Visit(left);
- Visit(right);
+ VisitForEffect(binop->left());
+ Visit(binop->right());
}
@@ -1312,15 +1460,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 +1479,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 +1497,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);
}
@@ -1393,6 +1541,68 @@ void BytecodeGenerator::VisitFunctionClosureForContext() {
}
+void BytecodeGenerator::PrepareForBinaryExpression() {
+ if (binary_expression_depth_++ == 0) {
+ binary_expression_hazard_set_.clear();
+ }
+}
+
+
+// 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.
rmcilroy 2015/10/20 16:39:20 Comment now out of date (no need to release tempor
oth 2015/10/20 20:58:42 Done.
+Register BytecodeGenerator::VisitForRegisterValue(Expression* expr) {
+ RegisterResultScope register_scope(this);
+ Visit(expr);
+ return register_scope.ResultRegister();
+}
+
+
+Register BytecodeGenerator::LoadFromAliasedRegister(Register reg) {
+ // 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::RecordStoreToRegister(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());
+ }
+}
+
+
+void BytecodeGenerator::CompleteBinaryExpression() {
+ DCHECK(binary_expression_depth_ > 0);
+ binary_expression_depth_ -= 1;
+ // TODO(oth): spill remapped registers into origins.
+ // TODO(oth): make statement/top-level.
+}
+
+
Register BytecodeGenerator::NextContextRegister() const {
if (execution_context() == nullptr) {
// Return the incoming function context for the outermost execution context.

Powered by Google App Engine
This is Rietveld 408576698