All comments 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 On 2010/02/19 13:11:21, Kevin Millikin wrote:
The frame doesn't really load the global, the code generator does.
The register
allocator will prefer eax if it's free.
Done. 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 On 2010/02/19 13:11:21, Kevin Millikin wrote:
Same. Frame is not involved in selecting a register as long as there
are
registers free.
Done. http://codereview.chromium.org/650028/diff/1/2#newcode4782 src/ia32/codegen-ia32.cc:4782: LoadGlobal(); On 2010/02/19 13:11:21, Kevin Millikin wrote:
This doesn't really belong here but down below.
Done. http://codereview.chromium.org/650028/diff/1/2#newcode4846 src/ia32/codegen-ia32.cc:4846: answer = EmitNamedGlobalStore(name); On 2010/02/19 13:11:21, Kevin Millikin wrote:
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.
Done. is_contextual flag added to EmitNamedStore, and to Frame::CallStoreIC that it calls. http://codereview.chromium.org/650028 -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
