I patched this code into my workspace, and seems to generate plausible code. We will need (as a separate change) to handle the deferred code properly. A few more short comments.
http://codereview.chromium.org/13665/diff/15/16 File src/codegen-ia32.cc (right): http://codereview.chromium.org/13665/diff/15/16#newcode1194 Line 1194: left_side_(left_side), right_side_(right_side) { The style is to put each initializer on its own line. http://codereview.chromium.org/13665/diff/15/16#newcode1212 Line 1212: frame_->SpillAll(); The frame will already be spilled on entry. http://codereview.chromium.org/13665/diff/15/16#newcode1235 Line 1235: Register comparee_reg = allocator()->Allocate(); I would call this register "temp", because it is not (yet) the comparee_reg and it's confusing to have comparee and comparee_reg at the same time (and have them be different). http://codereview.chromium.org/13665/diff/15/16#newcode1237 Line 1237: __ Set(comparee_reg, Immediate(temp.handle())); What's temp? http://codereview.chromium.org/13665/diff/15/16#newcode1238 Line 1238: comparee = Result(comparee_reg, this); // Usage count has net change 0. Seems wrong. The allocator has incremented the reference count and the constructor for the Result has too, so the count is now 2. We should avoid explicitly calling the constructor for Result in the code generator itself. http://codereview.chromium.org/13665/diff/15/16#newcode1248 Line 1248: deferred->exit()->Bind(); At some point, the reference to the result became dead. Probably right after the cmp instruction. We will have to track registers through the deferred code and should talk about it in person. http://codereview.chromium.org/13665 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
