LGTM. I like the code for try/finally much better than the optimizing compiler.
http://codereview.chromium.org/492002/diff/24/27 File src/fast-codegen.cc (right): http://codereview.chromium.org/492002/diff/24/27#newcode492 src/fast-codegen.cc:492: // The try-finally construct can enter the finally block in three ways: Consider some judicious code generator comments, to possibly help a reader understand the generated code. http://codereview.chromium.org/492002/diff/24/27#newcode499 src/fast-codegen.cc:499: // the fially block through finally_entry label. "fially" ==> "finally" http://codereview.chromium.org/492002/diff/24/27#newcode504 src/fast-codegen.cc:504: // and a value in the result register (rax/eax/r0), both of which must Probably should note that the value may be the return value or an exception value. http://codereview.chromium.org/492002/diff/24/27#newcode525 src/fast-codegen.cc:525: Visit(stmt->finally_block()); Isn't it OK to VisitStatements the body of this block? (We do in the optimizing backend.) The difference would only be breaks targeting the block, right? We should do the same thing for the try and finally blocks (unless there's a reason), otherwise the reader will wonder. http://codereview.chromium.org/492002/diff/24/27#newcode526 src/fast-codegen.cc:526: ReturnFromFinallyBlock(); // Performs a return to the calling code. Can we call this ExitFinallyBlock, to parallel EnterFinallyBlock and because "return" is ambiguous? http://codereview.chromium.org/492002/diff/24/27#newcode532 src/fast-codegen.cc:532: __ PushTryHandler(IN_JAVASCRIPT, TRY_FINALLY_HANDLER); It seems nicer to properly nest the Push/Pop of the handler with the nesting stack: { TryFinally try_block(this); __ PushTryHandler(...); /* ... */ __ PopTryHandler(); } http://codereview.chromium.org/492002/diff/24/29 File src/ia32/fast-codegen-ia32.cc (right): http://codereview.chromium.org/492002/diff/24/29#newcode1642 src/ia32/fast-codegen-ia32.cc:1642: __ mov(edx, Operand(esp, 0)); Assert result_register is not edx? http://codereview.chromium.org/492002/diff/24/29#newcode1649 src/ia32/fast-codegen-ia32.cc:1649: __ push(eax); __ push(result_register()) http://codereview.chromium.org/492002/diff/24/29#newcode1657 src/ia32/fast-codegen-ia32.cc:1657: __ mov(edx, Operand(esp, 0)); Is this really better than: __ pop(edx); __ sar(edx, 1); __ add(Operand(edx), Immediate(masm_->CodeObject())); __ jmp(Operand(edx)); http://codereview.chromium.org/492002 -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
