I could be wrong, but I thought this should be simpler (and more complicated in the 'look up in context' case).
http://codereview.chromium.org/354027/diff/1/3 File src/x64/fast-codegen-x64.cc (right): http://codereview.chromium.org/354027/diff/1/3#newcode1014 Line 1014: Comment cmnt(masm_, "[ UnaryOperationVOID"); OK to put specific comments for each type of operation. If you do take the general comment away ("[ UnaryOperation"), because there is no point in having the two immediately adjacent. (It wastes space in the disassembly and the reloc info, and doesn't add anything). I find something like "UnaryOperation: VOID" or "UnaryOperation (VOID)" more readable. http://codereview.chromium.org/354027/diff/1/3#newcode1104 Line 1104: if (proxy != NULL && (rewrite = proxy->var()->rewrite()) == NULL) { The variable 'rewrite' is assigned here but never used. I think the whole test reads better as: (proxy != NULL && proxy->var()->is_global()) http://codereview.chromium.org/354027/diff/1/3#newcode1106 Line 1106: __ push(CodeGenerator::GlobalObject()); Can't we accomplish the same thing with a simple load? Couldn't the whole body of this if be: __ push(CodeGenerator::GlobalObject); __ Move(rcx, proxy->name()); Handle<Code> ic = ...unitialized load IC... __ Call(ic, RelocInfo::CODE_TARGET); __ push(rax); Code seems shorter, and doesn't have two separate entries into the runtime to fetch the property in the case that it is present (the common case?). http://codereview.chromium.org/354027/diff/1/3#newcode1126 Line 1126: Visit(expr->expression()); Ultimately, we will have to handle variables that have to be looked up in the context here as well. Do you have a plan for that? http://codereview.chromium.org/354027 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
