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) {
On 2010/03/10 16:41:14, Kevin Millikin wrote:
Could we get rid of num_stack_variables and use
if (function->scope()->num_parameters() > 0 ||
function->scope()->num_stack_slots() > 0) { ... }
Done.
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) {
On 2010/03/10 16:41:14, Kevin Millikin wrote:
It might make more sense to require var to be non-NULL and check for
it at the
call sites.
Done.
http://codereview.chromium.org/669155/diff/35001/9002#newcode1185
src/data-flow.cc:1185: void
AssignedVariablesAnalyzer::ComputeTrivialExpression(Expression* expr) {
On 2010/03/10 16:41:14, Kevin Millikin wrote:
It doesn't really Compute. Maybe "MarkIfTrivial"?
Done.
http://codereview.chromium.org/669155/diff/35001/9002#newcode1189
src/data-flow.cc:1189: !var->is_this() &&
On 2010/03/10 16:41:14, Kevin Millikin wrote:
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)))) {
....
}
Done.
http://codereview.chromium.org/669155/diff/35001/9002#newcode1423
src/data-flow.cc:1423: RecordAssignedVar(var);
On 2010/03/10 16:41:14, Kevin Millikin wrote:
Then you'd have
if (var != NULL) RecordAssignedVar(var);
here.
Done.
http://codereview.chromium.org/669155/diff/35001/9002#newcode1425
src/data-flow.cc:1425: ProcessExpression(expr->value());
On 2010/03/10 16:41:14, Kevin Millikin wrote:
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)...
Done.
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) {
On 2010/03/10 16:41:14, Kevin Millikin wrote:
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;
}
Done.
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);
On 2010/03/10 16:41:14, Kevin Millikin wrote:
Maybe SetTypeForLocalAt?
Done.
http://codereview.chromium.org/669155
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
