LGTM!
http://codereview.chromium.org/546006/diff/3001/3011 File src/arm/fast-codegen-arm.cc (right): http://codereview.chromium.org/546006/diff/3001/3011#newcode785 src/arm/fast-codegen-arm.cc:785: switch (expr->context()) { This switch is identical to the one at line 703. Could they be combined somehow? http://codereview.chromium.org/546006/diff/3001/3003 File src/ia32/fast-codegen-ia32.cc (right): http://codereview.chromium.org/546006/diff/3001/3003#newcode199 src/ia32/fast-codegen-ia32.cc:199: Register scratch) { This function looks generic enough that it could be in fast-codegen.cc http://codereview.chromium.org/546006/diff/3001/3003#newcode220 src/ia32/fast-codegen-ia32.cc:220: void FastCodeGenerator::Apply(Expression::Context context, Literal* lit) { If we had MacroAssembler::Push(Handle<Object> source) on all backends and always used kResultRegister as scratch, this function could be moved to fast-codegen.cc too. http://codereview.chromium.org/546006/diff/3001/3003#newcode254 src/ia32/fast-codegen-ia32.cc:254: __ mov(eax, Operand(esp, 0)); With a function like MacroAssembler::MoveToStack(int stack_offset, Register source), this function could be generic too. http://codereview.chromium.org/546006/diff/3001/3003#newcode275 src/ia32/fast-codegen-ia32.cc:275: Register reg) { And this one too! (A good abstraction of the stack and a few registers seems to be all we are using :) http://codereview.chromium.org/546006/diff/3001/3003#newcode592 src/ia32/fast-codegen-ia32.cc:592: __ nop(); Can we assume that DropAndMove does not start with "test"? http://codereview.chromium.org/546006/diff/3001/3013 File src/x64/codegen-x64.cc (right): http://codereview.chromium.org/546006/diff/3001/3013#newcode4250 src/x64/codegen-x64.cc:4250: if (property->key()->IsPropertyName()) { This is a really great clean-up! So much easier to read and write. http://codereview.chromium.org/546006
-- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
