This looks good. You might consider manually inlining RawSyncElementAt at its two call sites and getting rid of its definition. Then, at the call site in SyncRange (in the loop), you can probably simplify further to just call the right one of the lower-level sync functions.
If you do that, I'd change the names RawSyncElementBelowStackPointer => SyncElementBelowStackPointer and RawSyncElementByPushing => SyncElementByPushing. http://codereview.chromium.org/49029/diff/1/2 File src/virtual-frame-ia32.cc (right): http://codereview.chromium.org/49029/diff/1/2#newcode67 Line 67: break; // The current value on the stack is some safe value. I'd get rid of the comment. It's not clear what "safe" means (it's not safe for space usage, for instance) and this is not the place to explain it anyway. http://codereview.chromium.org/49029/diff/1/2#newcode70 Line 70: // There was an early bailout for synced elements Not an early bailout, it's a precondition (and you might consider to ASSERT that !element.is_synced() to explicitly catch violations). http://codereview.chromium.org/49029/diff/1/2#newcode123 Line 123: // There was an early bailout for invalid and synced elements Not an early bailout, but a precondition. In this case, it's impossible to have memory elements (or any synced elements) above the stack pointer. http://codereview.chromium.org/49029 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
