A few small comments.
http://codereview.chromium.org/17607/diff/202/3 File src/codegen-ia32.cc (right): http://codereview.chromium.org/17607/diff/202/3#newcode419 Line 419: VirtualFrame::SpilledScope spilled_scope(this); Do we still need this spill? http://codereview.chromium.org/17607/diff/202/3#newcode646 Line 646: __ test(eax, Operand(eax)); Here we should (1) allocate eax, assert we get it, and then test it or else (2) have CallStub return an eax Result. http://codereview.chromium.org/17607/diff/202/3#newcode4060 Line 4060: if (has_valid_frame() || is_true.is_linked()) { Please rewrite this to: if (is_true.is_linked()) { is_true.Bind(); } if (has_valid_frame()) { LoadCondition(...); } http://codereview.chromium.org/17607/diff/202/3#newcode4064 Line 4064: true_target(), false_target(), false); Indentation is wrong here. http://codereview.chromium.org/17607/diff/202/3#newcode4102 Line 4102: if (has_valid_frame() || is_false.is_linked()) { Should be rewritten the same as above. http://codereview.chromium.org/17607/diff/202/3#newcode4106 Line 4106: true_target(), false_target(), false); Indentation is wrong here. http://codereview.chromium.org/17607 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
