LGTM
http://codereview.chromium.org/7084032/diff/4004/src/arm/builtins-arm.cc File src/arm/builtins-arm.cc (right): http://codereview.chromium.org/7084032/diff/4004/src/arm/builtins-arm.cc#newcode1065 src/arm/builtins-arm.cc:1065: // Exit the JS frame and remove the parameters (except function), and JS frame -> internal frame. http://codereview.chromium.org/7084032/diff/4004/src/arm/builtins-arm.cc#newcode1135 src/arm/builtins-arm.cc:1135: // Tear down temporary frame. temporary -> internal http://codereview.chromium.org/7084032/diff/4004/src/arm/full-codegen-arm.cc File src/arm/full-codegen-arm.cc (right): http://codereview.chromium.org/7084032/diff/4004/src/arm/full-codegen-arm.cc#newcode159 src/arm/full-codegen-arm.cc:159: FrameScope frame_scope(masm_, StackFrame::NONE); How about using StackFrame::JAVA_SCRIPT, and maybe place it after the __ Push where the frame is set up? Then a FrameScopt for StackFrame::JAVA_SCRIPT of cause should do nothing but mark that a frame is present. http://codereview.chromium.org/7084032/diff/4004/src/arm/lithium-codegen-arm.cc File src/arm/lithium-codegen-arm.cc (right): http://codereview.chromium.org/7084032/diff/4004/src/arm/lithium-codegen-arm.cc#newcode89 src/arm/lithium-codegen-arm.cc:89: FrameScope frame_scope(masm_, StackFrame::NONE); StackFrame::JAVA_SCRIPT? http://codereview.chromium.org/7084032/diff/4004/src/arm/macro-assembler-arm.h File src/arm/macro-assembler-arm.h (right): http://codereview.chromium.org/7084032/diff/4004/src/arm/macro-assembler-arm.h#newcode1063 src/arm/macro-assembler-arm.h:1063: Can these two classes be in platform independent macro-assembler.h? http://codereview.chromium.org/7084032/diff/4004/src/arm/regexp-macro-assembler-arm.cc File src/arm/regexp-macro-assembler-arm.cc (right): http://codereview.chromium.org/7084032/diff/4004/src/arm/regexp-macro-assembler-arm.cc#newcode380 src/arm/regexp-macro-assembler-arm.cc:380: __ CallCFunction(function, argument_count); How about having CallCFunctionWithoutFrame or CallCFunctionNotCausingGC instead? Then the comment is not needed. http://codereview.chromium.org/7084032/diff/4004/src/builtins.cc File src/builtins.cc (right): http://codereview.chromium.org/7084032/diff/4004/src/builtins.cc#newcode1620 src/builtins.cc:1620: NoCurrentFrameScope scope(&masm); This might deserve a comment. http://codereview.chromium.org/7084032/diff/4004/src/ia32/builtins-ia32.cc File src/ia32/builtins-ia32.cc (right): http://codereview.chromium.org/7084032/diff/4004/src/ia32/builtins-ia32.cc#newcode466 src/ia32/builtins-ia32.cc:466: __ ret(1 * kPointerSize); // Remove receiver. Maybe remove 1 * while you are here. http://codereview.chromium.org/7084032/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
