http://codereview.chromium.org/9769/diff/206/11
File src/codegen-ia32.cc (right):

http://codereview.chromium.org/9769/diff/206/11#newcode158
Line 158: frame_->AllocateLocals(scope_->num_stack_slots());
On 2008/11/11 10:47:05, Erik Corry wrote:
> Should AllocateLocals be called AllocateStackSlots now?

Probably.  Changed.

http://codereview.chromium.org/9769/diff/206/8
File src/virtual-frame-ia32.cc (right):

http://codereview.chromium.org/9769/diff/206/8#newcode43
Line 43: elements_(cgen->scope()->num_parameters() + 5),
On 2008/11/11 10:47:05, Erik Corry wrote:
> 5?

You think that needs some explanation :)

http://codereview.chromium.org/9769/diff/206/9
File src/virtual-frame-ia32.h (right):

http://codereview.chromium.org/9769/diff/206/9#newcode76
Line 76: void Adjust(int count);
On 2008/11/11 10:47:05, Erik Corry wrote:
> The name Adjust seems mighty generic.  Perhaps AdjustHeight?

Perhaps.  Eventually its use will be minimized: externally it is "magic"
and shouldn't be needed, internally its overused right now because it
happens to work for pushes that are materialized.

http://codereview.chromium.org/9769/diff/206/9#newcode79
Line 79: void Forget(int count);
On 2008/11/11 10:47:05, Erik Corry wrote:
> ForgetElements?

Maybe.  It will also be minimized, so I'll hold off on trying to find
the right name for it.

http://codereview.chromium.org/9769/diff/206/9#newcode108
Line 108: ASSERT(0 <= index && index < local_count_);
On 2008/11/11 10:47:05, Erik Corry wrote:
> 2 different asserts is nicer here if one is triggered.

Agreed.  Changed.

http://codereview.chromium.org/9769/diff/206/9#newcode150
Line 150: // emit code to effect the physical frame.  Does not clobber
any registers
On 2008/11/11 10:47:05, Erik Corry wrote:
> effect -> affect

Embarassing.

http://codereview.chromium.org/9769

--~--~---------~--~----~------------~-------~--~----~
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
-~----------~----~----~----~------~----~------~--~---

Reply via email to