LGTM. You should go back (in another change) and make use of the new abstractions for frame and context accesses.
http://codereview.chromium.org/491077/diff/1/4 File src/fast-codegen.cc (right): http://codereview.chromium.org/491077/diff/1/4#newcode341 src/fast-codegen.cc:341: if (FLAG_debug_info) { SetStatementPosition(stmt)? http://codereview.chromium.org/491077/diff/1/4#newcode361 src/fast-codegen.cc:361: if (FLAG_debug_info) { Same. http://codereview.chromium.org/491077/diff/1/5 File src/fast-codegen.h (right): http://codereview.chromium.org/491077/diff/1/5#newcode294 src/fast-codegen.h:294: static Register context_register(); This seems like going overboard to avoid platform-specific code in the backend. But if you want do this, we have lots of explicit references to the register in the toplevel code generator that should change to use the new abstraction. http://codereview.chromium.org/491077/diff/1/5#newcode298 src/fast-codegen.h:298: void StoreFrameField(int frame_offset, Register value); Not all of the constants in frames-{arm,ia32,x64}.h are fp relative. Probably should mention that the offset here is fp relative. We have a lot of uses of this pattern (and loading from a frame field, and pushing one) in the fast code generator. They should be rewritten. For some reason, I like StoreTo... or Set... better as names. http://codereview.chromium.org/491077/diff/1/5#newcode302 src/fast-codegen.h:302: void LoadContextField(Register dst, int context_index); We have a lot of uses of this (or a similar) pattern too. Search for Context::SlotOffset and CodeGenerator::ContextOperand in the fast code generator. http://codereview.chromium.org/491077 -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
