Looks good to me, but I have some questions.
http://codereview.chromium.org/11232/diff/201/13 File src/codegen-ia32.cc (left): http://codereview.chromium.org/11232/diff/201/13#oldcode203 Line 203: // Save the arguments object pointer, if any. What is this change? http://codereview.chromium.org/11232/diff/201/10 File src/virtual-frame-ia32.cc (right): http://codereview.chromium.org/11232/diff/201/10#newcode49 Line 49: frame_pointer_(kIllegalIndex) { In this case, is IllegalIndex actually the accurate value of frame_pointer, not a sign of illegality? Perhaps illegalIndex should not be -1, but should be a less likely negative value. http://codereview.chromium.org/11232/diff/201/10#newcode56 Line 56: // When cloned, a frame is a deep copy of the original. How deep? Are heap objects cloned? http://codereview.chromium.org/11232/diff/201/10#newcode77 Line 77: ASSERT(stack_pointer_ == elements_.length() - 1); Isn't this too strong a precondition for a useful Adjust()? Won't you want to use Adjust on more complex frames? http://codereview.chromium.org/11232/diff/201/10#newcode109 Line 109: __ mov(Operand(ebp, fp_relative(i)), Immediate(elements_[i].handle())); Is the splitting into two 16-bit quantities inside mov? http://codereview.chromium.org/11232/diff/201/11 File src/virtual-frame-ia32.h (right): http://codereview.chromium.org/11232/diff/201/11#newcode52 Line 52: Type type() const { return static_cast<Type>(type_ & ~0x1); } Name the dirty flag mask. Unless you are also storing SMIs in this word, I would use a higher bit for the flag, and store the types in the low bits, rather than using only even type IDs. http://codereview.chromium.org/11232/diff/201/11#newcode66 Line 66: Handle<Object> handle() const { return Handle<Object>(data_.handle_); } Put asserts here to assert it is a type that has handle data? http://codereview.chromium.org/11232/diff/201/11#newcode76 Line 76: } data_; ASSERT( sizeof data_ == sizeof what? ) http://codereview.chromium.org/11232/diff/201/11#newcode254 Line 254: void PrepareCall(int count); PrepareForCall? http://codereview.chromium.org/11232 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
