http://codereview.chromium.org/10993/diff/1/5 File src/codegen-ia32.cc (right):
http://codereview.chromium.org/10993/diff/1/5#newcode2600 Line 2600: // Smis are loaded in two steps via a temporary register. On 2008/11/28 10:32:45, William Hesse wrote: > Is now the time for special-casing small literals, and 0? > Load 0 by xor, and load literals between -255 and 255, or 16-bit literals, > specially? I think this would hit a majority of code literals. It's only the non-"InlineSmi" literals that are loaded in two pieces---those that don't fit in 16 bits. Small ones hit the path to be pushed as a single virtual frame element. If we eventually spill literals in VirtualFrame, we use MacroAssembler::Set, which takes care of using the xor trick for zero. Ultimately, we can even split the literal up lazily, so if it gets constant-folded there is no need. http://codereview.chromium.org/10993/diff/1/3 File src/virtual-frame-ia32.cc (right): http://codereview.chromium.org/10993/diff/1/3#newcode350 Line 350: void VirtualFrame::StoreToFrameSlotAt(int index) { On 2008/11/28 10:32:45, William Hesse wrote: > So this changelist enables the register and memory types, and also the constant > type, since there is no extra processing for the constant type? Should it be > commented that this correctly handles constants, since there is no explicit > mention of it in the code? It is expected to correctly handle constants. http://codereview.chromium.org/10993/diff/1/3#newcode358 Line 358: elements_[index] = top; On 2008/11/28 10:32:45, William Hesse wrote: > Don't we need to unuse elements_[index] if it is a register here? Yes. Good catch. http://codereview.chromium.org/10993/diff/1/3#newcode363 Line 363: ASSERT(!tmp.is(no_reg)); On 2008/11/28 10:32:45, William Hesse wrote: > Will we actually need error handling code in the future, to deal with the case > that the allocator fails? > Or did we decide that the allocator must always be able to succeed, by spilling, > because we will never have all regs externally used? We don't have a good way to handle that yet. I'm trying to put these assertions everywhere it could occur and we can address it when it does. The only way it can happen is if we are in new-codegen code (using the frame) and we have all registers referenced from outside the frame. That doesn't happen right now, because we never use more than one non-frame register (as a temporary) yet. Statically, we can guarantee that it can't happen, but I'm not sure what's the best way to enforce that in the code other than assertions and good tests.... http://codereview.chromium.org/10993/diff/1/4 File src/virtual-frame-ia32.h (right): http://codereview.chromium.org/10993/diff/1/4#newcode73 Line 73: type_ = (type_ & ~SyncField::mask()) | SyncField::encode(SYNCED); On 2008/11/28 10:32:45, William Hesse wrote: > Why isn't there a set_field accessor for bit fields? Is this the way we always > set bit fields? I don't know why there isn't such an accessor. It's not something we do very often, I guess (we don't have a lot of mutable bit fields). This seems like a safe way to set/clear it that doesn't rely on knowing which other fields are packed in with it or which sense the flag is. It's the same way we do it in AccessorInfo::set_property_attributes, for instance. http://codereview.chromium.org/10993 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
