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

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

Issue 1634153002: [Interpreter] Adds support for const/let variables to interpreter. (Closed) Base URL: https://chromium.googlesource.com/v8/v8.git@master
Patch Set: Refactored VisitVariableAssignment. Created 4 years, 11 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 4ef0506a12388cf0f534058b33b98e21f3689169..3f3fde63b9a2da33d602259f970f96615b56511b 100644
--- a/src/interpreter/bytecode-generator.cc
+++ b/src/interpreter/bytecode-generator.cc
@@ -718,7 +718,10 @@ void BytecodeGenerator::VisitFunctionDeclaration(FunctionDeclaration* decl) {
case VariableLocation::PARAMETER:
case VariableLocation::LOCAL: {
VisitForAccumulatorValue(decl->fun());
- VisitVariableAssignment(variable, FeedbackVectorSlot::Invalid());
+ DCHECK(variable->mode() == LET || variable->mode() == VAR ||
+ variable->mode() == CONST);
+ VisitVariableAssignment(variable, Token::INIT,
+ FeedbackVectorSlot::Invalid());
break;
}
case VariableLocation::CONTEXT: {
@@ -1000,7 +1003,7 @@ void BytecodeGenerator::VisitForInAssignment(Expression* expr,
switch (assign_type) {
case VARIABLE: {
Variable* variable = expr->AsVariableProxy()->var();
- VisitVariableAssignment(variable, slot);
+ VisitVariableAssignment(variable, Token::ASSIGN, slot);
break;
}
case NAMED_PROPERTY: {
@@ -1505,14 +1508,25 @@ void BytecodeGenerator::VisitVariableProxy(VariableProxy* proxy) {
VisitVariableLoad(proxy->var(), proxy->VariableFeedbackSlot());
}
+void BytecodeGenerator::BuildHoleCheckForVariableLoad(VariableMode mode,
+ Handle<String> name) {
+ if (mode == CONST_LEGACY) {
+ BytecodeLabel end_label;
+ builder()->JumpIfNotHole(&end_label).LoadUndefined().Bind(&end_label);
+ } else if (mode == LET || mode == CONST) {
+ BuildThrowIfHole(name);
+ }
+}
void BytecodeGenerator::VisitVariableLoad(Variable* variable,
FeedbackVectorSlot slot,
TypeofMode typeof_mode) {
+ VariableMode mode = variable->mode();
switch (variable->location()) {
case VariableLocation::LOCAL: {
Register source(Register(variable->index()));
builder()->LoadAccumulatorWithRegister(source);
+ BuildHoleCheckForVariableLoad(mode, variable->name());
execution_result()->SetResultInAccumulator();
break;
}
@@ -1521,6 +1535,7 @@ void BytecodeGenerator::VisitVariableLoad(Variable* variable,
// index -1 but is parameter index 0 in BytecodeArrayBuilder).
Register source = builder()->Parameter(variable->index() + 1);
builder()->LoadAccumulatorWithRegister(source);
+ BuildHoleCheckForVariableLoad(mode, variable->name());
execution_result()->SetResultInAccumulator();
break;
}
@@ -1553,10 +1568,10 @@ void BytecodeGenerator::VisitVariableLoad(Variable* variable,
.StoreAccumulatorInRegister(context_reg);
}
}
+
builder()->LoadContextSlot(context_reg, variable->index());
+ BuildHoleCheckForVariableLoad(mode, variable->name());
execution_result()->SetResultInAccumulator();
- // TODO(rmcilroy): Perform check for uninitialized legacy const, const and
- // let variables.
break;
}
case VariableLocation::LOOKUP: {
@@ -1582,20 +1597,101 @@ Register BytecodeGenerator::VisitVariableLoadForRegisterValue(
return register_scope.ResultRegister();
}
+void BytecodeGenerator::BuildThrowIfHole(Handle<String> name) {
+ Register name_reg = register_allocator()->NewRegister();
+ BytecodeLabel end_label;
rmcilroy 2016/02/03 12:28:26 nit - add a TODO that we will replace this with a
mythria 2016/02/04 10:28:52 Done.
+ builder()
+ ->JumpIfNotHole(&end_label)
+ .LoadLiteral(name)
+ .StoreAccumulatorInRegister(name_reg)
+ .CallRuntime(Runtime::kThrowReferenceError, name_reg, 1)
+ .Bind(&end_label);
+}
+
+void BytecodeGenerator::BuildThrowIfNotHole(Handle<String> name) {
+ Register name_reg = register_allocator()->NewRegister();
+ BytecodeLabel end_label;
rmcilroy 2016/02/03 12:28:26 ditto
mythria 2016/02/04 10:28:52 Done.
+ builder()
+ ->JumpIfHole(&end_label)
+ .LoadLiteral(name)
+ .StoreAccumulatorInRegister(name_reg)
+ .CallRuntime(Runtime::kThrowReferenceError, name_reg, 1)
+ .Bind(&end_label);
+}
+
+void BytecodeGenerator::BuildThrowReassignConstant(Handle<String> name) {
+ Register name_reg = register_allocator()->NewRegister();
+ BytecodeLabel else_label;
rmcilroy 2016/02/03 12:28:26 ditto
mythria 2016/02/04 10:28:52 Done.
+ builder()
+ ->JumpIfNotHole(&else_label)
+ .LoadLiteral(name)
+ .StoreAccumulatorInRegister(name_reg)
+ .CallRuntime(Runtime::kThrowReferenceError, name_reg, 1)
+ .Bind(&else_label)
+ .CallRuntime(Runtime::kThrowConstAssignError, Register(), 0);
+}
+
+void BytecodeGenerator::BuildHoleCheckForVariableAssignment(Variable* variable,
+ Token::Value op) {
+ VariableMode mode = variable->mode();
rmcilroy 2016/02/03 12:28:26 DCHECK not legacy const mode
mythria 2016/02/04 10:28:52 Done.
+ if (mode == CONST && op != Token::INIT) {
+ // Assigning to constant is not allowed.
+ BuildThrowReassignConstant(variable->name());
+ } else if (mode == LET && op != Token::INIT) {
+ BuildThrowIfHole(variable->name());
rmcilroy 2016/02/03 12:28:26 nit - add comment
mythria 2016/02/04 10:28:52 Done.
+ } else {
+ DCHECK(variable->is_this() && mode == CONST && op == Token::INIT);
+ // Perform an initialization check for 'this'. 'this' variable is the
+ // only variable able to trigger bind operations outside the TDZ
+ // via 'super' calls.
+ BuildThrowIfNotHole(variable->name());
+ }
+}
void BytecodeGenerator::VisitVariableAssignment(Variable* variable,
+ Token::Value op,
FeedbackVectorSlot slot) {
+ VariableMode mode = variable->mode();
+ RegisterAllocationScope assignment_register_scope(this);
+ BytecodeLabel end_label;
+ bool hole_check_required =
+ (mode == CONST_LEGACY) || (mode == LET && op != Token::INIT) ||
+ (mode == CONST && op != Token::INIT) ||
+ (mode == CONST && op == Token::INIT && variable->is_this());
switch (variable->location()) {
+ case VariableLocation::PARAMETER:
case VariableLocation::LOCAL: {
- // TODO(rmcilroy): support const mode initialization.
- Register destination(variable->index());
- builder()->StoreAccumulatorInRegister(destination);
- break;
- }
- case VariableLocation::PARAMETER: {
- // The parameter indices are shifted by 1 (receiver is variable
- // index -1 but is parameter index 0 in BytecodeArrayBuilder).
- Register destination(builder()->Parameter(variable->index() + 1));
+ Register destination;
+ if (VariableLocation::PARAMETER == variable->location()) {
+ destination = Register(builder()->Parameter(variable->index() + 1));
+ } else {
+ destination = Register(variable->index());
+ }
+
+ if (hole_check_required) {
+ // Load destination to check for hole.
+ Register value_temp = register_allocator()->NewRegister();
+ builder()
+ ->StoreAccumulatorInRegister(value_temp)
+ .LoadAccumulatorWithRegister(destination);
+
+ if (mode == CONST_LEGACY && op == Token::INIT) {
+ builder()
+ ->JumpIfNotHole(&end_label)
+ .MoveRegister(value_temp, destination)
+ .Bind(&end_label)
+ .LoadAccumulatorWithRegister(value_temp);
+ break;
rmcilroy 2016/02/03 12:28:26 nit - add comment on why this break is necessary
mythria 2016/02/04 10:28:52 Done.
+ } else if (mode == CONST_LEGACY && op != Token::INIT) {
rmcilroy 2016/02/03 12:28:26 Could you drop this entire branch if you just made
mythria 2016/02/04 10:28:53 As discussed offline, I am not moving this because
+ DCHECK(!is_strict(language_mode()));
+ builder()->LoadAccumulatorWithRegister(value_temp);
rmcilroy 2016/02/03 12:28:26 nit - add a comment
mythria 2016/02/04 10:28:52 Done.
+ break;
+ } else {
+ BuildHoleCheckForVariableAssignment(variable, op);
+ builder()->LoadAccumulatorWithRegister(value_temp);
+ }
+ }
+
builder()->StoreAccumulatorInRegister(destination);
break;
}
@@ -1606,10 +1702,10 @@ void BytecodeGenerator::VisitVariableAssignment(Variable* variable,
break;
}
case VariableLocation::CONTEXT: {
- // TODO(rmcilroy): support const mode initialization.
int depth = execution_context()->ContextChainDepth(variable->scope());
ContextScope* context = execution_context()->Previous(depth);
Register context_reg;
+
if (context) {
context_reg = context->reg();
} else {
@@ -1631,13 +1727,56 @@ void BytecodeGenerator::VisitVariableAssignment(Variable* variable,
}
builder()->LoadAccumulatorWithRegister(value_temp);
}
+
+ if (hole_check_required) {
+ // Load destination to check for hole.
+ Register value_temp = register_allocator()->NewRegister();
+ builder()
+ ->StoreAccumulatorInRegister(value_temp)
+ .LoadContextSlot(context_reg, variable->index());
+
+ if (mode == CONST_LEGACY && op == Token::INIT) {
+ builder()
+ ->JumpIfNotHole(&end_label)
+ .LoadAccumulatorWithRegister(value_temp)
+ .StoreContextSlot(context_reg, variable->index())
+ .Bind(&end_label);
+ builder()->LoadAccumulatorWithRegister(value_temp);
+ break;
rmcilroy 2016/02/03 12:28:26 ditto
mythria 2016/02/04 10:28:52 Done.
+ } else if (mode == CONST_LEGACY && op != Token::INIT) {
+ DCHECK(!is_strict(language_mode()));
rmcilroy 2016/02/03 12:28:26 Same comment
mythria 2016/02/04 10:28:52 Same as variable/parameter case earlier.
+ builder()->LoadAccumulatorWithRegister(value_temp);
+ break;
+ } else {
+ BuildHoleCheckForVariableAssignment(variable, op);
+ builder()->LoadAccumulatorWithRegister(value_temp);
+ }
+ }
+
builder()->StoreContextSlot(context_reg, variable->index());
break;
}
case VariableLocation::LOOKUP: {
- // TODO(mythria): Use Runtime::kInitializeLegacyConstLookupSlot for
- // initializations of const declarations.
- builder()->StoreLookupSlot(variable->name(), language_mode());
+ if (mode == CONST_LEGACY && op == Token::INIT) {
+ register_allocator()->PrepareForConsecutiveAllocations(3);
+ Register value = register_allocator()->NextConsecutiveRegister();
+ Register context = register_allocator()->NextConsecutiveRegister();
+ Register name = register_allocator()->NextConsecutiveRegister();
+
+ // InitializeLegacyConstLookupSlot runtime call returns the 'value'
+ // passed to it. So, accumulator will have its original contents when
+ // runtime call returns.
+ builder()
+ ->StoreAccumulatorInRegister(value)
+ .MoveRegister(execution_context()->reg(), context)
+ .LoadLiteral(variable->name())
+ .StoreAccumulatorInRegister(name)
+ .CallRuntime(Runtime::kInitializeLegacyConstLookupSlot, value, 3);
+ } else if (mode == CONST_LEGACY && op != Token::INIT) {
rmcilroy 2016/02/03 12:28:26 ditto
mythria 2016/02/04 10:28:52 Same here.
+ DCHECK(!is_strict(language_mode()));
+ } else {
+ builder()->StoreLookupSlot(variable->name(), language_mode());
+ }
break;
}
}
@@ -1730,7 +1869,7 @@ void BytecodeGenerator::VisitAssignment(Assignment* expr) {
// TODO(oth): The VisitVariableAssignment() call is hard to reason about.
// Is the value in the accumulator safe? Yes, but scary.
Variable* variable = expr->target()->AsVariableProxy()->var();
- VisitVariableAssignment(variable, slot);
+ VisitVariableAssignment(variable, expr->op(), slot);
break;
}
case NAMED_PROPERTY:
@@ -2156,7 +2295,7 @@ void BytecodeGenerator::VisitCountOperation(CountOperation* expr) {
switch (assign_type) {
case VARIABLE: {
Variable* variable = expr->expression()->AsVariableProxy()->var();
- VisitVariableAssignment(variable, feedback_slot);
+ VisitVariableAssignment(variable, expr->op(), feedback_slot);
break;
}
case NAMED_PROPERTY: {
@@ -2420,7 +2559,8 @@ void BytecodeGenerator::VisitArgumentsObject(Variable* variable) {
? CreateArgumentsType::kUnmappedArguments
: CreateArgumentsType::kMappedArguments;
builder()->CreateArguments(type);
- VisitVariableAssignment(variable, FeedbackVectorSlot::Invalid());
+ VisitVariableAssignment(variable, Token::ASSIGN,
+ FeedbackVectorSlot::Invalid());
}
@@ -2432,7 +2572,7 @@ void BytecodeGenerator::VisitThisFunctionVariable(Variable* variable) {
// Store the closure we were called with in the given variable.
builder()->LoadAccumulatorWithRegister(Register::function_closure());
- VisitVariableAssignment(variable, FeedbackVectorSlot::Invalid());
+ VisitVariableAssignment(variable, Token::INIT, FeedbackVectorSlot::Invalid());
}
@@ -2441,7 +2581,7 @@ void BytecodeGenerator::VisitNewTargetVariable(Variable* variable) {
// Store the new target we were called with in the given variable.
builder()->LoadAccumulatorWithRegister(Register::new_target());
- VisitVariableAssignment(variable, FeedbackVectorSlot::Invalid());
+ VisitVariableAssignment(variable, Token::INIT, FeedbackVectorSlot::Invalid());
}

Powered by Google App Engine
This is Rietveld 408576698