I think you should remove Scope::num_stack_variables.
Other than that, there are some suggestions and LGTM. http://codereview.chromium.org/669155/diff/35001/9010 File src/compiler.cc (right): http://codereview.chromium.org/669155/diff/35001/9010#newcode82 src/compiler.cc:82: if (function->scope()->num_stack_variables() > 0) { Could we get rid of num_stack_variables and use if (function->scope()->num_parameters() > 0 || function->scope()->num_stack_slots() > 0) { ... } http://codereview.chromium.org/669155/diff/35001/9002 File src/data-flow.cc (right): http://codereview.chromium.org/669155/diff/35001/9002#newcode1178 src/data-flow.cc:1178: void AssignedVariablesAnalyzer::RecordAssignedVar(Variable* var) { It might make more sense to require var to be non-NULL and check for it at the call sites. http://codereview.chromium.org/669155/diff/35001/9002#newcode1185 src/data-flow.cc:1185: void AssignedVariablesAnalyzer::ComputeTrivialExpression(Expression* expr) { It doesn't really Compute. Maybe "MarkIfTrivial"? http://codereview.chromium.org/669155/diff/35001/9002#newcode1189 src/data-flow.cc:1189: !var->is_this() && You could remove the special case for this in VariableProxy::IsTrivial, and have if (var != NULL && var->IsStackAllocated() && (var->is_this() || !av_.Contains(BitIndex(var)))) { .... } http://codereview.chromium.org/669155/diff/35001/9002#newcode1423 src/data-flow.cc:1423: RecordAssignedVar(var); Then you'd have if (var != NULL) RecordAssignedVar(var); here. http://codereview.chromium.org/669155/diff/35001/9002#newcode1425 src/data-flow.cc:1425: ProcessExpression(expr->value()); It doesn't make any difference, but since the assignment to var happens after the evaluation of expr->value(), it might make sense to have ProcessExpression(expr->value()); RecordAssignedVar(var)... http://codereview.chromium.org/669155/diff/35001/9006 File src/ia32/virtual-frame-ia32.cc (right): http://codereview.chromium.org/669155/diff/35001/9006#newcode1174 src/ia32/virtual-frame-ia32.cc:1174: if (proxy != NULL) { I don't think there needs to be a special case for parameter -1: if (proxy != NULL) { Slot* slot = proxy->var()->slot(); ASSERT(slot != NULL); if (slot->type() == Slot::LOCAL) { PushLocalAt(slot->index()); return; } if (slot->type() == Slot::PARAMETER) { PushParameterAt(slot->index()); return; } http://codereview.chromium.org/669155/diff/35001/9008 File src/ia32/virtual-frame-ia32.h (right): http://codereview.chromium.org/669155/diff/35001/9008#newcode426 src/ia32/virtual-frame-ia32.h:426: inline void SetTypeLocalAt(int index, NumberInfo info); Maybe SetTypeForLocalAt? http://codereview.chromium.org/669155 -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
