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

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

Issue 1576403004: [Interpreter] Removes assignment hazard scope. (Closed) Base URL: https://chromium.googlesource.com/v8/v8.git@master
Patch Set: rebased the patch 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
« no previous file with comments | « src/interpreter/bytecode-generator.h ('k') | test/cctest/interpreter/test-bytecode-generator.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/interpreter/bytecode-generator.cc
diff --git a/src/interpreter/bytecode-generator.cc b/src/interpreter/bytecode-generator.cc
index 73eee32fcb5aa7b66458639ab91fc363721c0b29..268b9fd79a1282e64e661dfc28222b309fcc646d 100644
--- a/src/interpreter/bytecode-generator.cc
+++ b/src/interpreter/bytecode-generator.cc
@@ -214,11 +214,12 @@ class BytecodeGenerator::ExpressionResultScope {
// VisitForRegisterValue allocates register in outer context.
return allocator_.NewRegister();
} else {
- // We need this when allocating registers due to an Assignment hazard.
- // It might be expensive to walk the full context chain and compute the
- // list of consecutive reservations in the innerscopes. So allocates a
- // new unallocated temporary register.
- return allocator_.AllocateNewRegister();
+ // If it is required to allocate a register other than current or outer
+ // scopes, allocate a new temporary register. It might be expensive to
+ // walk the full context chain and compute the list of consecutive
+ // reservations in the innerscopes.
+ UNIMPLEMENTED();
+ return Register(-1);
}
}
@@ -312,120 +313,6 @@ class BytecodeGenerator::RegisterResultScope final
};
-BytecodeGenerator::AssignmentHazardHelper::AssignmentHazardHelper(
- BytecodeGenerator* generator)
- : generator_(generator),
- alias_mappings_(generator->zone()),
- aliased_locals_and_parameters_(generator->zone()),
- execution_result_(nullptr),
- scope_depth_(0) {}
-
-
-void BytecodeGenerator::AssignmentHazardHelper::EnterScope() {
- DCHECK_GE(scope_depth_, 0);
- if (scope_depth_++ == 0) {
- execution_result_ = generator_->execution_result();
- }
-}
-
-
-void BytecodeGenerator::AssignmentHazardHelper::LeaveScope() {
- DCHECK_GT(scope_depth_, 0);
- if (--scope_depth_ == 0) {
- DCHECK_EQ(execution_result_, generator_->execution_result());
- RestoreAliasedLocalsAndParameters();
- }
-}
-
-
-// Returns a register that a load instruction should use when
-// loading from |reg|. This allows an alias for a modified version
-// of |reg| to be used within a hazard regions.
-MUST_USE_RESULT Register
-BytecodeGenerator::AssignmentHazardHelper::GetRegisterForLoad(Register reg) {
- if (scope_depth_ == 0) {
- return reg;
- } else {
- // A load from |reg| is to be issued. The register is placed in
- // the mappings table initially mapping to itself. Future stores
- // will update the mapping with temporaries thus preserving the
- // original register's value.
- //
- // NB This insert only updates the table if no mapping exists
- // already (std::map::insert semantics).
- auto insert_result =
- alias_mappings_.insert(std::make_pair(reg.index(), reg.index()));
- auto mapping = insert_result.first;
- // Return the current alias for reg.
- return Register(mapping->second);
- }
-}
-
-
-// Returns a register that a store instruction should use when
-// loading from |reg|. This allows an alias for a modified version
-// of |reg| to be used within hazard regions.
-MUST_USE_RESULT Register
-BytecodeGenerator::AssignmentHazardHelper::GetRegisterForStore(Register reg) {
- if (scope_depth_ == 0 ||
- alias_mappings_.find(reg.index()) == alias_mappings_.end()) {
- // If not in a hazard region or a load for this register has not
- // occurred no mapping is necessary.
- return reg;
- } else {
- // Storing to a register with 1 or more loads issued. The
- // register is mapped to a temporary alias so we don't overwrite
- // the lhs value, e.g. y = x + (x = 1); has a register for x on
- // the lhs and needs a new register x' for the upcoming store on
- // the rhs as the original x is an input to the add operation.
- Register alias = execution_result_->NewRegister();
- alias_mappings_[reg.index()] = alias.index();
- if (generator_->builder()->RegisterIsParameterOrLocal(reg)) {
- // Keep track of registers that need to be restored on exit
- // from the assignment hazard region.
- aliased_locals_and_parameters_.insert(reg.index());
- }
- return alias;
- }
-}
-
-
-void BytecodeGenerator::AssignmentHazardHelper::
- RestoreAliasedLocalsAndParameters() {
- DCHECK(scope_depth_ == 0);
- // Move temporary registers holding values for locals and
- // parameters back into their local and parameter registers.
- for (auto reg = aliased_locals_and_parameters_.begin();
- reg != aliased_locals_and_parameters_.end(); reg++) {
- auto mapping = alias_mappings_.find(*reg);
- if (mapping != alias_mappings_.end()) {
- generator_->builder()->MoveRegister(Register(mapping->second),
- Register(*reg));
- }
- }
- alias_mappings_.clear();
- aliased_locals_and_parameters_.clear();
-}
-
-
-class BytecodeGenerator::AssignmentHazardScope final {
- public:
- explicit AssignmentHazardScope(BytecodeGenerator* generator)
- : generator_(generator) {
- generator_->assignment_hazard_helper()->EnterScope();
- }
-
- ~AssignmentHazardScope() {
- generator_->assignment_hazard_helper()->LeaveScope();
- }
-
- private:
- BytecodeGenerator* generator_;
-
- DISALLOW_COPY_AND_ASSIGN(AssignmentHazardScope);
-};
-
-
BytecodeGenerator::BytecodeGenerator(Isolate* isolate, Zone* zone)
: isolate_(isolate),
zone_(zone),
@@ -435,8 +322,7 @@ BytecodeGenerator::BytecodeGenerator(Isolate* isolate, Zone* zone)
globals_(0, zone),
execution_control_(nullptr),
execution_context_(nullptr),
- execution_result_(nullptr),
- assignment_hazard_helper_(this) {
+ execution_result_(nullptr) {
InitializeAstVisitor(isolate);
}
@@ -708,6 +594,9 @@ void BytecodeGenerator::VisitWithStatement(WithStatement* stmt) {
void BytecodeGenerator::VisitSwitchStatement(SwitchStatement* stmt) {
+ // We need this scope because we visit for register values. We have to
+ // maintain a execution result scope where registers can be allocated.
+ EffectResultScope effect_results_scope(this);
ZoneList<CaseClause*>* clauses = stmt->cases();
SwitchBuilder switch_builder(builder(), clauses->length());
ControlScopeForBreakable scope(this, stmt, &switch_builder);
@@ -1281,16 +1170,16 @@ void BytecodeGenerator::VisitVariableLoad(Variable* variable,
switch (variable->location()) {
case VariableLocation::LOCAL: {
Register source(Register(variable->index()));
- source = assignment_hazard_helper()->GetRegisterForLoad(source);
- execution_result()->SetResultInRegister(source);
+ builder()->LoadAccumulatorWithRegister(source);
+ execution_result()->SetResultInAccumulator();
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);
- source = assignment_hazard_helper()->GetRegisterForLoad(source);
- execution_result()->SetResultInRegister(source);
+ builder()->LoadAccumulatorWithRegister(source);
+ execution_result()->SetResultInAccumulator();
break;
}
case VariableLocation::GLOBAL:
@@ -1358,8 +1247,6 @@ void BytecodeGenerator::VisitVariableAssignment(Variable* variable,
case VariableLocation::LOCAL: {
// TODO(rmcilroy): support const mode initialization.
Register destination(variable->index());
- destination =
- assignment_hazard_helper()->GetRegisterForStore(destination);
builder()->StoreAccumulatorInRegister(destination);
break;
}
@@ -1367,9 +1254,6 @@ void BytecodeGenerator::VisitVariableAssignment(Variable* variable,
// 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));
- destination =
- assignment_hazard_helper()->GetRegisterForStore(destination);
-
builder()->StoreAccumulatorInRegister(destination);
break;
}
@@ -1970,14 +1854,6 @@ void BytecodeGenerator::VisitBinaryOperation(BinaryOperation* binop) {
void BytecodeGenerator::VisitCompareOperation(CompareOperation* expr) {
- // The evaluation of binary comparison expressions has an assignment
- // hazard because the lhs may be a variable that evaluates to a
- // local or parameter and the rhs may modify that, e.g. y = x + (x = 1)
- // To get a correct result the generator treats the inner assigment
- // as being made to a temporary x' that is spilled on exit of the
- // assignment hazard.
- AssignmentHazardScope assignment_hazard_scope(this);
-
Register lhs = VisitForRegisterValue(expr->left());
VisitForAccumulatorValue(expr->right());
builder()->CompareOperation(expr->op(), lhs, language_mode_strength());
@@ -1986,14 +1862,6 @@ void BytecodeGenerator::VisitCompareOperation(CompareOperation* expr) {
void BytecodeGenerator::VisitArithmeticExpression(BinaryOperation* expr) {
- // The evaluation of binary arithmetic expressions has an assignment
- // hazard because the lhs may be a variable that evaluates to a
- // local or parameter and the rhs may modify that, e.g. y = x + (x = 1)
- // To get a correct result the generator treats the inner assigment
- // as being made to a temporary x' that is spilled on exit of the
- // assignment hazard.
- AssignmentHazardScope assignment_hazard_scope(this);
-
Register lhs = VisitForRegisterValue(expr->left());
VisitForAccumulatorValue(expr->right());
builder()->BinaryOperation(expr->op(), lhs, language_mode_strength());
« no previous file with comments | « src/interpreter/bytecode-generator.h ('k') | test/cctest/interpreter/test-bytecode-generator.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698