LGTM.
http://codereview.chromium.org/6263005/diff/6001/src/ia32/lithium-gap-resolver-ia32.cc File src/ia32/lithium-gap-resolver-ia32.cc (right): http://codereview.chromium.org/6263005/diff/6001/src/ia32/lithium-gap-resolver-ia32.cc#newcode35 src/ia32/lithium-gap-resolver-ia32.cc:35: : cgen_(owner), moves_(32), spilled_register_(-1) { Can we use no_reg, or no_reg.code(), for no spilled register? http://codereview.chromium.org/6263005/diff/6001/src/ia32/lithium-gap-resolver-ia32.cc#newcode104 src/ia32/lithium-gap-resolver-ia32.cc:104: for (int i = moves_.length() - 1; i >= 0; --i) { Do we have the source use counts at this point? I guess only for registers. Can we skip this loop if the destination is a register, and its use count is 0? http://codereview.chromium.org/6263005/diff/6001/src/ia32/lithium-gap-resolver-ia32.cc#newcode217 src/ia32/lithium-gap-resolver-ia32.cc:217: moves_.Rewind(0); Shouldn't we have a List::Clear for Rewind(0), and also shouldn't Rewind() assert that the new index < length_? Obviously, this could go in a different change. http://codereview.chromium.org/6263005/diff/6001/src/ia32/lithium-gap-resolver-ia32.cc#newcode265 src/ia32/lithium-gap-resolver-ia32.cc:265: // combinations are possible. Are only combinations prohibited by the double vs word size difference, and constants as destinations, prohibited, or are there other, hidden, non-supported cases? http://codereview.chromium.org/6263005/diff/6001/src/ia32/lithium-gap-resolver-ia32.cc#newcode340 src/ia32/lithium-gap-resolver-ia32.cc:340: // spilling src at this point. Memory-to-memory swaps are a rare case. Why is the memory-to-memory comment here? http://codereview.chromium.org/6263005/diff/6001/src/lithium.cc File src/lithium.cc (right): http://codereview.chromium.org/6263005/diff/6001/src/lithium.cc#newcode53 src/lithium.cc:53: stream->Add(" = "); Why not use a "move arrow" "<-" rather than "=" when printing it out? http://codereview.chromium.org/6263005/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
