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. On 2008/11/18 16:37:34, William Hesse wrote: > What is this change? As far as I can tell, the only reason to save ecx here is because it will be clobbered by SlotOperand and RecordWrite. All the gymnastics with saving ecx will go away as soon as we can support registers in the virtual frame, anyway. 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) { On 2008/11/18 16:37:34, William Hesse wrote: > 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. We don't know what the frame pointer is until we set it in Enter(). http://codereview.chromium.org/11232/diff/201/10#newcode77 Line 77: ASSERT(stack_pointer_ == elements_.length() - 1); On 2008/11/18 16:37:34, William Hesse wrote: > Isn't this too strong a precondition for a useful Adjust()? Won't you want to > use Adjust on more complex frames? I think it's right. The intent of "Adjust" is that it doesn't generate code but only fixes up the frame to reflect code already (or about to be) generated. If the sp is not at the top of the virtual frame when adjust is called, the it's not safe to have someone else push on the stack. http://codereview.chromium.org/11232/diff/201/10#newcode109 Line 109: __ mov(Operand(ebp, fp_relative(i)), Immediate(elements_[i].handle())); On 2008/11/18 16:37:34, William Hesse wrote: > Is the splitting into two 16-bit quantities inside mov? There is no splitting here (yet). The only constants right now are not from source code. 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); } On 2008/11/18 16:37:34, William Hesse wrote: > 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. OK. http://codereview.chromium.org/11232/diff/201/11#newcode66 Line 66: Handle<Object> handle() const { return Handle<Object>(data_.handle_); } On 2008/11/18 16:37:34, William Hesse wrote: > Put asserts here to assert it is a type that has handle data? OK. http://codereview.chromium.org/11232/diff/201/11#newcode254 Line 254: void PrepareCall(int count); On 2008/11/18 16:37:34, William Hesse wrote: > PrepareForCall? > OK. http://codereview.chromium.org/11232 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
