Seems about as simple as it can be. It will also be nicer to allow the generated code to propagate results in an accumulator register.
http://codereview.chromium.org/496009/diff/1/5 File src/compiler.cc (right): http://codereview.chromium.org/496009/diff/1/5#newcode1028 src/compiler.cc:1028: BAILOUT("Lookup slot"); Maybe change the string to mention count operation, so it doesn't overlap with another bailout string. http://codereview.chromium.org/496009/diff/1/5#newcode1032 src/compiler.cc:1032: ASSERT(prop->obj()->context() == Expression::kUninitialized || This assert is actually in ProcessExpression, so we don't need it here. http://codereview.chromium.org/496009/diff/1/5#newcode1051 src/compiler.cc:1051: BAILOUT("non-variable/non-property assignment"); Change this string too. http://codereview.chromium.org/496009/diff/1/3 File src/fast-codegen.h (right): http://codereview.chromium.org/496009/diff/1/3#newcode254 src/fast-codegen.h:254: // Load a value from a named property and push the result on the stack. "named" ==> "keyed" http://codereview.chromium.org/496009/diff/1/4 File src/ia32/fast-codegen-ia32.cc (right): http://codereview.chromium.org/496009/diff/1/4#newcode1403 src/ia32/fast-codegen-ia32.cc:1403: switch (assign_type) { Maybe remove the repeated code with: if (assign_type == VARIABLE) { EmitVariableLoad(...); } else { // Reserve space for result of postfix operation. Visit(prop->obj()); if (assign_type == NAMED_PROPERTY) { EmitNamedPropertyLoad(...); } else { Visit(prop->key(); EmitKeyedPropertyLoad(...); } } http://codereview.chromium.org/496009/diff/1/4#newcode1479 src/ia32/fast-codegen-ia32.cc:1479: EmitVariableAssignment(expr->expression()->AsVariableProxy()->var(), It means repeating the call to EmitVariableAssignment (but avoids testing is_postfix twice). I like: if (expr->is_postfix()) { EmitVariableAssignment(..., Expression::kEffect); // Postfix-specific handling. } else { EmitVariableAssignment(..., expr->context()); } http://codereview.chromium.org/496009/diff/1/4#newcode1525 src/ia32/fast-codegen-ia32.cc:1525: __ Drop(1); // Result in on the stack under the receiver. "in" ==> "is" http://codereview.chromium.org/496009/diff/1/4#newcode1526 src/ia32/fast-codegen-ia32.cc:1526: if (expr->context() != Expression::kEffect) { You might consider if (expr->context() != Expression::kEffect && expr->context() != Expression::kValue) { ... } It matches what you do for variables, and avoids pop/push for value contexts. http://codereview.chromium.org/496009/diff/1/4#newcode1542 src/ia32/fast-codegen-ia32.cc:1542: if (expr->context() != Expression::kEffect) { Same comment. http://codereview.chromium.org/496009 -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
