Thanks, Kevin!
http://codereview.chromium.org/6625048/diff/1/src/arm/codegen-arm.cc
File src/arm/codegen-arm.cc (right):
http://codereview.chromium.org/6625048/diff/1/src/arm/codegen-arm.cc#newcode620
src/arm/codegen-arm.cc:620: ASSERT(shadow != NULL && shadow->AsSlot() !=
NULL ||
On 2011/03/07 11:46:48, Kevin Millikin wrote:
Parens around the && please.
Done.
http://codereview.chromium.org/6625048/diff/1/src/arm/full-codegen-arm.cc
File src/arm/full-codegen-arm.cc (right):
http://codereview.chromium.org/6625048/diff/1/src/arm/full-codegen-arm.cc#newcode217
src/arm/full-codegen-arm.cc:217: if (arguments_shadow != NULL) {
On 2011/03/07 11:46:48, Kevin Millikin wrote:
A bit simpler (also on the other platforms):
if (arguments_shadow != NULL) {
// Duplicate the value; move-to-slot operation might clobber
registers.
__ mov(r3, r0);
Move(arguments_shadow->AsSlot(), r3, r1, r2);
}
Move(arguments->AsSlot(), r0, r1, r2);
Done.
http://codereview.chromium.org/6625048/diff/1/src/hydrogen.cc
File src/hydrogen.cc (right):
http://codereview.chromium.org/6625048/diff/1/src/hydrogen.cc#newcode2238
src/hydrogen.cc:2238: scope->arguments_shadow() != NULL &&
On 2011/03/07 11:46:48, Kevin Millikin wrote:
Parens around the && please.
Done.
http://codereview.chromium.org/6625048/diff/1/src/parser.cc
File src/parser.cc (right):
http://codereview.chromium.org/6625048/diff/1/src/parser.cc#newcode656
src/parser.cc:656: TemporaryScope temp_scope(&this->temp_scope_);
On 2011/03/07 11:46:48, Kevin Millikin wrote:
Hmmm. TemporaryScope seems like a bad name---it's not really a
"scope" at all.
It seems like a bundle of function-specific bits (literals and a loop
flag). We
already have a stack-allocated "LexicalScope" everywhere we have a
TemporaryScope, so we should probably just combine them.
Maybe roll it into the next change you make to the parser.
Putting it on my list of opportunistic changes to make.
http://codereview.chromium.org/6625048/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
