Drive-by comments (just the usual :).

http://codereview.chromium.org/541047/diff/1004/1006
File src/ia32/fast-codegen-ia32.cc (right):

http://codereview.chromium.org/541047/diff/1004/1006#newcode200
src/ia32/fast-codegen-ia32.cc:200: switch (context) {
Could this function be in fast-codegen.cc? It seems entirely platform
independent.

http://codereview.chromium.org/541047/diff/1004/1006#newcode247
src/ia32/fast-codegen-ia32.cc:247: Apply(context, eax);
If eax is converted to result_register(), this function looks generic
too.

http://codereview.chromium.org/541047/diff/1004/1006#newcode286
src/ia32/fast-codegen-ia32.cc:286: __ mov(eax, Operand(esp, 0));
If we hae a macro to load the top of stack into a register (or possibly
any stack-pointer indexed word), this function could be generic too.

http://codereview.chromium.org/541047/diff/1004/1006#newcode483
src/ia32/fast-codegen-ia32.cc:483: __ mov(Operand(ebp,
SlotOffset(slot)), result_register());
Could this use StoreToFrameField?

http://codereview.chromium.org/541047/diff/1004/1006#newcode507
src/ia32/fast-codegen-ia32.cc:507: result_register());
Could we have an opposite of LoadContextField to use here?

http://codereview.chromium.org/541047/diff/1004/1006#newcode1022
src/ia32/fast-codegen-ia32.cc:1022: TestAndBranch(eax, true_label_,
&discard);
Maybe use result_register() here for consistency?

http://codereview.chromium.org/541047
-- 
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to