I have added the register-to-register moves to this changelist, in MergeTo, but not yet addressed the comments. Check out the code.
On Wed, Dec 10, 2008 at 10:17 AM, Kevin Millikin <[EMAIL PROTECTED]>wrote: > On Wed, Dec 10, 2008 at 2:04 AM, <[EMAIL PROTECTED]> wrote: > >> Line 1225: Register r1 = allocator()->Allocate(); >> I do not understand why you bother loading the constant value into a >> register in this case. You are comparing two constants so the outcome >> should be determined at compile time and you can branch straight to the >> appropriate label. >> > > That is the intermediate-term plan. The main reasons not to do it > immediately revolve around keeping this change small. > > * We want to see a performance improvement on the loop and sum benchmarks > before making bigger changes, so we're focusing on the stuff we need for > those and falling back on the existing way of doing things in all other > cases. > > * We want to make sure that new code is adequately tested and that changes > are small enough to easily roll back. Right now, the constant/constant case > can't actually get hit (but it could by making this change a little bit > bigger). I actually suggested UNIMPLEMENTED for that case :) > > * We have to adequately handle dead code during expression compilation. > Right now an unconditional jump deeply nested in an expression would > generate a bunch of dead code for the remainder of the expression as we > return out of the visitor. That's safe right now, but as we start to > improve things we want to just avoid it. (The same thing can happen already > when we hit a stack overflow at compile-time in the visitor.) > -- We can IMAGINE what is not --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
