On Thu, Jan 8, 2009 at 2:13 PM, <[email protected]> wrote: > LGTM. But for consistency, we should make either all or none of the > frame's call methods responsible for allocating the eax result register. > > > http://codereview.chromium.org/17414/diff/1/4 > File src/codegen-ia32.cc (right): > > http://codereview.chromium.org/17414/diff/1/4#newcode459 > Line 459: Result temp = allocator_->Allocate(); > We haven't sorted out what happens when a call to Allocate() fails. It > can fail if there are too many off-frame register references. > > For now, we have asserts that the result is valid after all the calls to > Allocate to help us catch when it happens.
I don't think the code changes I've just made makes any difference here, but I do agree that we should find a way of dealing with it. Right now, it looks like we don't have that many off-frame register references. > http://codereview.chromium.org/17414/diff/1/4#newcode1358 > Line 1358: Result result = allocator_->Allocate(eax); > CallStub should be responsible for allocating the eax result and > returning it. There are some overloaded ones that do already. > > http://codereview.chromium.org/17414/diff/1/4#newcode3198 > Line 3198: Result result = allocator_->Allocate(eax); > Ditto CallCodeObject. My plan was to do this once all the CallCodeObject and CallStub sites had been rewritten to deal with register allocation. I hope that's okay. > http://codereview.chromium.org/17414/diff/1/4#newcode3266 > Line 3266: frame_->EmitPush(frame_->ElementAt(ref.size())); > There is a function in the frame class that copies a (base-relative) > frame element to the top of the frame without requiring spilling. Maybe > it's worth considering the top-relative version of that here even though > we know a call is coming right up. That's certainly the next thing on the list. Cheers, Kasper --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
