From 48fed9a8ab49dd1ee5fe077b85b5bc8c1293e5ae Mon Sep 17 00:00:00 2001 From: "keuchel@chromium.org" Date: Thu, 3 Nov 2011 14:50:19 +0000 Subject: [PATCH] Clean up Scope::CollectUsedVariables. Review URL: http://codereview.chromium.org/8438071 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@9874 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/liveedit.cc | 32 +++++++------------------------ src/scopeinfo.cc | 49 +++++++++--------------------------------------- src/scopes.cc | 35 +++++++++++++++++++++++++++------- src/scopes.h | 10 ++++++++-- src/variables.cc | 8 ++++++++ src/variables.h | 2 ++ 6 files changed, 62 insertions(+), 74 deletions(-) diff --git a/src/liveedit.cc b/src/liveedit.cc index ce903d1f7..9b77be17a 100644 --- a/src/liveedit.cc +++ b/src/liveedit.cc @@ -855,38 +855,20 @@ class FunctionInfoListener { return HEAP->undefined_value(); } do { - ZoneList list(10); - outer_scope->CollectUsedVariables(&list); - int j = 0; - for (int i = 0; i < list.length(); i++) { - Variable* var1 = list[i]; - if (var1->IsContextSlot()) { - if (j != i) { - list[j] = var1; - } - j++; - } - } + ZoneList stack_list(outer_scope->StackLocalCount()); + ZoneList context_list(outer_scope->ContextLocalCount()); + outer_scope->CollectStackAndContextLocals(&stack_list, &context_list); + context_list.Sort(&Variable::CompareIndex); - // Sort it. - for (int k = 1; k < j; k++) { - int l = k; - for (int m = k + 1; m < j; m++) { - if (list[l]->index() > list[m]->index()) { - l = m; - } - } - list[k] = list[l]; - } - for (int i = 0; i < j; i++) { + for (int i = 0; i < context_list.length(); i++) { SetElementNonStrict(scope_info_list, scope_info_length, - list[i]->name()); + context_list[i]->name()); scope_info_length++; SetElementNonStrict( scope_info_list, scope_info_length, - Handle(Smi::FromInt(list[i]->index()))); + Handle(Smi::FromInt(context_list[i]->index()))); scope_info_length++; } SetElementNonStrict(scope_info_list, diff --git a/src/scopeinfo.cc b/src/scopeinfo.cc index 5b652e13d..6ea92cee3 100644 --- a/src/scopeinfo.cc +++ b/src/scopeinfo.cc @@ -38,45 +38,16 @@ namespace v8 { namespace internal { -static int CompareLocal(Variable* const* v, Variable* const* w) { - int x = (*v)->index(); - int y = (*w)->index(); - // Consider sorting them according to type as well? - return x - y; -} - - Handle ScopeInfo::Create(Scope* scope) { - ZoneList variables(32); // 32 is a wild guess - ASSERT(variables.is_empty()); - scope->CollectUsedVariables(&variables); - - ZoneList stack_locals(scope->num_stack_slots()); - ZoneList context_locals(scope->num_heap_slots()); - // Collect stack and context locals. - for (int i = 0; i < variables.length(); i++) { - Variable* var = variables[i]; - ASSERT(var->is_used()); - switch (var->location()) { - case Variable::UNALLOCATED: - case Variable::PARAMETER: - break; - - case Variable::LOCAL: - stack_locals.Add(var); - break; - - case Variable::CONTEXT: - context_locals.Add(var); - break; - - case Variable::LOOKUP: - // We don't expect lookup variables in the locals list. - UNREACHABLE(); - break; - } - } + ZoneList stack_locals(scope->StackLocalCount()); + ZoneList context_locals(scope->ContextLocalCount()); + scope->CollectStackAndContextLocals(&stack_locals, &context_locals); + const int stack_local_count = stack_locals.length(); + const int context_local_count = context_locals.length(); + // Make sure we allocate the correct amount. + ASSERT(scope->StackLocalCount() == stack_local_count); + ASSERT(scope->ContextLocalCount() == context_local_count); // Determine use and location of the function variable if it is present. FunctionVariableInfo function_name_info; @@ -99,8 +70,6 @@ Handle ScopeInfo::Create(Scope* scope) { const bool has_function_name = function_name_info != NONE; const int parameter_count = scope->num_parameters(); - const int stack_local_count = stack_locals.length(); - const int context_local_count = context_locals.length(); const int length = kVariablePartIndex + parameter_count + stack_local_count + 2 * context_local_count + (has_function_name ? 2 : 0); @@ -140,7 +109,7 @@ Handle ScopeInfo::Create(Scope* scope) { // according to usage, the allocated slot indices may not be in increasing // order with the variable list anymore. Thus, we first need to sort them by // context slot index before adding them to the ScopeInfo object. - context_locals.Sort(&CompareLocal); + context_locals.Sort(&Variable::CompareIndex); // Add context locals' names. ASSERT(index == scope_info->ContextLocalNameEntriesIndex()); diff --git a/src/scopes.cc b/src/scopes.cc index 6ace29259..f9cebcdda 100644 --- a/src/scopes.cc +++ b/src/scopes.cc @@ -529,23 +529,31 @@ Declaration* Scope::CheckConflictingVarDeclarations() { } -void Scope::CollectUsedVariables(ZoneList* locals) { - // Collect variables in this scope. - // Note that the function_ variable - if present - is not - // collected here but handled separately in ScopeInfo - // which is the current user of this function). +void Scope::CollectStackAndContextLocals(ZoneList* stack_locals, + ZoneList* context_locals) { + ASSERT(stack_locals != NULL); + ASSERT(context_locals != NULL); + + // Collect temporaries which are always allocated on the stack. for (int i = 0; i < temps_.length(); i++) { Variable* var = temps_[i]; if (var->is_used()) { - locals->Add(var); + ASSERT(var->IsStackLocal()); + stack_locals->Add(var); } } + + // Collect declared local variables. for (VariableMap::Entry* p = variables_.Start(); p != NULL; p = variables_.Next(p)) { Variable* var = reinterpret_cast(p->value); if (var->is_used()) { - locals->Add(var); + if (var->IsStackLocal()) { + stack_locals->Add(var); + } else if (var->IsContextSlot()) { + context_locals->Add(var); + } } } } @@ -1165,4 +1173,17 @@ void Scope::AllocateVariablesRecursively() { ASSERT(num_heap_slots_ == 0 || num_heap_slots_ >= Context::MIN_CONTEXT_SLOTS); } + +int Scope::StackLocalCount() const { + return num_stack_slots() - + (function_ != NULL && function_->var()->IsStackLocal() ? 1 : 0); +} + + +int Scope::ContextLocalCount() const { + if (num_heap_slots() == 0) return 0; + return num_heap_slots() - Context::MIN_CONTEXT_SLOTS - + (function_ != NULL && function_->var()->IsContextSlot() ? 1 : 0); +} + } } // namespace v8::internal diff --git a/src/scopes.h b/src/scopes.h index c95def3b2..d08e29482 100644 --- a/src/scopes.h +++ b/src/scopes.h @@ -303,8 +303,11 @@ class Scope: public ZoneObject { // --------------------------------------------------------------------------- // Variable allocation. - // Collect all used locals in this scope. - void CollectUsedVariables(ZoneList* locals); + // Collect stack and context allocated local variables in this scope. Note + // that the function variable - if present - is not collected and should be + // handled separately. + void CollectStackAndContextLocals(ZoneList* stack_locals, + ZoneList* context_locals); // Resolve and fill in the allocation information for all variables // in this scopes. Must be called *after* all scopes have been @@ -323,6 +326,9 @@ class Scope: public ZoneObject { int num_stack_slots() const { return num_stack_slots_; } int num_heap_slots() const { return num_heap_slots_; } + int StackLocalCount() const; + int ContextLocalCount() const; + // Make sure this scope and all outer scopes are eagerly compiled. void ForceEagerCompilation() { force_eager_compilation_ = true; } diff --git a/src/variables.cc b/src/variables.cc index a0952a1f2..aa6a010fa 100644 --- a/src/variables.cc +++ b/src/variables.cc @@ -85,4 +85,12 @@ bool Variable::is_global() const { return mode_ != TEMPORARY && scope_ != NULL && scope_->is_global_scope(); } + +int Variable::CompareIndex(Variable* const* v, Variable* const* w) { + int x = (*v)->index(); + int y = (*w)->index(); + // Consider sorting them according to type as well? + return x - y; +} + } } // namespace v8::internal diff --git a/src/variables.h b/src/variables.h index 15788fb09..f20bd399c 100644 --- a/src/variables.h +++ b/src/variables.h @@ -159,6 +159,8 @@ class Variable: public ZoneObject { index_ = index; } + static int CompareIndex(Variable* const* v, Variable* const* w); + private: Scope* scope_; Handle name_; -- 2.34.1