LGTM after comments are addressed.
http://codereview.chromium.org/650028/diff/1/2 File src/ia32/codegen-ia32.cc (right): http://codereview.chromium.org/650028/diff/1/2#newcode692 src/ia32/codegen-ia32.cc:692: // If eax is free, the virtual frame will load the global object directly The frame doesn't really load the global, the code generator does. The register allocator will prefer eax if it's free. http://codereview.chromium.org/650028/diff/1/2#newcode4314 src/ia32/codegen-ia32.cc:4314: // If eax is free, the virtual frame will load the global object directly Same. Frame is not involved in selecting a register as long as there are registers free. http://codereview.chromium.org/650028/diff/1/2#newcode4782 src/ia32/codegen-ia32.cc:4782: LoadGlobal(); This doesn't really belong here but down below. http://codereview.chromium.org/650028/diff/1/2#newcode4814 src/ia32/codegen-ia32.cc:4814: if (is_trivial_receiver) { if (var != NULL) { frame()->Spill(eax); LoadGlobal(); } else if (is_trivial_receiver) { frame()->Push(prop->obj()); } else { frame()->Dup(); } http://codereview.chromium.org/650028/diff/1/2#newcode4846 src/ia32/codegen-ia32.cc:4846: answer = EmitNamedGlobalStore(name); I don't really like the name GlobalStore, because not all global stores go through it. EmitContextualStore? It's sort of annoying to have a special case for global variables in the code generator just to plumb it all the way down to the virtual frame. Maybe it would be better to pass an is_contextual flag to EmitNamedStore, analogous to EmitNamedLoad. http://codereview.chromium.org/650028 -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
