On top of the comments I have made here I think that this class is missing a way to communicate that an item is in memory. At least on IA32 memory operands are perfectly useful.
PS. I ran out of time looking at the actual register moves implementation. I will do that once the first set of comments is addressed. http://codereview.chromium.org/13344/diff/1/3 File src/virtual-frame-ia32.h (right): http://codereview.chromium.org/13344/diff/1/3#newcode43 Line 43: explicit Result(Register reg, CodeGenerator* cgen); Do you need explicit here? http://codereview.chromium.org/13344/diff/1/3#newcode54 Line 54: if (is_register() && !reg().is(no_reg)) { Maybe you should introduce a special type INVALID. The fact that a Result can be is_register() but return no_reg for the register value is confusing and will likely lead to edge cases which will not be caught in regular code. Basically every user of the Result class will have to not only check for is_register() but also that the register corresponding to this result is not a no_reg. http://codereview.chromium.org/13344/diff/1/3#newcode456 Line 456: // memory-to-register moves to break cycles. This part of the comment is unclear. If the value is in a register what does it mean to be converted to a memory-to-register move? And even so, is it even necessary to put the internal implementation of how cycles are broken into the function description? http://codereview.chromium.org/13344/diff/1/3#newcode460 Line 460: int first_register_move); Why does the caller of the API have to pass this parameter? Isn't it the task of this function to figure out where to start? http://codereview.chromium.org/13344 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
