LGTM if the --trace option still works in all cases. Comments:

http://codereview.chromium.org/16579/diff/1/3
File src/codegen-ia32.cc (right):

http://codereview.chromium.org/16579/diff/1/3#newcode306
Line 306: if (function_return_.is_linked()) {
Should (or could) this really be an else if?

http://codereview.chromium.org/16579/diff/1/3#newcode1682
Line 1682: frame_->Push(eax);  // Materialize result on the stack.
Is this really okay when the frame is invalid? Can you run the
regression test with --trace?

http://codereview.chromium.org/16579/diff/1/3#newcode1687
Line 1687: Label check_exit_codesize;
The more I look at this size checking construct, there more I feel it
needs an abstraction. How about having a stack-allocated helper that
verifies the generated code size? It seems like it's only used in this
one place, but whenever I read this code I wonder who jumps to the label
that we explicitly bind.

http://codereview.chromium.org/16579

--~--~---------~--~----~------------~-------~--~----~
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
-~----------~----~----~----~------~----~------~--~---

Reply via email to